fix: place a match inside a block that inline markup rewrote

Fixes #502

Signed-off-by: Joseph Kato <joseph@jdkato.io>

Author: jdkato

internal/check/anchor.go

@@ -23,7 +23,28 @@ import (
 // so their offsets do not address the original document; checking that the
 // text really sits where the offset claims is what keeps those out.
 func anchor(a *core.Alert, blk nlp.Block) {
-	if blk.Offset < 0 || len(a.Span) != 2 {
+	if len(a.Span) != 2 {
+		return
+	}
+
+	lo, hi, ok := runeSpanToBytes(blk.Text, a.Span[0], a.Span[1])
+	if !ok {
+		return
+	}
+
+	if blk.Offset < 0 {
+		// A block that inline markup has rewritten -- `has <b>has</b>` read as
+		// `has has` -- is nowhere in the document to be found, but each of its
+		// runs was placed as it was read, and that is enough to say where the
+		// match is. The span reaches from the first byte to the last, markup
+		// between them included, because that is its extent in the file. See
+		// #502.
+		from, to := blk.SourceOffset(lo), blk.SourceOffset(hi-1)
+		if from < 0 || to < from {
+			return
+		}
+		a.Span = []int{from, to + 1}
+		a.HasByteOffsets = true
 		return
 	}
 
@@ -36,11 +57,6 @@ func anchor(a *core.Alert, blk nlp.Block) {
 		return
 	}
 
-	lo, hi, ok := runeSpanToBytes(blk.Text, a.Span[0], a.Span[1])
-	if !ok {
-		return
-	}
-
 	a.Span = []int{blk.Offset + lo, blk.Offset + hi}
 	a.HasByteOffsets = true
 }

internal/check/anchor_test.go

@@ -3,6 +3,9 @@ package check
 import (
 	"testing"
 	"unicode/utf8"
+
+	"github.com/errata-ai/vale/v3/internal/core"
+	"github.com/errata-ai/vale/v3/internal/nlp"
 )
 
 // runeSpanToBytes underpins every anchored alert; an off-by-one here
@@ -117,3 +120,51 @@ func TestRe2LocMatchesReference(t *testing.T) {
 		}
 	}
 }
+
+// A block whose text inline markup rewrote has no offset of its own, and its
+// alerts used to fall back to a text search that placed them on the wrong line
+// -- or dropped them. Its runs place them exactly. See #502.
+func TestAnchorFromRuns(t *testing.T) {
+	// "  <p>ab <b>cd</b> e</p>" read as "ab cd e".
+	blk := nlp.Block{
+		Context: "  <p>ab <b>cd</b> e</p>",
+		Text:    "ab cd e",
+		Offset:  -1,
+		Runs: []nlp.Run{
+			{At: 0, Src: 5, N: 3},  // "ab "
+			{At: 3, Src: 11, N: 2}, // "cd"
+			{At: 5, Src: 17, N: 2}, // " e"
+		},
+	}
+
+	tests := []struct {
+		name     string
+		span     []int
+		want     []int
+		anchored bool
+	}{
+		// "ab cd" reaches from the first byte to the last, `<b>` included.
+		{"across the markup", []int{0, 5}, []int{5, 13}, true},
+		{"before it", []int{0, 2}, []int{5, 7}, true},
+		{"after it", []int{3, 5}, []int{11, 13}, true},
+		{"past the last run", []int{0, 30}, nil, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			a := core.Alert{Span: tt.span}
+
+			anchor(&a, blk)
+
+			if a.HasByteOffsets != tt.anchored {
+				t.Fatalf("HasByteOffsets = %v, want %v", a.HasByteOffsets, tt.anchored)
+			}
+			if !tt.anchored {
+				return
+			}
+			if a.Span[0] != tt.want[0] || a.Span[1] != tt.want[1] {
+				t.Errorf("Span = %v, want %v", a.Span, tt.want)
+			}
+		})
+	}
+}

internal/lint/ast.go

@@ -214,17 +214,16 @@ func (l *Linter) lintHTMLTokens(f *core.File, raw []byte, offset int) error { //
 					txt = txt[1:]
 				}
 				// Record where this run came from before it loses its identity
-				// in the block -- but only when something is going to ask. The
-				// mapping exists to place inline fragments, so a style that
-				// scopes none of them should not pay for it.
+				// in the block. It places inline fragments, and it places a
+				// match in a block that inline markup has made unfindable in
+				// the source -- `has <b>has</b>` arrives as `has has`, which is
+				// nowhere in the file. See #502.
 				//
 				// Only a run that survived extraction unchanged can be mapped;
 				// `clean` may have prefixed a space, which belongs to the block
 				// and not to the source.
-				if len(wanted) > 0 {
-					if body := strings.TrimLeft(txt, " "); body == raw {
-						walker.mapRun(buf.Len()+(len(txt)-len(body)), raw)
-					}
+				if body := strings.TrimLeft(txt, " "); body == raw {
+					walker.mapRun(buf.Len()+(len(txt)-len(body)), raw)
 				}
 				buf.WriteString(txt)
 				// Feed the same text to any inline element still open, so a
@@ -283,7 +282,7 @@ func (l *Linter) lintScope(f *core.File, state *walker, txt string) error {
 			txt = strings.TrimLeft(txt, " ")
 			shift -= len(txt)
 
-			b := state.block(txt, withClasses(scope, state)+f.MetaScope+f.RealExt)
+			b := state.block(txt, withClasses(scope, state)+f.MetaScope+f.RealExt, shift)
 
 			// Prose, not just a block: a list item or a heading is made of
 			// sentences the same way a paragraph is, and only this path segments
@@ -317,7 +316,7 @@ func (l *Linter) lintScope(f *core.File, state *walker, txt string) error {
 	// `paragraphs` means what the `paragraph` scope reaches -- this branch.
 	f.Metrics["paragraphs"]++
 
-	b := state.block(txt, withClasses("text", state)+f.MetaScope+f.RealExt)
+	b := state.block(txt, withClasses("text", state)+f.MetaScope+f.RealExt, 0)
 	if err := l.lintProse(f, b, state.lines, true); err != nil {
 		return err
 	}
@@ -418,7 +417,7 @@ func (l *Linter) lintTags(f *core.File, state *walker, tok html.Token) error {
 			if a.Key == "alt" && !ignored {
 				err := l.lintBlock(
 					f,
-					state.block(a.Val, "text.attr."+a.Key), state.lines, 0, false)
+					state.block(a.Val, "text.attr."+a.Key, 0), state.lines, 0, false)
 				if err != nil {
 					return err
 				}

internal/lint/walk.go

@@ -13,18 +13,6 @@ import (
 	"github.com/errata-ai/vale/v3/internal/nlp"
 )
 
-// srcSpan maps a run of a block's stripped text back to where it came from in
-// the source.
-//
-// Extraction removes markup, so a block's text is not a substring of the source
-// and an index into one is not an index into the other. Recording each run as
-// it is read keeps the correspondence that would otherwise be lost.
-type srcSpan struct {
-	text int // where the run begins in the block's text
-	src  int // where it begins in the source
-	n    int // its length, the same in both
-}
-
 // inlineCapture is the text of one inline element, gathered as the block that
 // contains it is read.
 type inlineCapture struct {
@@ -80,7 +68,7 @@ type walker struct {
 	// spans maps this block's text back to the source, and srcCursor is how far
 	// the search for the next run has already gone. Separate from `cursor`,
 	// which places whole blocks and must not be moved by this.
-	spans     []srcSpan
+	spans     []nlp.Run
 	srcCursor int
 
 	// inline holds the inline elements captured within the current block --
@@ -266,7 +254,9 @@ func (w *walker) close() {
 	w.end = 0
 }
 
-func (w *walker) block(text, scope string) nlp.Block {
+// block builds the block for text, which begins `shift` bytes into the runs
+// recorded as it was read -- the space `clean` prefixes and lintScope trims.
+func (w *walker) block(text, scope string, shift int) nlp.Block {
 	line := w.idx
 
 	pos := w.advance(text)
@@ -277,9 +267,30 @@ func (w *walker) block(text, scope string) nlp.Block {
 	b := nlp.NewLinedBlock(w.getCtx(), text, scope, line)
 	b.Offset = w.locate(text)
 
+	// A block whose text is in the source verbatim needs nothing more; one
+	// holding inline markup is not there to be found, and only its runs can
+	// place a match inside it. See #502.
+	if b.Offset < 0 {
+		b.Runs = w.runs(shift, len(text))
+	}
+
 	return b
 }
 
+// runs returns the recorded runs covering [shift, shift+n) of the walker's
+// buffer, rebased onto a block that starts there.
+func (w *walker) runs(shift, n int) []nlp.Run {
+	var out []nlp.Run
+	for _, r := range w.spans {
+		lo, hi := max(r.At, shift), min(r.At+r.N, shift+n)
+		if lo >= hi {
+			continue
+		}
+		out = append(out, nlp.Run{At: lo - shift, Src: r.Src + (lo - r.At), N: hi - lo})
+	}
+	return out
+}
+
 // locate returns where text begins in the context, advancing the cursor past
 // it, or -1 if it cannot be found from the cursor onwards.
 //
@@ -344,15 +355,15 @@ func (w *walker) mapRun(at int, raw string) {
 
 	src := w.srcCursor + i
 	w.srcCursor = src + len(raw)
-	w.spans = append(w.spans, srcSpan{text: at, src: src, n: len(raw)})
+	w.spans = append(w.spans, nlp.Run{At: at, Src: src, N: len(raw)})
 }
 
 // sourceOffset returns where index `i` of the block's text sits in the source,
 // or -1 if that run was never mapped.
 func (w *walker) sourceOffset(i int) int {
 	for _, s := range w.spans {
-		if i >= s.text && i < s.text+s.n {
-			return s.src + (i - s.text)
+		if i >= s.At && i < s.At+s.N {
+			return s.Src + (i - s.At)
 		}
 	}
 	return -1
@@ -417,11 +428,12 @@ func (w *walker) replaceToks(tok html.Token) {
 //     more work: 38% slower overall. Worth revisiting with assignLoc rather
 //     than on its own; the two are coupled.
 //
-// That last attempt did surface a real defect: with the line taken from the
-// offset, an rst alert moved from 56:19 -- inside a literal block -- to 43:45,
-// the prose it actually belongs to. `testdata/features/frontmatter.feature`
-// still encodes the wrong position, and core.assignLoc has a standing NOTE
-// saying blk.Line is untrustworthy. Both are the same bug.
+// The line this returns is a fallback, and core.assignLoc has a standing NOTE
+// saying so. It matters less than it did: an alert in a block holding inline
+// markup is now placed by the runs the block was read from rather than by
+// searching from this line, which is what moved the rst alert in
+// `testdata/e2e/frontmatter.yaml` out of a literal block and onto the prose it
+// belongs to. What still comes through here is a block no run could place.
 func (w *walker) advance(text string) int {
 	ctx := w.getCtx()
 

internal/nlp/provider.go

@@ -31,6 +31,52 @@ type Block struct {
 	// finds the first occurrence rather than the one that matched. A sentence
 	// repeated in a document is the common case.
 	Offset int
+
+	// Runs maps pieces of Text back to the source they were read from, for a
+	// block that has no Offset of its own.
+	//
+	// Extraction drops inline markup, so `has <b>has</b>` arrives as `has has`
+	// and the block is nowhere in Context as a whole. Its pieces are, though,
+	// and each was placed as it was read. See #502.
+	Runs []Run
+}
+
+// A Run is a piece of a block's text and where it came from: At indexes the
+// block, Src the source, and N is how long both are.
+type Run struct {
+	At, Src, N int
+}
+
+// SourceOffset returns where index i of Text sits in Context, or -1 if that
+// part of the block was never mapped.
+func (b *Block) SourceOffset(i int) int {
+	if b.Offset >= 0 {
+		return b.Offset + i
+	}
+	for _, r := range b.Runs {
+		if i >= r.At && i < r.At+r.N {
+			return r.Src + (i - r.At)
+		}
+	}
+	return -1
+}
+
+// withRuns returns a copy of b carrying the runs of `parent` that fall within
+// [start, start+len(b.Text)), rebased onto b's own text.
+func (b Block) withRuns(parent []Run, start int) Block {
+	if start < 0 || len(parent) == 0 {
+		return b
+	}
+
+	end := start + len(b.Text)
+	for _, r := range parent {
+		lo, hi := max(r.At, start), min(r.At+r.N, end)
+		if lo >= hi {
+			continue
+		}
+		b.Runs = append(b.Runs, Run{At: lo - start, Src: r.Src + (lo - r.At), N: hi - lo})
+	}
+	return b
 }
 
 // NewBlock makes a new Block with prepared text and a Selector.
@@ -149,22 +195,27 @@ func (n *Info) Compute(block *Block, split bool) ([]Block, error) {
 // cursor advances past each piece so that repeated text resolves to successive
 // occurrences rather than always the first -- which is the whole point of
 // tracking offsets instead of searching for them later.
-func offsetOf(blk *Block, base int, piece string, cursor *int) int {
-	if base < 0 || *cursor > len(blk.Text) {
-		return -1
+func offsetOf(blk *Block, base int, piece string, cursor *int) (int, int) {
+	if *cursor > len(blk.Text) {
+		return -1, -1
 	}
 
 	i := strings.Index(blk.Text[*cursor:], piece)
 	if i < 0 {
 		// A segmenter that rewrites text (a remote endpoint, say) can return
 		// something that is not a substring of the input.
-		return -1
+		return -1, -1
 	}
 
 	start := *cursor + i
 	*cursor = start + len(piece)
 
-	return base + start
+	if base < 0 {
+		// The piece is placed within the block, which is itself unplaced; the
+		// runs it inherits are what will locate it.
+		return start, -1
+	}
+	return start, base + start
 }
 
 func (n *Info) doNLP(blk *Block, seg segmenter, split bool) ([]Block, error) {
@@ -178,7 +229,8 @@ func (n *Info) doNLP(blk *Block, seg segmenter, split bool) ([]Block, error) {
 		cursor := 0
 		for _, p := range strings.SplitAfter(blk.Text, "\n\n") {
 			b := NewLinedBlock(ctx, p, "paragraph."+blk.Scope, idx)
-			blks = append(blks, b.at(offsetOf(blk, base, p, &cursor)))
+			start, off := offsetOf(blk, base, p, &cursor)
+			blks = append(blks, b.at(off).withRuns(blk.Runs, start))
 		}
 	}
 
@@ -190,14 +242,15 @@ func (n *Info) doNLP(blk *Block, seg segmenter, split bool) ([]Block, error) {
 				continue
 			}
 			b := NewLinedBlock(ctx, s, "sentence."+blk.Scope, idx)
-			blks = append(blks, b.at(offsetOf(blk, base, s, &cursor)))
+			start, off := offsetOf(blk, base, s, &cursor)
+			blks = append(blks, b.at(off).withRuns(blk.Runs, start))
 		}
 	}
 
 	// The block itself, which is what most rules run against. It needs the
 	// offset as much as its children do.
 	blks = append(
-		blks, NewLinedBlock(ctx, blk.Text, blk.Scope, idx).at(base))
+		blks, NewLinedBlock(ctx, blk.Text, blk.Scope, idx).at(base).withRuns(blk.Runs, 0))
 
 	return blks, nil
 }