-
Notifications
You must be signed in to change notification settings - Fork 486
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.
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
tools/agentlint: add linter for river tags #2891
Conversation
Run: run, | ||
} | ||
|
||
var noLintRegex = regexp.MustCompile(`//\s*nolint:rivertags`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some places in our codebase we do something like this: //nolint:golint,rivertags
. Can we amend the regex to catch this as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Doing it at the regex level felt ugly, but I changed my approach to support this 👍
} else if len(nameParts) > 1 { | ||
diagnostics = append(diagnostics, "attr field names must not contain `.`") | ||
} | ||
for _, name := range nameParts { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's always going to be 1 element in nameParts here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm iterating over it anyway just in case they incorrectly put more than one, it's still useful (imo) to show the various errors across all the name fragments..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, easy to follow!
Extend tools/agentlint with a linter for
river
tags, which will help catch improper usages of tags where unit tests for unmarshaling aren't present (first seen in #2629).Linting rules can be skipped when necessary by adding a
//nolint:rivertags
comment.