nogo: add more staticcheck analyzers#12598
Conversation
36970ec to
0fa9fad
Compare
c83b596 to
4f17a0e
Compare
4f17a0e to
9924852
Compare
nixprime
left a comment
There was a problem hiding this comment.
Fix QF1008 violations and drop suppression
IMO this rule makes the code worse rather than better.
9924852 to
bed421a
Compare
Fair. I've split it out of this PR so we can get the less controversial bits merged. |
Add Go version plumbing to support version-gated lints.
bed421a to
a50651c
Compare
|
Gentle ping here folks. Anything I can do to nudge this along? |
|
Sorry, this is still pending on internal review :( |
Please see individual commits: - **nogo: add more staticcheck analyzers** - **Fix QF1001 violations and drop suppression** - **Fix QF1002 violations and drop suppression** - **Fix QF1003 violations and drop suppression** - **Fix QF1004 violations and drop suppression** - **Fix QF1005 violations and drop suppression** - **Fix QF1006 violations and drop suppression** - **Fix QF1007 violations and drop suppression** - **Fix QF1008 violations and drop suppression** - **Fix QF1012 violations and drop suppression** - **Fix S1000 violations and drop suppression** - **Fix S1002 violations and drop suppression** - **Fix S1003 violations and drop suppression** - **Fix S1004 violations and drop suppression** - **Fix S1005 violations and drop suppression** - **Fix S1007 violations and drop suppression** - **Fix S1008 violations and drop suppression** - **Fix S1009 violations and drop suppression** - **Fix S1011 violations and drop suppression** - **Fix S1012 violations and drop suppression** - **Fix S1016 violations and drop suppression** - **Fix S1019 violations and drop suppression** - **Fix S1020 violations and drop suppression** - **Fix S1021 violations and drop suppression** - **Fix S1023 violations and drop suppression** - **Fix S1024 violations and drop suppression** - **Fix S1025 violations and drop suppression** - **Fix S1034 violations and drop suppression** - **Fix S1030 violations and drop suppression** - **Fix S1039 violations and drop suppression** - **Fix S1033 violations and drop suppression** - **Fix S1040 violations and drop suppression** FUTURE_COPYBARA_INTEGRATE_REVIEW=#12598 from tamird:nogo-more-checks a50651c PiperOrigin-RevId: 868765670
Please see individual commits: - **nogo: add more staticcheck analyzers** - **Fix QF1001 violations and drop suppression** - **Fix QF1002 violations and drop suppression** - **Fix QF1003 violations and drop suppression** - **Fix QF1004 violations and drop suppression** - **Fix QF1005 violations and drop suppression** - **Fix QF1006 violations and drop suppression** - **Fix QF1007 violations and drop suppression** - **Fix QF1008 violations and drop suppression** - **Fix QF1012 violations and drop suppression** - **Fix S1000 violations and drop suppression** - **Fix S1002 violations and drop suppression** - **Fix S1003 violations and drop suppression** - **Fix S1004 violations and drop suppression** - **Fix S1005 violations and drop suppression** - **Fix S1007 violations and drop suppression** - **Fix S1008 violations and drop suppression** - **Fix S1009 violations and drop suppression** - **Fix S1011 violations and drop suppression** - **Fix S1012 violations and drop suppression** - **Fix S1016 violations and drop suppression** - **Fix S1019 violations and drop suppression** - **Fix S1020 violations and drop suppression** - **Fix S1021 violations and drop suppression** - **Fix S1023 violations and drop suppression** - **Fix S1024 violations and drop suppression** - **Fix S1025 violations and drop suppression** - **Fix S1034 violations and drop suppression** - **Fix S1030 violations and drop suppression** - **Fix S1039 violations and drop suppression** - **Fix S1033 violations and drop suppression** - **Fix S1040 violations and drop suppression** FUTURE_COPYBARA_INTEGRATE_REVIEW=#12598 from tamird:nogo-more-checks a50651c PiperOrigin-RevId: 868765670
|
|
||
| QF1001: # Apply De Morgan’s law | ||
| # Controversial. | ||
| internal: | ||
| exclude: [".*"] | ||
| QF1008: # Omit embedded fields from selector expression | ||
| # Controversial. | ||
| internal: | ||
| exclude: [".*"] |
There was a problem hiding this comment.
Why add these at all if these are going to be excluded on .*?
There was a problem hiding this comment.
What does that code look like, that doesn't add them? have a look at tools/nogo/check/analyzers.go, we aren't adding these by name.
There was a problem hiding this comment.
What does that code look like, that doesn't add them?
The version of nogo.yaml at HEAD does not reference these (QF1001 and QF1008). I see you have an intermediate commit to "move and explain suppression": 2744619 and d171aaa. I don't know which other commit in this PR added these to nogo.yaml in the first place.
But what I was suggesting was just removing these lines altogether?
There was a problem hiding this comment.
Removing these lines amounts to enabling these linters. The suppressions were added in the very first commit.
I changed that commit to move these to the bottom so that the later commits would apply without merge conflicts.
The bottom line is that we need these here because we want these lints off.
Please see individual commits: