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