diff --git a/linter/linter.go b/linter/linter.go index 0b89e98..a30aa63 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -54,10 +54,13 @@ func (l *Linter) HasRules(t FileType) bool { } // LintDocument applies the linter's rules to doc, a configuration file of the given type, and -// returns the findings sorted by position. path is used only for reporting, and dir describes the -// directory containing the file; both are handed to the rules as [Context.Path] and [Context.Dir], -// and dir may be left zero for an in-memory document. It reads the document as given; any mutation -// of its tree (see Document.Tree) must happen before calling it. +// returns the findings sorted by position, then by rule ID and message. path is used only for +// reporting, and dir describes the directory containing the file; both are handed to the rules as +// [Context.Path] and [Context.Dir], and dir may be left zero for an in-memory document. It reads the +// document as given; any mutation of its tree (see Document.Tree) must happen before calling it. +// +// Issues that match in every field are reported once, so a rule that reaches the same problem by +// more than one route does not report it twice. func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir Dir) []Issue { patterns := l.patterns[fileType] if len(patterns) == 0 { @@ -65,6 +68,7 @@ func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir } rctx := &Context{Path: path, Type: fileType, Root: doc.tree, Dir: dir} var issues []Issue + seen := map[Issue]struct{}{} walk(doc.tree, "", nil, patterns, func(r *Rule, node *Node) { id := r.ID severity := l.severities[id] @@ -73,16 +77,23 @@ func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir if doc.ignores.ignores(line, id) { continue } - issues = append(issues, Issue{ + issue := Issue{ Path: path, Line: line, Col: col, RuleID: id, Message: f.Message, Severity: severity, - }) + } + if _, dup := seen[issue]; dup { + continue + } + seen[issue] = struct{}{} + issues = append(issues, issue) } }) + // Deduplication leaves position, rule ID and message a unique key, so this order is total: the + // same document always yields the same sequence. sort.Slice(issues, func(i, j int) bool { a, b := issues[i], issues[j] if a.Line != b.Line { @@ -91,7 +102,10 @@ func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir if a.Col != b.Col { return a.Col < b.Col } - return a.RuleID < b.RuleID + if a.RuleID != b.RuleID { + return a.RuleID < b.RuleID + } + return a.Message < b.Message }) return issues } diff --git a/linter/linter_test.go b/linter/linter_test.go index 7647a57..3fb9d38 100644 --- a/linter/linter_test.go +++ b/linter/linter_test.go @@ -229,6 +229,113 @@ func TestLintDocument_SortsByRuleIDAtSamePosition(t *testing.T) { } } +func TestLintDocument_SortsByMessageAtSamePosition(t *testing.T) { + t.Parallel() + + // One rule reporting several missing properties at the document root emits findings that share a + // position and a rule ID, so only the message can order them. They are emitted in reverse so the + // assertion fails unless the message decides. + src := `{}` + want := []string{`"id" is required`, `"name" is required`, `"version" is required`} + rule := &Rule{ + ID: "missing-required-props", + FileTypes: []FileType{Devcontainer}, + Paths: []string{""}, + Check: func(_ *Context, node *Node) []Finding { + return []Finding{ + {Message: `"version" is required`, Offset: node.Value.StartOffset}, + {Message: `"name" is required`, Offset: node.Value.StartOffset}, + {Message: `"id" is required`, Offset: node.Value.StartOffset}, + } + }, + } + l := New() + l.RegisterRule(rule, SeverityWarn) + + got := messagesOf(lintSource(t, l, "devcontainer.json", Devcontainer, src)) + if !slices.Equal(got, want) { + t.Errorf("issue order = %v, want %v (sorted by Message)", got, want) + } +} + +func TestLintDocument_Deduplicates(t *testing.T) { + t.Parallel() + + // "aa" and "bb" share a line at different columns; "cc" is on a later line, so cases can differ + // in column alone or in line alone. + src := "{\n \"x\": \"aa bb\",\n \"y\": \"cc\"\n}" + offAA := strings.Index(src, "aa") + offBB := strings.Index(src, "bb") + offCC := strings.Index(src, "cc") + + type finding struct { + ruleID string + message string + offset int + } + for _, tt := range []struct { + name string + findings []finding + want int + }{ + { + name: "identical findings collapse", + findings: []finding{{"dup-rule", "same", offAA}, {"dup-rule", "same", offAA}}, + want: 1, + }, + { + name: "differing column kept", + findings: []finding{{"dup-rule", "same", offAA}, {"dup-rule", "same", offBB}}, + want: 2, + }, + { + name: "differing line kept", + findings: []finding{{"dup-rule", "same", offAA}, {"dup-rule", "same", offCC}}, + want: 2, + }, + { + name: "differing message kept", + findings: []finding{{"dup-rule", "one", offAA}, {"dup-rule", "two", offAA}}, + want: 2, + }, + { + name: "differing rule ID kept", + findings: []finding{{"one-rule", "same", offAA}, {"two-rule", "same", offAA}}, + want: 2, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Group the case's findings by rule ID and register one rule per group, so that a case + // differing only in rule ID reports the same finding from two rules. + byRule := map[string][]Finding{} + var ids []string + for _, f := range tt.findings { + if _, ok := byRule[f.ruleID]; !ok { + ids = append(ids, f.ruleID) + } + byRule[f.ruleID] = append(byRule[f.ruleID], Finding{Message: f.message, Offset: f.offset}) + } + l := New() + for _, id := range ids { + l.RegisterRule(&Rule{ + ID: id, + FileTypes: []FileType{Devcontainer}, + Paths: []string{""}, + Check: func(*Context, *Node) []Finding { + return byRule[id] + }, + }, SeverityWarn) + } + + if got := lintSource(t, l, "devcontainer.json", Devcontainer, src); len(got) != tt.want { + t.Errorf("got %d issues %v, want %d", len(got), got, tt.want) + } + }) + } +} + func TestIssueString(t *testing.T) { t.Parallel()