Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 38 additions & 12 deletions internal/lint/code/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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))
}
}

Expand Down
2 changes: 1 addition & 1 deletion testdata/comments/out/0.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions testdata/comments/out/1.json
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion testdata/comments/out/5.json
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
43 changes: 43 additions & 0 deletions testdata/e2e/fragments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading