From 4005e1150bada047f2f7fdb670ffcc17d75d142c Mon Sep 17 00:00:00 2001 From: Nim G Date: Tue, 1 Sep 2026 23:52:34 -0300 Subject: [PATCH] fix: a sequence rule with a negated scope double-reports every match sentenceScope's negation branch was a no-op for a bare negated term: `~list` narrowed to `~list`, itself, via strings.CutPrefix re-adding the same prefix it had just stripped. A negated term never mentions `sentence`, so asksForSentence (scope.go) then skipped every `sentence.*` fragment block for such a rule, and Scope.Matches instead matched both the whole-block copy and its own paragraph wrapper for the same text. One real match dispatched to Run twice, once per block, and produced two identical alerts. The negation branch now AND-s `sentence` in front of the term instead of leaving it untouched, so `~list` narrows to `sentence&~list`, sentences outside a list, the same as every other declared scope already does. --- internal/check/sequence.go | 14 ++++--- internal/check/sequence_test.go | 69 ++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/internal/check/sequence.go b/internal/check/sequence.go index 44faa2de..46fef37e 100644 --- a/internal/check/sequence.go +++ b/internal/check/sequence.go @@ -609,11 +609,15 @@ func sentenceScope(declared []string) []string { scopes = append(scopes, s) continue } - // Negation applies to the part being excluded, so it stays in front: - // `~list` narrows to sentences outside a list, not to something - // outside `sentence.list`. - if neg, found := strings.CutPrefix(s, "~"); found { - scopes = append(scopes, "~"+neg) + // A bare negated term names only what to exclude and never mentions + // `sentence` itself, so asksForSentence (scope.go) skips every + // `sentence.*` fragment block for it and the rule matched the whole + // unsegmented block instead -- narrowing to `~list` alone was a + // no-op. AND-ing `sentence` in front keeps the exclusion and still + // narrows: `sentence&~list` is "sentences outside a list", which is + // what the rule actually needs. + if strings.HasPrefix(s, "~") { + scopes = append(scopes, "sentence&"+s) continue } // `paragraph` names no block of its own. Splitting wraps every block diff --git a/internal/check/sequence_test.go b/internal/check/sequence_test.go index 6bd5ec12..bf2c22cb 100644 --- a/internal/check/sequence_test.go +++ b/internal/check/sequence_test.go @@ -71,7 +71,15 @@ func TestSentenceScope(t *testing.T) { {"a block scope is narrowed", []string{"list"}, []string{"sentence.list"}}, {"already a sentence scope", []string{"sentence"}, []string{"sentence"}}, {"already narrowed", []string{"sentence.list"}, []string{"sentence.list"}}, - {"negation stays in front", []string{"~list"}, []string{"~list"}}, + // A bare negated term never mentions `sentence`, so asksForSentence + // (scope.go) skipped every `sentence.*` fragment block for it and the + // rule matched the whole unsegmented block instead: `~list` alone left + // `s` unchanged. `sentence&~list` still excludes list items, but only + // within sentence-fragment blocks. + {"negation is AND-ed with sentence", + []string{"~list"}, []string{"sentence&~list"}}, + {"a chained negation is AND-ed the same way", + []string{"~list&text"}, []string{"sentence&~list&text"}}, {"several at once", []string{"heading", "list"}, []string{"sentence.heading", "sentence.list"}}, @@ -103,6 +111,65 @@ func TestSentenceScope(t *testing.T) { } } +// The real, dispatched consequence of the negation bug above: a plain rule +// with a bare negated scope matched the same real-world sentence through two +// different blocks at once. `~list` narrowed to nothing, so Scope.Matches +// treated the rule as if it had no scope at all and matched both +// `paragraph.text.md` and its own whole-block copy `text.md` -- the same +// underlying text, dispatched to Run twice, once for each block. One real +// match produced two identical alerts. +// +// Dispatched the way the real linter dispatches a `sequence` check: through +// the same block splitting (nlp.Info.Compute) and scope matching +// (Scope.Matches) it uses, instead of handing text to Run directly. A block +// built by hand and passed straight to Run bypasses that dispatch entirely, +// so it cannot see this bug at all. +func TestSequenceNegatedScopeDoesNotDoubleReport(t *testing.T) { + rule, err := NewSequence(testConfig(), baseCheck{ + "extends": "sequence", + "name": "Test.WidgetArrivedNegatedScope", + "level": "error", + "ignorecase": true, + "message": "matched", + "scope": []string{"~list"}, + "tokens": []interface{}{ + map[string]interface{}{"pattern": "widget"}, + map[string]interface{}{"pattern": "arrived", "skip": 1}, + }, + }, "Test.WidgetArrivedNegatedScope") + if err != nil { + t.Fatalf("building rule: %v", err) + } + + text := "I bought a widget that arrived promptly, said the courier." + + f := &core.File{NLP: nlp.Info{Segmentation: true, Splitting: true}} + paragraph := nlp.NewLinedBlock("", text, "text.md", 1) + + blocks, cerr := f.NLP.Compute(¶graph, true) + if cerr != nil { + t.Fatalf("computing blocks: %v", cerr) + } + + scope := NewScope(rule.Fields().Scope) + + var alerts []core.Alert + for _, blk := range blocks { + if !scope.Matches(blk) { + continue + } + got, rerr := rule.Run(blk, f, testConfig()) + if rerr != nil { + t.Fatalf("running rule: %v", rerr) + } + alerts = append(alerts, got...) + } + + if len(alerts) != 1 { + t.Errorf("produced %d alerts for one real match, want exactly 1", len(alerts)) + } +} + // The narrowed scope has to match a block that is actually built. A paragraph's // sentences arrive as `sentence.text.`, so a rule scoped to `paragraph` // must match that or it reports nothing at all.