fix: don't mask an autolink's URL twice

Fixes #847

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

Author: jdkato

internal/lint/walk.go

@@ -88,6 +88,14 @@ type walker struct {
 	// since their position is derived from its own. Reset with the block.
 	inline []inlineCapture
 
+	// pending holds the attribute values of the element just opened, waiting
+	// to be masked out of the context. They can't be masked on the start tag
+	// itself: an autolink's `href` and its text are the same source bytes, so
+	// masking the attribute leaves the text token to find its string somewhere
+	// else in the document -- and `idx`, which only moves forward, then places
+	// every later block on that line. See #847.
+	pending []string
+
 	begin int
 	end   int
 
@@ -126,7 +134,22 @@ func (w *walker) update(txt string, tokt html.TokenType) {
 	}
 }
 
+// flush masks the attribute values held since the last start tag.
+//
+// `text` is the element's own text, if it has arrived; an attribute equal to it
+// is dropped rather than masked, because the two are one run of source and
+// masking it twice consumes an occurrence that belongs to something else.
+func (w *walker) flush(text string) {
+	for _, val := range w.pending {
+		if val != text {
+			w.update(val, html.TextToken)
+		}
+	}
+	w.pending = nil
+}
+
 func (w *walker) reset() {
+	w.flush("")
 	for _, s := range w.queue {
 		w.update(s, html.TextToken)
 	}
@@ -143,6 +166,10 @@ func (w *walker) getCtx() string {
 
 func (w *walker) append(text string) {
 	if text != "" {
+		// Before the search below, so the text finds its own occurrence, and
+		// after the drop, so an autolink's `href` doesn't take it first.
+		w.flush(text)
+
 		pos := w.advance(text)
 		if pos > -1 {
 			w.idx = pos
@@ -340,6 +367,9 @@ func (w *walker) walk() (html.TokenType, html.Token, string) {
 func (w *walker) replaceToks(tok html.Token) {
 	tags := core.StringInSlice(tok.Data, []string{
 		"img", "a", "p", "script", "h1", "h2", "h3", "h4", "h5", "h6", "span"})
+	// Anything still waiting belongs to an element that had no text of its own.
+	w.flush("")
+
 	if tags {
 		names := []string{"href", "id", "src", "alt"}
 		if w.ext == ".html" {
@@ -356,7 +386,7 @@ func (w *walker) replaceToks(tok html.Token) {
 				if a.Key == "href" {
 					a.Val, _ = url.QueryUnescape(a.Val)
 				}
-				w.update(a.Val, html.TextToken)
+				w.pending = append(w.pending, a.Val)
 			}
 		}
 	}

internal/lint/walk_test.go

@@ -148,3 +148,52 @@ func TestWalkerClasses(t *testing.T) {
 		})
 	}
 }
+
+// An element's attributes are masked out of the context, but not the source
+// bytes its own text still has to find. An autolink writes the URL once and
+// the parser reports it twice -- as `href` and as text -- so masking the
+// attribute first sent the text's search to the next copy of that URL, wherever
+// in the document it happened to be. See #847.
+func TestWalkerFlush(t *testing.T) {
+	tests := []struct {
+		name    string
+		pending []string
+		text    string
+		want    string
+	}{
+		{
+			name:    "autolink keeps its own bytes",
+			pending: []string{"http://github.com"},
+			text:    "http://github.com",
+			want:    "http://github.com here\nand http://github.com there\n",
+		},
+		{
+			name:    "a link target is masked",
+			pending: []string{"http://github.com"},
+			text:    "read this",
+			want:    "@@@@@@@@@@@@@@@@@ here\nand http://github.com there\n",
+		},
+		{
+			name:    "no text of its own",
+			pending: []string{"http://github.com"},
+			text:    "",
+			want:    "@@@@@@@@@@@@@@@@@ here\nand http://github.com there\n",
+		},
+	}
+
+	const src = "http://github.com here\nand http://github.com there\n"
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			w := &walker{context: []byte(src), pending: tt.pending}
+
+			w.flush(tt.text)
+
+			if got := w.getCtx(); got != tt.want {
+				t.Errorf("flush(%q) left %q, want %q", tt.text, got, tt.want)
+			}
+			if w.pending != nil {
+				t.Errorf("pending = %v, want nil", w.pending)
+			}
+		})
+	}
+}

testdata/e2e/patterns.yaml

@@ -130,3 +130,64 @@ cases:
     want: |
       guides/b.md:1:15:T.Tok:'ZQX' found
       tutorials/a.md:1:27:T.Tok:'QQV' found
+
+  - name: repeated-autolink
+    about: "#847 -- an autolink's URL is one run of source, reported twice by
+      the parser; masking the `href` copy used to send the text's search to the
+      next identical URL in the document, moving every later block's alerts
+      onto its line. The three files differ only in lines that hold no alert,
+      so all three report the same two."
+    files:
+      .vale.ini: |
+        StylesPath = styles
+        MinAlertLevel = suggestion
+
+        [*.md]
+        BasedOnStyles = T
+      styles/T/GitHub.yml: |
+        extends: substitution
+        message: Use '%s' instead of '%s'
+        level: warning
+        ignorecase: true
+        swap:
+          GitHub: GitHub
+      trailing-prose.md: |
+        http://github.com bar
+
+        [github](#whatever)
+        [github1](#whatever)
+        [github.](#whatever)
+
+        https://jwodder.github.io/
+        http://github.com
+
+        whatever
+      trailing-url.md: |
+        http://github.com bar
+
+        [github](#whatever)
+        [github1](#whatever)
+        [github.](#whatever)
+
+        https://jwodder.github.io/
+
+        http://github.com
+      no-leading-url.md: |
+        bar
+
+        [github](#whatever)
+        [github1](#whatever)
+        [github.](#whatever)
+
+        https://jwodder.github.io/
+
+        http://github.com
+    args: .
+    exit: 0
+    want: |
+      no-leading-url.md:3:2:T.GitHub:Use 'GitHub' instead of 'github'
+      no-leading-url.md:5:2:T.GitHub:Use 'GitHub' instead of 'github'
+      trailing-prose.md:3:2:T.GitHub:Use 'GitHub' instead of 'github'
+      trailing-prose.md:5:2:T.GitHub:Use 'GitHub' instead of 'github'
+      trailing-url.md:3:2:T.GitHub:Use 'GitHub' instead of 'github'
+      trailing-url.md:5:2:T.GitHub:Use 'GitHub' instead of 'github'