diff --git a/internal/check/consistency.go b/internal/check/consistency.go index 69a430c4..85384c48 100644 --- a/internal/check/consistency.go +++ b/internal/check/consistency.go @@ -2,6 +2,8 @@ package check import ( "fmt" + "maps" + "slices" "strings" "github.com/mitchellh/mapstructure" @@ -57,7 +59,9 @@ func NewConsistency(cfg *core.Config, generic baseCheck, path string) (Consisten parts := strings.Split(name, ".") chkKey := parts[len(parts)-1] count := 0 - for v1, v2 := range rule.Either { + // In a fixed order, so the steps run the same way on every run. + for _, v1 := range slices.Sorted(maps.Keys(rule.Either)) { + v2 := rule.Either[v1] count += 2 subs := []string{ diff --git a/internal/core/file.go b/internal/core/file.go index 7c2d413f..679fc723 100755 --- a/internal/core/file.go +++ b/internal/core/file.go @@ -354,10 +354,11 @@ func (f *File) FindLoc(ctx, s string, pad, count int, a Alert, at int) (int, []i var length int var lines []string - given := len(ctx) + given := ctx for _, s := range a.Offset { ctx, _ = Substitute(ctx, s, '@') } + at = maskedOffset(given, ctx, at) pos, substring, hit := locateMatch(ctx, s, a, at) if pos < 0 { @@ -366,7 +367,7 @@ func (f *File) FindLoc(ctx, s string, pad, count int, a Alert, at int) (int, []i } if hit >= 0 { // The Offset masks above map rune to rune; the tail keeps its place. - hit += given - len(ctx) + hit += len(given) - len(ctx) } loc := a.Span @@ -580,7 +581,10 @@ func (f *File) AddAlert(a Alert, blk nlp.Block, lines, pad int, lookup bool) { a.Line, a.Span = f.assignLoc(ctx, blk, pad, a) } if (!lookup && a.Span[0] < 0) || lookup { - a.Line, a.Span, hit = f.FindLoc(ctx, blk.Text, pad, lines, a, blk.Offset) + // An earlier alert's mask may have shortened ctx, so the block's + // offset into its context is carried over to the masked copy. + at := maskedOffset(blk.Context, ctx, blk.Offset) + a.Line, a.Span, hit = f.FindLoc(ctx, blk.Text, pad, lines, a, at) } } diff --git a/internal/core/location.go b/internal/core/location.go index dc9e0400..ec82fbed 100644 --- a/internal/core/location.go +++ b/internal/core/location.go @@ -243,20 +243,40 @@ func directPosition(ctx string, idx, from int, sub string) bool { return !strings.Contains(ctx[from:idx], sub) } -// throughMask reports whether ctx begins with txt, a masked byte in ctx -// standing for any byte of txt. +// throughMask reports whether ctx begins with txt, a masked rune in ctx +// standing for any rune of txt. Rune by rune: a mask keeps the rune count, +// not the byte count, since a multi-byte rune is masked as a single `#`. func throughMask(ctx, txt string) bool { - if len(ctx) < len(txt) { - return false - } - for i := 0; i < len(txt); i++ { - if c := ctx[i]; c != txt[i] && c != '#' && c != '@' { + for _, want := range txt { + if ctx == "" { + return false + } + got, n := utf8.DecodeRuneInString(ctx) + if got != want && got != '#' && got != '@' { return false } + ctx = ctx[n:] } return true } +// maskedOffset maps the byte offset at in ctx to the same rune in masked, a +// copy of ctx that masking has shortened. The runes still line up, so the +// offset is carried over by rune count. +func maskedOffset(ctx, masked string, at int) int { + if at <= 0 || at > len(ctx) || len(ctx) == len(masked) { + return at + } + runes := utf8.RuneCountInString(ctx[:at]) + for i := range masked { + if runes == 0 { + return i + } + runes-- + } + return len(masked) +} + // located is positionOf with the match's byte index in the unmasked context. func located(ctx string, idx int, sub string, shift int) (int, string, int) { if strings.HasPrefix(ctx[idx:], "_") { diff --git a/internal/core/location_test.go b/internal/core/location_test.go index f360c299..66c5f6a7 100644 --- a/internal/core/location_test.go +++ b/internal/core/location_test.go @@ -377,3 +377,39 @@ func TestLocateMatchThroughMask(t *testing.T) { t.Errorf("got pos %d hit %d, want 19 and 18", pos, hit) } } + +// A mask keeps the rune count and not the byte count, so a block whose +// earlier match was a multi-byte rune must still be found at its offset, and +// the offset itself has to be carried over to the shortened context. See #1184. +func TestLocateMatchThroughMultiByteMask(t *testing.T) { + ctx := "\"'’\n\n”’" + txt := "”’" + + masked := maskMatch(ctx, "”", 7) + if masked != "\"'’\n\n#’" { + t.Fatalf("maskMatch = %q", masked) + } + if !throughMask(masked[7:], txt) { + t.Errorf("throughMask rejected %q against %q", masked[7:], txt) + } + if throughMask("#x", "”y") { + t.Error("throughMask accepted a differing rune after a mask") + } + + at := maskedOffset(ctx, masked, 7) + if at != 7 { + t.Errorf("maskedOffset = %d, want 7", at) + } + if got := maskedOffset(ctx, masked, 13); got != 11 { + t.Errorf("maskedOffset at the end = %d, want 11", got) + } + if got := maskedOffset(ctx, ctx, 7); got != 7 { + t.Errorf("maskedOffset on an unmasked copy = %d, want 7", got) + } + + // The second `’` is the one in the block, not the one on line 1. + pos, _, hit := locateMatch(masked, txt, Alert{Match: "’", Span: []int{3, 6}}, at) + if pos != 7 || hit != 8 { + t.Errorf("got pos %d hit %d, want 7 and 8", pos, hit) + } +} diff --git a/testdata/e2e/checks.yaml b/testdata/e2e/checks.yaml index dd0d4cf2..f0b66429 100644 --- a/testdata/e2e/checks.yaml +++ b/testdata/e2e/checks.yaml @@ -297,6 +297,19 @@ cases: test2.md:9:1:demo.CapSub:Use 'Change to the `/etc/X11` directory' instead of 'Change into the `/etc/X11` directory'. test2.md:11:6:Bugs.WrongExp:Use 'versus' instead of 'vs '. + - name: consistency + about: "#1184" + dir: Consistency + args: . + exit: 1 + want: | + input.txt:1:3:Test.Quotes:Inconsistent quote style '”' + input.txt:3:1:Test.Quotes:Inconsistent quote style '”' + input.txt:3:2:Test.Quotes:Inconsistent quote style '’' + swapped.txt:1:3:Test.Quotes:Inconsistent quote style '’' + swapped.txt:3:1:Test.Quotes:Inconsistent quote style '”' + swapped.txt:3:2:Test.Quotes:Inconsistent quote style '’' + - name: style-level dir: StyleLevel args: . diff --git a/testdata/fixtures/checks/Consistency/.vale.ini b/testdata/fixtures/checks/Consistency/.vale.ini new file mode 100644 index 00000000..bc0b1591 --- /dev/null +++ b/testdata/fixtures/checks/Consistency/.vale.ini @@ -0,0 +1,4 @@ +StylesPath = styles + +[*.txt] +BasedOnStyles = Test diff --git a/testdata/fixtures/checks/Consistency/input.txt b/testdata/fixtures/checks/Consistency/input.txt new file mode 100644 index 00000000..bb3920b7 --- /dev/null +++ b/testdata/fixtures/checks/Consistency/input.txt @@ -0,0 +1,3 @@ +"'” + +”’ \ No newline at end of file diff --git a/testdata/fixtures/checks/Consistency/styles/Test/Para.yml b/testdata/fixtures/checks/Consistency/styles/Test/Para.yml new file mode 100644 index 00000000..fd4f1f0f --- /dev/null +++ b/testdata/fixtures/checks/Consistency/styles/Test/Para.yml @@ -0,0 +1,6 @@ +extends: existence +message: "never fires" +level: error +scope: paragraph +tokens: + - zzzqqq diff --git a/testdata/fixtures/checks/Consistency/styles/Test/Quotes.yml b/testdata/fixtures/checks/Consistency/styles/Test/Quotes.yml new file mode 100644 index 00000000..1db35452 --- /dev/null +++ b/testdata/fixtures/checks/Consistency/styles/Test/Quotes.yml @@ -0,0 +1,7 @@ +extends: consistency +message: "Inconsistent quote style '%s'" +level: error +nonword: true +either: + '"': '”' + "'": '’' diff --git a/testdata/fixtures/checks/Consistency/swapped.txt b/testdata/fixtures/checks/Consistency/swapped.txt new file mode 100644 index 00000000..50a0a9d6 --- /dev/null +++ b/testdata/fixtures/checks/Consistency/swapped.txt @@ -0,0 +1,3 @@ +"'’ + +”’ \ No newline at end of file