Skip to content

Prevent nil entity in MLE #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AryaHassanli wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Prevent nil entity in MLE #42

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
098a5c2
Prevent nil entity
AryaHassanli Nov 20, 2025
48359e3
Use relative path in github action
AryaHassanli Nov 20, 2025
384352c
Revert "Prevent nil entity"
AryaHassanli Nov 20, 2025
454991b
Handle nil entity case
AryaHassanli Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions mle/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/project-chip/alchemy/internal/log"
"github.com/project-chip/alchemy/matter"
"github.com/project-chip/alchemy/matter/spec"
"github.com/project-chip/alchemy/matter/types"
)

func Process(root string, s *spec.Specification) (violations map[string][]spec.Violation, err error) {
Expand Down Expand Up @@ -130,6 +131,8 @@ func parseMasterList(filePath asciidoc.Path, idColumn matter.TableColumn, nameCo
for row := range listTable.Body() {
var id *matter.Number
var name, pics string
var entity types.Entity

id, err = listTable.ReadID(asciidoc.RawReader, row, idColumn)
if err != nil {
return
Expand All @@ -141,18 +144,32 @@ func parseMasterList(filePath asciidoc.Path, idColumn matter.TableColumn, nameCo
if err != nil {
return
}

switch idColumn {
case matter.TableColumnClusterID:
entity = &matter.Cluster{
ID: id,
Name: name,
}
case matter.TableColumnDeviceID:
entity = &matter.DeviceType{
ID: id,
Name: name,
}
}
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make this function more robust, consider adding a default case to this switch statement. While parseMasterList is currently called only with TableColumnClusterID and TableColumnDeviceID, adding a default case will prevent entity from being implicitly nil if the function is ever used with other idColumn values. This would also help in debugging by logging a warning.

		switch idColumn {
		case matter.TableColumnClusterID:
			entity = &matter.Cluster{
				ID:   id,
				Name: name,
			}
		case matter.TableColumnDeviceID:
			entity = &matter.DeviceType{
				ID:   id,
				Name: name,
			}
		default:
			slog.Warn("unhandled idColumn type in parseMasterList, violation entity will be nil", "idColumn", idColumn.String())
		}


switch name {
case "":
continue
case "Reserved":
if _, taken := reserveds[id.Value()]; taken {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("Duplicate reserved %s in Master List. ID='%s'", idColumn.String(), id.HexString())}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("Duplicate reserved %s in Master List. ID='%s'", idColumn.String(), id.HexString())}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is some repetitive code for creating and adding violations throughout this function. To improve maintainability and reduce duplication, you could extract this logic into a helper function.

For example, you could create a helper like this:

func addMasterListViolation(violations map[string][]spec.Violation, entity types.Entity, row *asciidoc.TableRow, format string, args ...any) {
	v := spec.Violation{
		Entity: entity,
		Type:   spec.ViolationMasterList,
		Text:   fmt.Sprintf(format, args...),
	}
	v.Path, v.Line = row.Origin()
	violations[v.Path] = append(violations[v.Path], v)
}

Then, you can simplify the violation reporting calls, for instance:

if _, taken := reserveds[id.Value()]; taken {
    addMasterListViolation(violations, entity, row, "Duplicate reserved %s in Master List. ID='%s'", idColumn.String(), id.HexString())
    continue
}

This would make the code cleaner and easier to manage.

continue
}
if _, taken := masterList[id.Value()]; taken {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is both reserved and in use in Master List. ID='%s'", idColumn.String(), id.HexString())}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is both reserved and in use in Master List. ID='%s'", idColumn.String(), id.HexString())}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
continue
Expand All @@ -161,19 +178,19 @@ func parseMasterList(filePath asciidoc.Path, idColumn matter.TableColumn, nameCo
continue
}
if _, reserved := reserveds[id.Value()]; reserved {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is both reserved and in use in Master List. ID='%s'", idColumn.String(), id.HexString())}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is both reserved and in use in Master List. ID='%s'", idColumn.String(), id.HexString())}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
continue
}
if _, taken := masterList[id.Value()]; taken {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. ID='%s'", idColumn.String(), id.HexString())}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. ID='%s'", idColumn.String(), id.HexString())}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
continue
}
if _, taken := uniqueNames[name]; taken {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. name='%s'", nameColumn.String(), name)}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. name='%s'", nameColumn.String(), name)}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
continue
Expand All @@ -185,7 +202,7 @@ func parseMasterList(filePath asciidoc.Path, idColumn matter.TableColumn, nameCo
}
if pics != "" {
if _, taken := uniquePics[pics]; taken {
v := spec.Violation{Entity: nil, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. PICS='%s'", picsColumn.String(), pics)}
v := spec.Violation{Entity: entity, Type: spec.ViolationMasterList, Text: fmt.Sprintf("%s is duplicated on Master List. PICS='%s'", picsColumn.String(), pics)}
v.Path, v.Line = row.Origin()
violations[v.Path] = append(violations[v.Path], v)
continue
Expand Down