Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix comment ignore with trailing comment not recognized #3295

Merged
merged 6 commits into from
Sep 6, 2024
Merged

Fix comment ignore with trailing comment not recognized #3295

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
545555a
Allow comment ignore with trailing comment
oliversun9 Sep 6, 2024
ea4093e
Merge branch 'main' into osun/allow-comment-ignore-with-trailing-comment
oliversun9 Sep 6, 2024
a2d58ba
update comment
oliversun9 Sep 6, 2024
f516a9c
fix comment
oliversun9 Sep 6, 2024
7406b5d
add changelog entry
oliversun9 Sep 6, 2024
78c78e6
fix changelog
oliversun9 Sep 6, 2024
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
24 changes: 11 additions & 13 deletions private/bufpkg/bufcheck/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,27 +491,25 @@ func ignoreLocation(
}

// checkCommentLineForCheckIgnore checks that the comment line starts with the configured
// comment ignore prefix, and the rest of the string is the ruleID of the check.
// comment ignore prefix, a space and the ruleID of the check.
//
// We currently do not support multiple rules per comment ignore:
// All of the following comments are valid, ignoring SERVICE_PASCAL_CASE and this rule only:
//
// Invalid:
// // buf:lint:ignore SERVICE_SUFFIX, SERVICE_PASCAL_CASE
// // buf:lint:ignore SERVICE_PASCAL_CASE, SERVICE_SUFFIX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this saying that you can do comma-separated ignores? I don't think that's in scope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated comment in f516a9c (it also said ignoring SERVICE_PASCAL_CASE and this rule only on top)

// // buf:lint:ignore SERVICE_PASCAL_CASE
// // buf:lint:ignore SERVICE_PASCAL_CASEsome other comment
// // buf:lint:ignore SERVICE_PASCAL_CASE some other comment
//
// Valid:
// // buf:lint:ignore SERVICE_SUFFIX
// // buf:lint:ignore SERVICE_PASCAL_CASE
// While the following is invalid and a nop
//
// // buf:lint:ignoreSERVICE_PASCAL_CASE
func checkCommentLineForCheckIgnore(
commentLine string,
commentIgnorePrefix string,
ruleID string,
) bool {
if after, ok := strings.CutPrefix(commentLine, commentIgnorePrefix); ok {
if strings.TrimSpace(after) == ruleID {
return true
}
}
return false
fullIgnorePrefix := commentIgnorePrefix + " " + ruleID
return strings.HasPrefix(commentLine, fullIgnorePrefix)
}

type lintOptions struct {
Expand Down
8 changes: 8 additions & 0 deletions private/bufpkg/bufcheck/lint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,14 @@ func TestCommentIgnoresOnlyRule(t *testing.T) {
)
}

func TestCommentIgnoresWithTrailingComment(t *testing.T) {
t.Parallel()
testLint(
t,
"comment_ignores_with_trailing_comment",
)
}

func TestRunLintCustomPlugins(t *testing.T) {
t.Parallel()
testLint(
Expand Down
4 changes: 4 additions & 0 deletions private/bufpkg/bufcheck/testdata/lint/comment_ignores_with_trailing_comment/a.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
syntax = "proto3";

// buf:lint:ignore PACKAGE_DIRECTORY_MATCH trailing comment after the ignore comment is also OK
package a;
4 changes: 4 additions & 0 deletions private/bufpkg/bufcheck/testdata/lint/comment_ignores_with_trailing_comment/buf.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
version: v2
lint:
use:
- PACKAGE_DIRECTORY_MATCH