-
-
Couldn't load subscription status.
- Fork 1.5k
feat: add support for SuggestedFixes #5232
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
Conversation
54579a1 to
6752f12
Compare
6752f12 to
94073d2
Compare
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.
Awesome work.
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.
This is really good, thanks for putting your time and effort into this!
Extra plus for the separate commits and flow making -10k LOC not so scary. Really nice with all the tests as well!
The linters that now have disabled Cgo tests are marked with TODO, should we create a tracking issue for that? Thinking it would be easier to help out with that part.
ca51724 to
1851cda
Compare
|
db85f32 to
898a5da
Compare
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.
🎉 Awesome
This PR extends the support of the "autofix" to all the linters that support
SuggestedFixes.I did my best to create a reviewable PR, so there are several commits but some are more important than others.
Supported linters
canonicalheader(new)dupword(new)err113(new)errorlint(new)fatcontext(new)gciginkgolinter(new)gocriticgodotgofmtgofumptgoheadergoimportsgosimple(new)s1001s1002s1003s1004s1005s1010s1011s1012s1016s1018s1021s1024s1025s1028s1030s1033s1034s1035s1036s1037s1039govet(new) (10 passes)assigncompositesfieldalignmentfindcallprintfsigchanyzersortslicestringintconvtimeformatunreachableiface(new)importas(new)intrange(new)mirrormisspellnakedret(new)nlreturn(new)nolintlintperfsprint(new)protogetterrevive(new) (2 rules)rangeerrorfstaticcheck(new)sa1004sa1006sa1008sa1012sa1013sa1016sa4013sa4026sa4029sa5004sa6005sa9002sa9004stylecheck(new)st1013st1017st1018tagaligntestifylint(new)whitespace(new)wsl(new)Total: 32 linters
Before the PR only 13 linters were able to apply "autofix".
With this PR, every linter that support SuggestedFixes will be able to "autofix" without require a specific implementation inside golangci-lint or inside the linter.
And the plugins can also apply "autofix".
Recommended Usage
I recommend using the following command to apply fixes:
ex:
I recommend not using a globally
fix: trueinside the configuration, or using it globally without--enable-onlyunless you trust all suggested fixes.About overlaps
Also overlapping is now managed in a stable way.
The situation:
This situation is 99% related to formatters because they want to apply changes to the same sections but not in the same way, so not in the same positions.
Before/Currently:
The overlap was managed by picking "randomly" one change for one of both linters (L4).
And all the other changes (without overlap) from the 2 linters were applied (L5, L6).
The result was a mix of non-consistent changes: deletion of imports, duplication of imports, etc.
The problem here is not the overlaps by themselves (because they are "managed") but the other changes.
With my implementation:
If there are overlaps, only one of the 2 linters is "allowed" to apply changes on the targeted file (ex: L4, L5, not L6).
The other linters that don't overlap are also "allowed" to apply changes to this file.
I tested my implementation with the examples from the issues, and it works as expected: no unexpected deletions, and no duplications.
Fixes #1779
Fixes #1490
Fixes #1728
Fixes #2161
Fixes #1726
Fixes #4885
Fixes #1510
Fixes #3819
Fixes #3454
Fixes #1510
Related to #2300
Related to #2609
Notes
Linters that have not been migrated cannot be migrated without creating regressions.
These linters will be processed, but they require significant work so this will be done gradually.
If you enjoy this change, and you or your company want to help golangci-lint or me: