From 506fb1c9b88630a7f50804d94ef2080afe7b81c4 Mon Sep 17 00:00:00 2001 From: Andries Smit Date: Fri, 26 Jun 2026 11:37:42 +0200 Subject: [PATCH] fix(lint): respect lint-config.yaml in mxcli lint CLI and options in Starlark rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01R1yYeYn9rosf8UdAimdatt --- cmd/mxcli/cmd_lint.go | 13 ++ mdl/linter/config_test.go | 208 +++++++++++++++++++++++++++ mdl/linter/context.go | 6 + mdl/linter/linter.go | 15 ++ mdl/linter/linter_test.go | 47 ++++++ mdl/linter/rules/domain_size.go | 13 ++ mdl/linter/rules/domain_size_test.go | 48 +++++++ mdl/linter/starlark.go | 44 ++++++ 8 files changed, 394 insertions(+) create mode 100644 mdl/linter/config_test.go diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go index 89aa594ea..98489468b 100644 --- a/cmd/mxcli/cmd_lint.go +++ b/cmd/mxcli/cmd_lint.go @@ -166,6 +166,19 @@ Examples: } } + // Load lint config file and apply (excludedModules, rule severity/enabled overrides). + // Config ExcludeModules merges with --exclude flag values. + configPath := linter.FindConfigFile(projectDir) + if cfg, err := linter.LoadConfig(configPath); err == nil { + if len(cfg.ExcludeModules) > 0 { + merged := append(excludeModules, cfg.ExcludeModules...) + ctx.SetExcludedModules(merged) + } + cfg.ApplyConfig(lint) + } else { + fmt.Fprintf(os.Stderr, "Warning: failed to load lint config: %v\n", err) + } + // If --rules is specified, disable every rule not in the allowlist. if len(onlyRules) > 0 { allowed := make(map[string]bool, len(onlyRules)) diff --git a/mdl/linter/config_test.go b/mdl/linter/config_test.go new file mode 100644 index 000000000..4ac8f54b2 --- /dev/null +++ b/mdl/linter/config_test.go @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter + +import ( + "os" + "path/filepath" + "testing" +) + +func writeYAML(t *testing.T, content string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "lint-config.yaml") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write YAML: %v", err) + } + return path +} + +func TestLoadConfig_ExcludedModules(t *testing.T) { + path := writeYAML(t, ` +excludeModules: + - Administration + - Anonymous +`) + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if len(cfg.ExcludeModules) != 2 { + t.Fatalf("want 2 excluded modules, got %d", len(cfg.ExcludeModules)) + } + if cfg.ExcludeModules[0] != "Administration" || cfg.ExcludeModules[1] != "Anonymous" { + t.Errorf("unexpected modules: %v", cfg.ExcludeModules) + } +} + +func TestLoadConfig_RuleEnabled(t *testing.T) { + path := writeYAML(t, ` +rules: + MPR001: + enabled: false +`) + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + rule, ok := cfg.Rules["MPR001"] + if !ok { + t.Fatal("MPR001 rule not found") + } + if rule.Enabled == nil || *rule.Enabled != false { + t.Errorf("expected enabled=false, got %v", rule.Enabled) + } +} + +func TestLoadConfig_RuleSeverity(t *testing.T) { + path := writeYAML(t, ` +rules: + PH009: + severity: hint +`) + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if cfg.Rules["PH009"].Severity != "hint" { + t.Errorf("expected severity hint, got %q", cfg.Rules["PH009"].Severity) + } +} + +func TestLoadConfig_RuleOptions(t *testing.T) { + path := writeYAML(t, ` +rules: + MPR003: + options: + max_entities: 20 +`) + cfg, err := LoadConfig(path) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + opts := cfg.Rules["MPR003"].Options + if opts == nil { + t.Fatal("expected options map, got nil") + } + // YAML numbers unmarshal as int when they fit. + v, ok := opts["max_entities"] + if !ok { + t.Fatal("max_entities not found in options") + } + switch n := v.(type) { + case int: + if n != 20 { + t.Errorf("max_entities = %d, want 20", n) + } + case float64: + if n != 20 { + t.Errorf("max_entities = %v, want 20", n) + } + default: + t.Errorf("unexpected type %T for max_entities", v) + } +} + +func TestLoadConfig_MissingFileReturnsDefault(t *testing.T) { + cfg, err := LoadConfig("/nonexistent/lint-config.yaml") + if err != nil { + t.Fatalf("expected no error for missing file, got %v", err) + } + if len(cfg.ExcludeModules) != 0 || len(cfg.Rules) != 0 { + t.Errorf("expected empty default config, got %+v", cfg) + } +} + +func TestApplyConfig_DisablesRule(t *testing.T) { + path := writeYAML(t, ` +rules: + MPR001: + enabled: false +`) + cfg, _ := LoadConfig(path) + + lint := New(nil) + lint.AddRule(&stubRule{"MPR001"}) + lint.AddRule(&stubRule{"MPR002"}) + cfg.ApplyConfig(lint) + + configs := lint.configs + if c, ok := configs["MPR001"]; !ok || c.Enabled { + t.Errorf("MPR001 should be disabled, got %+v", c) + } + if c, ok := configs["MPR002"]; ok && !c.Enabled { + t.Errorf("MPR002 should not be disabled, got %+v", c) + } +} + +func TestApplyConfig_OverridesSeverity(t *testing.T) { + path := writeYAML(t, ` +rules: + MPR001: + severity: error +`) + cfg, _ := LoadConfig(path) + + lint := New(nil) + lint.AddRule(&stubRule{"MPR001"}) + cfg.ApplyConfig(lint) + + if c := lint.configs["MPR001"]; c.Severity != SeverityError { + t.Errorf("expected SeverityError, got %v", c.Severity) + } +} + +func TestApplyConfig_PassesOptions(t *testing.T) { + path := writeYAML(t, ` +rules: + MPR003: + options: + max_entities: 25 +`) + cfg, _ := LoadConfig(path) + + lint := New(nil) + lint.AddRule(&stubRule{"MPR003"}) + cfg.ApplyConfig(lint) + + opts := lint.configs["MPR003"].Options + if opts == nil { + t.Fatal("expected options in RuleConfig, got nil") + } +} + +func TestFindConfigFile_LocatesRootFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "lint-config.yaml") + if err := os.WriteFile(path, []byte(""), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + got := FindConfigFile(dir) + if got != path { + t.Errorf("FindConfigFile = %q, want %q", got, path) + } +} + +func TestFindConfigFile_LocatesDotClaudeFile(t *testing.T) { + dir := t.TempDir() + sub := filepath.Join(dir, ".claude") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + path := filepath.Join(sub, "lint-config.yaml") + if err := os.WriteFile(path, []byte(""), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + got := FindConfigFile(dir) + if got != path { + t.Errorf("FindConfigFile = %q, want %q", got, path) + } +} + +func TestFindConfigFile_ReturnsEmptyWhenAbsent(t *testing.T) { + dir := t.TempDir() + if got := FindConfigFile(dir); got != "" { + t.Errorf("expected empty string, got %q", got) + } +} diff --git a/mdl/linter/context.go b/mdl/linter/context.go index 42f65a1a9..f621043c3 100644 --- a/mdl/linter/context.go +++ b/mdl/linter/context.go @@ -377,6 +377,9 @@ func (ctx *LintContext) RoleMappings() iter.Seq[RoleMappingInfo] { if err := rows.Scan(&rm.UserRoleName, &rm.ModuleRoleName, &rm.ModuleName); err != nil { continue } + if ctx.IsExcluded(rm.ModuleName) { + continue + } if !yield(rm) { return } @@ -412,6 +415,9 @@ func (ctx *LintContext) ModuleRoles() iter.Seq[ModuleRoleInfo] { if err := rows.Scan(&mr.Name, &mr.ModuleName); err != nil { continue } + if ctx.IsExcluded(mr.ModuleName) { + continue + } if !yield(mr) { return } diff --git a/mdl/linter/linter.go b/mdl/linter/linter.go index a029664cf..6faffbedc 100644 --- a/mdl/linter/linter.go +++ b/mdl/linter/linter.go @@ -84,6 +84,12 @@ type Rule interface { Check(ctx *LintContext) []Violation } +// Configurable is an optional interface for rules that accept options from the lint config file. +// Rules implementing this interface receive their options map before Check is called. +type Configurable interface { + Configure(options map[string]any) +} + // RuleConfig holds configuration for a specific rule. type RuleConfig struct { Enabled bool @@ -149,6 +155,15 @@ func (l *Linter) Run(ctx context.Context) ([]Violation, error) { default: } + // Pass options to rules that support configuration. + if config, ok := l.configs[rule.ID()]; ok { + if len(config.Options) > 0 { + if c, ok := rule.(Configurable); ok { + c.Configure(config.Options) + } + } + } + // Run the rule violations := rule.Check(l.ctx) diff --git a/mdl/linter/linter_test.go b/mdl/linter/linter_test.go index f2bb5765d..f023e8a18 100644 --- a/mdl/linter/linter_test.go +++ b/mdl/linter/linter_test.go @@ -21,6 +21,16 @@ func (r *stubRule) Check(_ *LintContext) []Violation { return []Violation{{RuleID: r.id, Severity: SeverityWarning, Message: "hit"}} } +// configurableRule is a stubRule that also implements Configurable. +type configurableRule struct { + stubRule + configuredOptions map[string]any +} + +func (r *configurableRule) Configure(options map[string]any) { + r.configuredOptions = options +} + func TestRuleFilter_AllowlistRestrictsExecution(t *testing.T) { lint := New(nil) lint.AddRule(&stubRule{"MPR001"}) @@ -62,6 +72,43 @@ func TestRuleFilter_EmptyAllowlistRunsAll(t *testing.T) { } } +func TestConfigurable_OptionsDeliveredBeforeCheck(t *testing.T) { + rule := &configurableRule{stubRule: stubRule{"MPR003"}} + lint := New(nil) + lint.AddRule(rule) + lint.ConfigureRule("MPR003", RuleConfig{ + Enabled: true, + Severity: SeverityWarning, + Options: map[string]any{"max_entities": 20}, + }) + + _, err := lint.Run(context.Background()) + if err != nil { + t.Fatalf("Run: %v", err) + } + if rule.configuredOptions == nil { + t.Fatal("Configure was never called") + } + if rule.configuredOptions["max_entities"] != 20 { + t.Errorf("max_entities = %v, want 20", rule.configuredOptions["max_entities"]) + } +} + +func TestConfigurable_NotCalledWhenNoOptions(t *testing.T) { + rule := &configurableRule{stubRule: stubRule{"MPR003"}} + lint := New(nil) + lint.AddRule(rule) + // Config exists but Options is empty — Configure should not be called. + lint.ConfigureRule("MPR003", RuleConfig{Enabled: true}) + + if _, err := lint.Run(context.Background()); err != nil { + t.Fatalf("Run: %v", err) + } + if rule.configuredOptions != nil { + t.Errorf("Configure should not be called when options are empty") + } +} + func TestRuleFilter_MultipleRulesAllowed(t *testing.T) { lint := New(nil) lint.AddRule(&stubRule{"MPR001"}) diff --git a/mdl/linter/rules/domain_size.go b/mdl/linter/rules/domain_size.go index 7349580d6..78081feea 100644 --- a/mdl/linter/rules/domain_size.go +++ b/mdl/linter/rules/domain_size.go @@ -32,6 +32,19 @@ func (r *DomainModelSizeRule) Description() string { return fmt.Sprintf("Checks that modules have no more than %d persistent entities", r.MaxPersistentEntities) } +// Configure applies options from the lint config file. +// Supported option: max_entities (int) — override DefaultMaxPersistentEntities. +func (r *DomainModelSizeRule) Configure(options map[string]any) { + if v, ok := options["max_entities"]; ok { + switch n := v.(type) { + case int: + r.MaxPersistentEntities = n + case float64: + r.MaxPersistentEntities = int(n) + } + } +} + // Check counts persistent entities per module and flags those exceeding the limit. func (r *DomainModelSizeRule) Check(ctx *linter.LintContext) []linter.Violation { // Count persistent entities per module diff --git a/mdl/linter/rules/domain_size_test.go b/mdl/linter/rules/domain_size_test.go index 8b4736e60..e21c6a429 100644 --- a/mdl/linter/rules/domain_size_test.go +++ b/mdl/linter/rules/domain_size_test.go @@ -123,6 +123,54 @@ func TestDomainModelSizeRule_OneOverThreshold(t *testing.T) { } } +func TestDomainModelSizeRule_Configure_MaxEntitiesInt(t *testing.T) { + rule := NewDomainModelSizeRule() + rule.Configure(map[string]any{"max_entities": 25}) + if rule.MaxPersistentEntities != 25 { + t.Errorf("MaxPersistentEntities = %d, want 25", rule.MaxPersistentEntities) + } +} + +func TestDomainModelSizeRule_Configure_MaxEntitiesFloat(t *testing.T) { + rule := NewDomainModelSizeRule() + // YAML may unmarshal integers as float64. + rule.Configure(map[string]any{"max_entities": float64(30)}) + if rule.MaxPersistentEntities != 30 { + t.Errorf("MaxPersistentEntities = %d, want 30", rule.MaxPersistentEntities) + } +} + +func TestDomainModelSizeRule_Configure_UnknownKeyIgnored(t *testing.T) { + rule := NewDomainModelSizeRule() + rule.Configure(map[string]any{"unknown_key": "value"}) + if rule.MaxPersistentEntities != DefaultMaxPersistentEntities { + t.Errorf("unexpected change: MaxPersistentEntities = %d", rule.MaxPersistentEntities) + } +} + +func TestDomainModelSizeRule_Configure_RaisesThreshold(t *testing.T) { + // With 20 entities and default threshold 15 there is a violation; + // raising threshold to 25 should eliminate it. + var entities [][]any + for i := 0; i < 20; i++ { + entities = append(entities, []any{ + fmt.Sprintf("id%d", i), fmt.Sprintf("Entity%d", i), + fmt.Sprintf("BigModule.Entity%d", i), "BigModule", "", + "PERSISTENT", "", "", 3, 1, 0, 0, 0, + }) + } + db := setupEntitiesDB(t, entities) + defer db.Close() + + ctx := linter.NewLintContextFromDB(db) + rule := NewDomainModelSizeRule() + rule.Configure(map[string]any{"max_entities": 25}) + violations := rule.Check(ctx) + if len(violations) != 0 { + t.Errorf("expected 0 violations with raised threshold, got %d", len(violations)) + } +} + func TestDomainModelSizeRule_Metadata(t *testing.T) { r := NewDomainModelSizeRule() if r.ID() != "MPR003" { diff --git a/mdl/linter/starlark.go b/mdl/linter/starlark.go index 2531587d3..37b36bf9e 100644 --- a/mdl/linter/starlark.go +++ b/mdl/linter/starlark.go @@ -25,6 +25,12 @@ type StarlarkRule struct { ctx *LintContext checkFn starlark.Callable globals starlark.StringDict + options map[string]any +} + +// Configure stores options from the lint config file so Starlark rules can read them via get_option(). +func (r *StarlarkRule) Configure(options map[string]any) { + r.options = options } // ID returns the rule ID. @@ -270,6 +276,9 @@ func (r *StarlarkRule) buildPredeclared() starlark.StringDict { // Struct constructor (from starlarkstruct) "struct": starlark.NewBuiltin("struct", starlarkstruct.Make), + + // Options access: get_option("key") or get_option("key", default) + "get_option": starlark.NewBuiltin("get_option", r.builtinGetOption), } } @@ -476,6 +485,41 @@ func (r *StarlarkRule) builtinScheduledEvents(_ *starlark.Thread, _ *starlark.Bu return starlark.NewList(result), nil } +// builtinGetOption returns a rule option value from the lint config file. +// Signature: get_option(key, default=None) +func (r *StarlarkRule) builtinGetOption(_ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) { + var key starlark.String + var defaultVal starlark.Value = starlark.None + if err := starlark.UnpackArgs("get_option", args, kwargs, + "key", &key, + "default?", &defaultVal, + ); err != nil { + return nil, err + } + if r.options != nil { + if v, ok := r.options[string(key)]; ok { + return goValueToStarlark(v), nil + } + } + return defaultVal, nil +} + +// goValueToStarlark converts a Go value (from YAML unmarshaling) to a Starlark value. +func goValueToStarlark(v any) starlark.Value { + switch val := v.(type) { + case bool: + return starlark.Bool(val) + case int: + return starlark.MakeInt(val) + case float64: + return starlark.Float(val) + case string: + return starlark.String(val) + default: + return starlark.None + } +} + // builtinActivitiesFor returns the activities for a given microflow. func (r *StarlarkRule) builtinActivitiesFor(_ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) { if r.ctx == nil {