Skip to content

Commit 506fb1c

Browse files
Andries-Smitclaude
andcommitted
fix(lint): respect lint-config.yaml in mxcli lint CLI and options in Starlark rules
The `mxcli lint` CLI command was ignoring the lint-config.yaml file entirely — config loading only existed in the executor (REPL/MDL script) path, not the CLI subcommand. Additionally, rule `options` from the YAML were never passed to rules even when loaded. Fixes: - Wire FindConfigFile/LoadConfig/ApplyConfig into cmd/mxcli/cmd_lint.go so excludeModules, rule enabled flags, severity overrides, and options are all respected when running `mxcli lint` - Add Configurable interface (linter.go): rules implementing Configure(options) receive their options map before Check is called - Fix ModuleRoles() and RoleMappings() iterators in context.go to apply IsExcluded() so Starlark rules using module_roles()/role_mappings() respect the excludeModules list - Add get_option(key, default) builtin to Starlark rules so .star files can read per-rule options from lint-config.yaml at check time - Implement Configurable on DomainModelSizeRule with max_entities option - Add config_test.go covering LoadConfig YAML parsing, ApplyConfig, and FindConfigFile; extend linter_test.go and domain_size_test.go for new paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1yYeYn9rosf8UdAimdatt
1 parent d1b17a4 commit 506fb1c

8 files changed

Lines changed: 394 additions & 0 deletions

File tree

cmd/mxcli/cmd_lint.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,19 @@ Examples:
166166
}
167167
}
168168

169+
// Load lint config file and apply (excludedModules, rule severity/enabled overrides).
170+
// Config ExcludeModules merges with --exclude flag values.
171+
configPath := linter.FindConfigFile(projectDir)
172+
if cfg, err := linter.LoadConfig(configPath); err == nil {
173+
if len(cfg.ExcludeModules) > 0 {
174+
merged := append(excludeModules, cfg.ExcludeModules...)
175+
ctx.SetExcludedModules(merged)
176+
}
177+
cfg.ApplyConfig(lint)
178+
} else {
179+
fmt.Fprintf(os.Stderr, "Warning: failed to load lint config: %v\n", err)
180+
}
181+
169182
// If --rules is specified, disable every rule not in the allowlist.
170183
if len(onlyRules) > 0 {
171184
allowed := make(map[string]bool, len(onlyRules))

mdl/linter/config_test.go

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package linter
4+
5+
import (
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
)
10+
11+
func writeYAML(t *testing.T, content string) string {
12+
t.Helper()
13+
dir := t.TempDir()
14+
path := filepath.Join(dir, "lint-config.yaml")
15+
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
16+
t.Fatalf("write YAML: %v", err)
17+
}
18+
return path
19+
}
20+
21+
func TestLoadConfig_ExcludedModules(t *testing.T) {
22+
path := writeYAML(t, `
23+
excludeModules:
24+
- Administration
25+
- Anonymous
26+
`)
27+
cfg, err := LoadConfig(path)
28+
if err != nil {
29+
t.Fatalf("LoadConfig: %v", err)
30+
}
31+
if len(cfg.ExcludeModules) != 2 {
32+
t.Fatalf("want 2 excluded modules, got %d", len(cfg.ExcludeModules))
33+
}
34+
if cfg.ExcludeModules[0] != "Administration" || cfg.ExcludeModules[1] != "Anonymous" {
35+
t.Errorf("unexpected modules: %v", cfg.ExcludeModules)
36+
}
37+
}
38+
39+
func TestLoadConfig_RuleEnabled(t *testing.T) {
40+
path := writeYAML(t, `
41+
rules:
42+
MPR001:
43+
enabled: false
44+
`)
45+
cfg, err := LoadConfig(path)
46+
if err != nil {
47+
t.Fatalf("LoadConfig: %v", err)
48+
}
49+
rule, ok := cfg.Rules["MPR001"]
50+
if !ok {
51+
t.Fatal("MPR001 rule not found")
52+
}
53+
if rule.Enabled == nil || *rule.Enabled != false {
54+
t.Errorf("expected enabled=false, got %v", rule.Enabled)
55+
}
56+
}
57+
58+
func TestLoadConfig_RuleSeverity(t *testing.T) {
59+
path := writeYAML(t, `
60+
rules:
61+
PH009:
62+
severity: hint
63+
`)
64+
cfg, err := LoadConfig(path)
65+
if err != nil {
66+
t.Fatalf("LoadConfig: %v", err)
67+
}
68+
if cfg.Rules["PH009"].Severity != "hint" {
69+
t.Errorf("expected severity hint, got %q", cfg.Rules["PH009"].Severity)
70+
}
71+
}
72+
73+
func TestLoadConfig_RuleOptions(t *testing.T) {
74+
path := writeYAML(t, `
75+
rules:
76+
MPR003:
77+
options:
78+
max_entities: 20
79+
`)
80+
cfg, err := LoadConfig(path)
81+
if err != nil {
82+
t.Fatalf("LoadConfig: %v", err)
83+
}
84+
opts := cfg.Rules["MPR003"].Options
85+
if opts == nil {
86+
t.Fatal("expected options map, got nil")
87+
}
88+
// YAML numbers unmarshal as int when they fit.
89+
v, ok := opts["max_entities"]
90+
if !ok {
91+
t.Fatal("max_entities not found in options")
92+
}
93+
switch n := v.(type) {
94+
case int:
95+
if n != 20 {
96+
t.Errorf("max_entities = %d, want 20", n)
97+
}
98+
case float64:
99+
if n != 20 {
100+
t.Errorf("max_entities = %v, want 20", n)
101+
}
102+
default:
103+
t.Errorf("unexpected type %T for max_entities", v)
104+
}
105+
}
106+
107+
func TestLoadConfig_MissingFileReturnsDefault(t *testing.T) {
108+
cfg, err := LoadConfig("/nonexistent/lint-config.yaml")
109+
if err != nil {
110+
t.Fatalf("expected no error for missing file, got %v", err)
111+
}
112+
if len(cfg.ExcludeModules) != 0 || len(cfg.Rules) != 0 {
113+
t.Errorf("expected empty default config, got %+v", cfg)
114+
}
115+
}
116+
117+
func TestApplyConfig_DisablesRule(t *testing.T) {
118+
path := writeYAML(t, `
119+
rules:
120+
MPR001:
121+
enabled: false
122+
`)
123+
cfg, _ := LoadConfig(path)
124+
125+
lint := New(nil)
126+
lint.AddRule(&stubRule{"MPR001"})
127+
lint.AddRule(&stubRule{"MPR002"})
128+
cfg.ApplyConfig(lint)
129+
130+
configs := lint.configs
131+
if c, ok := configs["MPR001"]; !ok || c.Enabled {
132+
t.Errorf("MPR001 should be disabled, got %+v", c)
133+
}
134+
if c, ok := configs["MPR002"]; ok && !c.Enabled {
135+
t.Errorf("MPR002 should not be disabled, got %+v", c)
136+
}
137+
}
138+
139+
func TestApplyConfig_OverridesSeverity(t *testing.T) {
140+
path := writeYAML(t, `
141+
rules:
142+
MPR001:
143+
severity: error
144+
`)
145+
cfg, _ := LoadConfig(path)
146+
147+
lint := New(nil)
148+
lint.AddRule(&stubRule{"MPR001"})
149+
cfg.ApplyConfig(lint)
150+
151+
if c := lint.configs["MPR001"]; c.Severity != SeverityError {
152+
t.Errorf("expected SeverityError, got %v", c.Severity)
153+
}
154+
}
155+
156+
func TestApplyConfig_PassesOptions(t *testing.T) {
157+
path := writeYAML(t, `
158+
rules:
159+
MPR003:
160+
options:
161+
max_entities: 25
162+
`)
163+
cfg, _ := LoadConfig(path)
164+
165+
lint := New(nil)
166+
lint.AddRule(&stubRule{"MPR003"})
167+
cfg.ApplyConfig(lint)
168+
169+
opts := lint.configs["MPR003"].Options
170+
if opts == nil {
171+
t.Fatal("expected options in RuleConfig, got nil")
172+
}
173+
}
174+
175+
func TestFindConfigFile_LocatesRootFile(t *testing.T) {
176+
dir := t.TempDir()
177+
path := filepath.Join(dir, "lint-config.yaml")
178+
if err := os.WriteFile(path, []byte(""), 0o644); err != nil {
179+
t.Fatalf("write: %v", err)
180+
}
181+
got := FindConfigFile(dir)
182+
if got != path {
183+
t.Errorf("FindConfigFile = %q, want %q", got, path)
184+
}
185+
}
186+
187+
func TestFindConfigFile_LocatesDotClaudeFile(t *testing.T) {
188+
dir := t.TempDir()
189+
sub := filepath.Join(dir, ".claude")
190+
if err := os.MkdirAll(sub, 0o755); err != nil {
191+
t.Fatalf("mkdir: %v", err)
192+
}
193+
path := filepath.Join(sub, "lint-config.yaml")
194+
if err := os.WriteFile(path, []byte(""), 0o644); err != nil {
195+
t.Fatalf("write: %v", err)
196+
}
197+
got := FindConfigFile(dir)
198+
if got != path {
199+
t.Errorf("FindConfigFile = %q, want %q", got, path)
200+
}
201+
}
202+
203+
func TestFindConfigFile_ReturnsEmptyWhenAbsent(t *testing.T) {
204+
dir := t.TempDir()
205+
if got := FindConfigFile(dir); got != "" {
206+
t.Errorf("expected empty string, got %q", got)
207+
}
208+
}

mdl/linter/context.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,9 @@ func (ctx *LintContext) RoleMappings() iter.Seq[RoleMappingInfo] {
377377
if err := rows.Scan(&rm.UserRoleName, &rm.ModuleRoleName, &rm.ModuleName); err != nil {
378378
continue
379379
}
380+
if ctx.IsExcluded(rm.ModuleName) {
381+
continue
382+
}
380383
if !yield(rm) {
381384
return
382385
}
@@ -412,6 +415,9 @@ func (ctx *LintContext) ModuleRoles() iter.Seq[ModuleRoleInfo] {
412415
if err := rows.Scan(&mr.Name, &mr.ModuleName); err != nil {
413416
continue
414417
}
418+
if ctx.IsExcluded(mr.ModuleName) {
419+
continue
420+
}
415421
if !yield(mr) {
416422
return
417423
}

mdl/linter/linter.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ type Rule interface {
8484
Check(ctx *LintContext) []Violation
8585
}
8686

87+
// Configurable is an optional interface for rules that accept options from the lint config file.
88+
// Rules implementing this interface receive their options map before Check is called.
89+
type Configurable interface {
90+
Configure(options map[string]any)
91+
}
92+
8793
// RuleConfig holds configuration for a specific rule.
8894
type RuleConfig struct {
8995
Enabled bool
@@ -149,6 +155,15 @@ func (l *Linter) Run(ctx context.Context) ([]Violation, error) {
149155
default:
150156
}
151157

158+
// Pass options to rules that support configuration.
159+
if config, ok := l.configs[rule.ID()]; ok {
160+
if len(config.Options) > 0 {
161+
if c, ok := rule.(Configurable); ok {
162+
c.Configure(config.Options)
163+
}
164+
}
165+
}
166+
152167
// Run the rule
153168
violations := rule.Check(l.ctx)
154169

mdl/linter/linter_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ func (r *stubRule) Check(_ *LintContext) []Violation {
2121
return []Violation{{RuleID: r.id, Severity: SeverityWarning, Message: "hit"}}
2222
}
2323

24+
// configurableRule is a stubRule that also implements Configurable.
25+
type configurableRule struct {
26+
stubRule
27+
configuredOptions map[string]any
28+
}
29+
30+
func (r *configurableRule) Configure(options map[string]any) {
31+
r.configuredOptions = options
32+
}
33+
2434
func TestRuleFilter_AllowlistRestrictsExecution(t *testing.T) {
2535
lint := New(nil)
2636
lint.AddRule(&stubRule{"MPR001"})
@@ -62,6 +72,43 @@ func TestRuleFilter_EmptyAllowlistRunsAll(t *testing.T) {
6272
}
6373
}
6474

75+
func TestConfigurable_OptionsDeliveredBeforeCheck(t *testing.T) {
76+
rule := &configurableRule{stubRule: stubRule{"MPR003"}}
77+
lint := New(nil)
78+
lint.AddRule(rule)
79+
lint.ConfigureRule("MPR003", RuleConfig{
80+
Enabled: true,
81+
Severity: SeverityWarning,
82+
Options: map[string]any{"max_entities": 20},
83+
})
84+
85+
_, err := lint.Run(context.Background())
86+
if err != nil {
87+
t.Fatalf("Run: %v", err)
88+
}
89+
if rule.configuredOptions == nil {
90+
t.Fatal("Configure was never called")
91+
}
92+
if rule.configuredOptions["max_entities"] != 20 {
93+
t.Errorf("max_entities = %v, want 20", rule.configuredOptions["max_entities"])
94+
}
95+
}
96+
97+
func TestConfigurable_NotCalledWhenNoOptions(t *testing.T) {
98+
rule := &configurableRule{stubRule: stubRule{"MPR003"}}
99+
lint := New(nil)
100+
lint.AddRule(rule)
101+
// Config exists but Options is empty — Configure should not be called.
102+
lint.ConfigureRule("MPR003", RuleConfig{Enabled: true})
103+
104+
if _, err := lint.Run(context.Background()); err != nil {
105+
t.Fatalf("Run: %v", err)
106+
}
107+
if rule.configuredOptions != nil {
108+
t.Errorf("Configure should not be called when options are empty")
109+
}
110+
}
111+
65112
func TestRuleFilter_MultipleRulesAllowed(t *testing.T) {
66113
lint := New(nil)
67114
lint.AddRule(&stubRule{"MPR001"})

mdl/linter/rules/domain_size.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@ func (r *DomainModelSizeRule) Description() string {
3232
return fmt.Sprintf("Checks that modules have no more than %d persistent entities", r.MaxPersistentEntities)
3333
}
3434

35+
// Configure applies options from the lint config file.
36+
// Supported option: max_entities (int) — override DefaultMaxPersistentEntities.
37+
func (r *DomainModelSizeRule) Configure(options map[string]any) {
38+
if v, ok := options["max_entities"]; ok {
39+
switch n := v.(type) {
40+
case int:
41+
r.MaxPersistentEntities = n
42+
case float64:
43+
r.MaxPersistentEntities = int(n)
44+
}
45+
}
46+
}
47+
3548
// Check counts persistent entities per module and flags those exceeding the limit.
3649
func (r *DomainModelSizeRule) Check(ctx *linter.LintContext) []linter.Violation {
3750
// Count persistent entities per module

0 commit comments

Comments
 (0)