From 05bed1e8460a77c59a140fadfa6d1d57135513ed Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sat, 8 Aug 2026 21:16:57 +0000 Subject: [PATCH] Keep the extracted text one line per source line A run of line comments was joined with two newlines for a blank one and a terminator that was decided by looking at the run instead of the text it was being attached to. The extracted text and the source it maps back to then had different numbers of lines, so every alert below a blank comment line was reported a line too far down, with its column collapsed to 1 because the source line it was measured against was past the end. The two errors cancelled for a comment whose node content carries its own newline -- Rust's `///` -- which is why columns there looked right while the text was already a line out. Lining the two up exposes that, so the space after the delimiter is now trimmed from every joined line rather than only the ones that still needed a newline; it is counted once, in the padding. Fixes #1022 --- internal/lint/code/comments.go | 50 ++++++++++++++++++++++++++-------- testdata/comments/out/0.json | 2 +- testdata/comments/out/1.json | 14 +++++----- testdata/comments/out/5.json | 2 +- testdata/e2e/fragments.yaml | 43 +++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 21 deletions(-) diff --git a/internal/lint/code/comments.go b/internal/lint/code/comments.go index ffa99b71..9b1436c3 100644 --- a/internal/lint/code/comments.go +++ b/internal/lint/code/comments.go @@ -50,22 +50,48 @@ func doneMerging(curr, prev Comment) bool { return false } -func addSourceLine(line string, atEnd bool) string { +// appendLine adds one line to a pending run of line comments. +// +// One newline per line, so the run ends up with exactly as many lines as the +// source it came from. A blank line comment -- `//` with nothing after it -- +// is empty and contributes only its newline; giving it two ended that line +// twice and put a line in the extracted text that the source doesn't have. +// See #1022. +func appendLine(line string) string { if line == "" { - return "\n\n" + return "\n" } - if !strings.HasPrefix(line, "\n") && !atEnd { - line = strings.TrimLeft(line, " ") - line = fmt.Sprintf("\n%s", line) - } else if !strings.HasSuffix(line, "\n") && atEnd { - line = strings.TrimLeft(line, " ") - line = fmt.Sprintf("%s\n", line) + // The space after the delimiter belongs to the delimiter, and the padding + // added when an alert is mapped back already counts it. Trimming only the + // lines that still needed a newline left it on the ones that didn't -- + // Rust's `///`, whose node content carries its own -- where it was then + // counted twice. The first line of a run is trimmed by the caller. + line = strings.TrimLeft(line, " ") + + if !strings.HasSuffix(line, "\n") { + return fmt.Sprintf("%s\n", line) } return line } +// attachRun joins a pending run of lines to the text it belongs to. +// +// The run needs the line above it terminated, which is a question about that +// text and not about the run: a comment whose node content already carried its +// newline -- Rust's `//!`, for one -- is terminated, and adding another puts a +// blank line between the two that the source doesn't have. Keying this off the +// run instead lost the terminator whenever the run began with a blank comment +// line, which cost a line in the other direction. +func attachRun(text, run string) string { + if !strings.HasSuffix(text, "\n") { + text += "\n" + } + + return text + strings.TrimLeft(run, " ") +} + func coalesce(comments []Comment) []Comment { var joined []Comment @@ -78,8 +104,8 @@ func coalesce(comments []Comment) []Comment { if tBuf.Len() > 0 { last := joined[len(joined)-1] - last.Text += addSourceLine(tBuf.String(), false) - last.Source += addSourceLine(sBuf.String(), false) + last.Text = attachRun(last.Text, tBuf.String()) + last.Source = attachRun(last.Source, sBuf.String()) joined[len(joined)-1] = last @@ -100,8 +126,8 @@ func coalesce(comments []Comment) []Comment { flush() joined = append(joined, comment) } else { - tBuf.WriteString(addSourceLine(comment.Text, true)) - sBuf.WriteString(addSourceLine(comment.Source, true)) + tBuf.WriteString(appendLine(comment.Text)) + sBuf.WriteString(appendLine(comment.Source)) } } diff --git a/testdata/comments/out/0.json b/testdata/comments/out/0.json index e6ef9cc6..427e4f43 100644 --- a/testdata/comments/out/0.json +++ b/testdata/comments/out/0.json @@ -7,7 +7,7 @@ "Scope": "text.comment.block" }, { - "Text": "Println formats using the default formats for its oprands and writes to\nstandard output.\n\n\nSpaces are always added between operands and a newline is appended.\n\n\nIt returns the number of bytes written and any write error encountered.\n", + "Text": "Println formats using the default formats for its oprands and writes to\nstandard output.\n\nSpaces are always added between operands and a newline is appended.\n\nIt returns the number of bytes written and any write error encountered.\n", "Source": "// Println formats using the default formats for its oprands and writes to\n// standard output.\n//\n// Spaces are always added between operands and a newline is appended.\n//\n// It returns the number of bytes written and any write error encountered.\n", "Line": 11, "Offset": 0, diff --git a/testdata/comments/out/1.json b/testdata/comments/out/1.json index 7b36fa5c..0a006346 100644 --- a/testdata/comments/out/1.json +++ b/testdata/comments/out/1.json @@ -1,14 +1,14 @@ [ { - "Text": "This module defines the set of command line arguments that ripgrep supports,\nincluding some light validation.\n\n\nThis module is purposely written in a bare-bones way, since it is included\nin ripgrep's build.rs file as a way to generate a man page and completion\nfiles for common shells.\n\n\nThe only other place that ripgrep deals with clap is in src/args.rs, which\nis where we read clap's configuration from the end user's arguments and turn\nit into a ripgrep-specific configuration type that is not coupled with clap.\n", + "Text": "This module defines the set of command line arguments that ripgrep supports,\nincluding some light validation.\n\nThis module is purposely written in a bare-bones way, since it is included\nin ripgrep's build.rs file as a way to generate a man page and completion\nfiles for common shells.\n\nThe only other place that ripgrep deals with clap is in src/args.rs, which\nis where we read clap's configuration from the end user's arguments and turn\nit into a ripgrep-specific configuration type that is not coupled with clap.\n", "Source": "// This module defines the set of command line arguments that ripgrep supports,\n// including some light validation.\n//\n// This module is purposely written in a bare-bones way, since it is included\n// in ripgrep's build.rs file as a way to generate a man page and completion\n// files for common shells.\n//\n// The only other place that ripgrep deals with clap is in src/args.rs, which\n// is where we read clap's configuration from the end user's arguments and turn\n// it into a ripgrep-specific configuration type that is not coupled with clap.\n", "Line": 1, "Offset": 0, "Scope": "text.comment.line" }, { - "Text": "A human being is representd here\n\n A human being is representd here\n", - "Source": "/// A human being is representd here\n\n///\n/// A human being is representd here\n", + "Text": "A human being is representd here\n\nA human being is representd here\n", + "Source": "/// A human being is representd here\n///\n/// A human being is representd here\n", "Line": 13, "Offset": 0, "Scope": "text.comment.line" @@ -21,15 +21,15 @@ "Scope": "text.comment.line" }, { - "Text": "Returns a person with the name given them\n\n # Arguments\n\n * `foof` - A string slice doof that holds the nme of the person\n\n # Exmples\n\n ```\n You can have rust code between fences inside the comments\n If you pass --test to `rustdoc`, it will even test it for you!\n use doc::Person;\n let person = Person::new(\"name\");\n ```\n", - "Source": "/// Returns a person with the name given them\n\n///\n/// # Arguments\n///\n/// * `foof` - A string slice doof that holds the nme of the person\n///\n/// # Exmples\n///\n/// ```\n/// // You can have rust code between fences inside the comments\n/// // If you pass --test to `rustdoc`, it will even test it for you!\n/// use doc::Person;\n/// let person = Person::new(\"name\");\n/// ```\n", + "Text": "Returns a person with the name given them\n\n# Arguments\n\n* `foof` - A string slice doof that holds the nme of the person\n\n# Exmples\n\n```\nYou can have rust code between fences inside the comments\nIf you pass --test to `rustdoc`, it will even test it for you!\nuse doc::Person;\nlet person = Person::new(\"name\");\n```\n", + "Source": "/// Returns a person with the name given them\n///\n/// # Arguments\n///\n/// * `foof` - A string slice doof that holds the nme of the person\n///\n/// # Exmples\n///\n/// ```\n/// // You can have rust code between fences inside the comments\n/// // If you pass --test to `rustdoc`, it will even test it for you!\n/// use doc::Person;\n/// let person = Person::new(\"name\");\n/// ```\n", "Line": 22, "Offset": 4, "Scope": "text.comment.line" }, { - "Text": "Gives a friendly hello!\n\n Says \"Hello, [name]\" to the `Person` it is called on.\n", - "Source": "/// Gives a friendly hello!\n\n///\n/// Says \"Hello, [name]\" to the `Person` it is called on.\n", + "Text": "Gives a friendly hello!\n\nSays \"Hello, [name]\" to the `Person` it is called on.\n", + "Source": "/// Gives a friendly hello!\n///\n/// Says \"Hello, [name]\" to the `Person` it is called on.\n", "Line": 42, "Offset": 4, "Scope": "text.comment.line" diff --git a/testdata/comments/out/5.json b/testdata/comments/out/5.json index ad2ed4e6..8ea98f66 100644 --- a/testdata/comments/out/5.json +++ b/testdata/comments/out/5.json @@ -1,6 +1,6 @@ [ { - "Text": "This Deployment runs our API component\n\nTo increase the TODO number of replicas that run,\nchange the value of the spec.replicas field.\n\n\nAlways use a value higher than 1 to ensure\nmultiple replicas are running, as this\nguarantees redundancy if one instance fails.\n", + "Text": "This Deployment runs our API component\n\nTo increase the TODO number of replicas that run,\nchange the value of the spec.replicas field.\n\nAlways use a value higher than 1 to ensure\nmultiple replicas are running, as this\nguarantees redundancy if one instance fails.\n", "Source": "# This Deployment runs our API component\n#\n# To increase the TODO number of replicas that run,\n# change the value of the spec.replicas field.\n#\n# Always use a value higher than 1 to ensure\n# multiple replicas are running, as this\n# guarantees redundancy if one instance fails.\n", "Line": 1, "Offset": 0, diff --git a/testdata/e2e/fragments.yaml b/testdata/e2e/fragments.yaml index 05db2555..02bbfe9c 100644 --- a/testdata/e2e/fragments.yaml +++ b/testdata/e2e/fragments.yaml @@ -55,6 +55,49 @@ cases: test2.rs:409:38:Vale.Spelling:Did you really mean 'RGArg'? test2.rs:2860:34:Vale.Spelling:Did you really mean 'conlicts'? + - name: blank-comment-line + about: | + #1022: a blank line comment produced two lines in the extracted text + where the source has one, so every alert below it was reported a line + too far down with its column collapsed to 1. + files: + .vale.ini: | + StylesPath = styles + MinAlertLevel = suggestion + + [*.{proto,go}] + BasedOnStyles = T + styles/T/Eror.yml: | + extends: existence + message: "Found '%s'." + level: error + tokens: + - Eror + test.proto: | + syntax = "proto3"; + + // Correct. + // Correct. + // Eror. + // + // Eror. + message Message {} + test.go: | + package main + + // Eror. + // + // + // Eror. + func main() {} + args: . + exit: 1 + want: | + test.go:3:4:T.Eror:Found 'Eror'. + test.go:6:4:T.Eror:Found 'Eror'. + test.proto:5:4:T.Eror:Found 'Eror'. + test.proto:7:4:T.Eror:Found 'Eror'. + - name: tab-indented-block about: | #1130: only spaces were stripped when dedenting a block comment, so one