Add nolintlint linter #740
Conversation
|
|
ashanbrown
force-pushed
the
asb/nolintlint
branch
from
300aff5 to
6be564b
Compare
go.mod
Outdated
| @@ -38,7 +39,9 @@ require ( | |||
| github.com/ultraware/funlen v0.0.2 | |||
| github.com/ultraware/whitespace v0.0.3 | |||
| github.com/valyala/quicktemplate v1.2.0 | |||
| golang.org/x/tools v0.0.0-20190912215617-3720d1ec3678 | |||
| golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab // indirect | |||
| @@ -213,6 +213,7 @@ lll: Reports long lines [fast: true, auto-fix: false] | |||
| maligned: Tool to detect Go structs that would take less memory if their fields were sorted [fast: true, auto-fix: false] | |||
| misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true] | |||
| nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false] | |||
| nolintlint: Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false] | |||
There was a problem hiding this comment.
Can you make these changes to README.tmpl.md and regenerate the README.md from that?
There was a problem hiding this comment.
My understanding is that this is automatically generated already from somewhere else. I just ran make README.md to create it, so I think this is right but please let me know if there is some other file I should be changing.
There was a problem hiding this comment.
You are right, @ashanbrown, it is expected for README.md to be re-generated after the README.tmpl.md change. The CI checks specifically for that, forbidding outdated README.md.
So it should be OK.
|
@ashanbrown Thanks for the contribution! It looks like the CI build is failing due to a vendor discrepancy. I've also provided a few comments on the change. |
|
@tpounds I think I've addressed your concerns. I'm a bit of newb wrt to go.mod so I'm not quite sure what the implications of removing indirect dependencies from go.mod might be. I may be misunderstanding what happened but I think I also had to run go mod vendor in go1.13 to remove the extra vendor files that go mod vendor in go1.12 wanted to add. I also added a few more comments to explain what the nolintlint options do and why we need to skip the nolint check for the nolintlint linter. Thanks for considering this change. If you're interested, I'd be happy to put up a PR to start enforcing nolintlint on golangci-lint itself, probably starting with requiring a specific linter wherever |
|
Hi, sorry for writing this but I think it should be implemented inside
|
|
@jirfag That definitely makes sense and was something I was anticipating. I'd be happy to create a PR to move this into golangci-lint itself if you'd welcome that. |
|
I'd be happy to merge such improvement, thank you! |
This PR adds a linter for nolint directives. The purpose of this linter to enforce an optional rule that nolint directives have a specific linter mentioned and an explanation, which is a requirement at my workplace. It also reports nolint directives that don't match the prescribed format.
I realize this is not an existing, proven linter and not a linter that makes much sense outside of golangci-lint. There is a library at https://github.com/ashanbrown/nolintlint that implements this although it might make sense to roll this into golangci-lint itself if you think it's interesting. Would be happy to chat more about this.
FWIW, the nolintlint linter cannot be skipped with nolint.