lintcmd: make -check case-insensitive, error on unknown checks#1546
lintcmd: make -check case-insensitive, error on unknown checks#1546arp242 wants to merge 1 commit intodominikh:masterfrom
Conversation
This changes it so that the check is case-insensitive, and so it will return an error on unknown checks. Previously: % staticcheck -checks=sa1000 ./lintcmd [no output, exit 0] % staticcheck -checks=banana ./lintcmd [no output, exit 0] % staticcheck -checks=SA1000 ./lintcmd lintcmd/cmd.go:270:34: error parsing regexp: missing closing ): `$^(` (SA1000) This is massively confusing! When I was doing my testing for the other PRs yesterday I found it odd that I didn't get any error for the defer one, since I *knew* there was at least one module with this error. Turned out it just always passed because I used lower-case. I wouldn't be surprised if tons of people have made this mistake and have run (or are running) staticcheck runs that don't actually do anything. All of this also applies to checks = [..] from the config file.
We intentionally don't complain on unknown checks, at least in the Maybe as a compromise we can emit diagnostics for unknown checks?
I can see the appeal in that, but for consistency we'd also have to do this for I think I'd prefer it if we stuck to only one way of spelling check names (i.e. the current way), and instead focused on making it more obvious when check names are misspelled. |
|
Emitting a diagnostic is okay. I'll change that. Personally I'd strongly prefer case-insensitive checks whenever feasible. To me at least it's much more natural to type "sa1000" rather than "SA1000" (even when knowing about this, I again did it wrong yesterday). It should probably be case-insensitive in We can't make everything 100% case-insensitive, and that's okay; not many people are manually typing anchor tags for the website. If we can do >99% of where people are typing it then that's a win IMO. We can know what's intended with 100% certainty, so it just doesn't make any sense to stand on ceremony and reject it. This is also why I made |
This changes it so that the check is case-insensitive, and so it will return an error on unknown checks. Previously:
This is massively confusing! When I was doing my testing for the other PRs yesterday I found it odd that I didn't get any error for the defer one, since I knew there was at least one module with this error. Turned out it just always passed because I used lower-case.
I wouldn't be surprised if tons of people have made this mistake and have run (or are running) staticcheck runs that don't actually do anything.
All of this also applies to checks = [..] from the config file.