Allow custom linters to auto-fix suggested fixes #2300
Conversation
StevenACoffman
changed the title
|
The lint failure for is for |
|
|
This allows custom linters hook into the `--fix` functionality. Custom linters specify the fixes using the Go analysis structures, which allow for arbitrary char offsets for fixes; they get converted into golangci structures, which are line-based. If the conversion is not possible, the fix is dropped on the floor. Signed-off-by: Steve Coffman <steve@khanacademy.org>
StevenACoffman
force-pushed
the
suggested_fix
branch
from
57ed6d5
to
d8ad9c3
Compare
| @@ -116,7 +116,7 @@ func configureCheckerInfo(info *gocriticlinter.CheckerInfo, allParams map[string | |||
| // but the file parsers (TOML, YAML, JSON) don't create the same representation for raw type. | |||
| // then we have to convert value types into the expected value types. | |||
| // Maybe in the future, this kind of conversion will be done in go-critic itself. | |||
| //nolint:exhaustive // only 3 types (int, bool, and string) are supported by CheckerParam.Value | |||
| //nolint:exhaustive,nolintlint // only 3 types (int, bool, and string) are supported by CheckerParam.Value | |||
There was a problem hiding this comment.
Without nolintlint this is incorrectly identified as unused, but removing the entire comment correctly causes exhaustive linter to fail. This seems unrelated to my changes, so I added this to unblock.
|
I don't have the time now to explain why, but this PR is |
|
Thanks for letting me know. I know you are busy so I can wait until you have more time. I saw your ldez@28b5be3 so I assume you have some other unpushed commits and have something else in mind. |
|
@ldez @StevenACoffman If there's anything I can do here to help out, please let me know. I'm only working half time for the next month or so, but glad to put in some time where I can find it to clean this PR up if needed, or adjust it to an alternate design if desired. |
|
I think that the real solution is to add support for SuggestedFixes (#1779) to replace the current/old system. I'm still working on it but, for me, it's a part of the v2. I'll need to take time to think about a v2 plan. |
|
Thanks for your update. While this PR works well as is, it will not be merged at this time, as a more sweeping redesign is being planned by a maintainer who has a track record of past excellent contributions. I'll close the PR, and we'll continue to carry it in our fork, so there's no need to further rush your design or add stress to your volunteer commitment. Again, sorry to have troubled you. Feel free to ping me or anyone at Khan if you reconsider. |
|
For other people's future reference, an alternative implementation for applying suggested fixes is here: https://github.com/golang/tools/blob/54dc8c5edb561003330ec7e649333181bbd0a9bd/go/analysis/internal/checker/checker.go#L316 |
This allows custom linters hook into the
--fixfunctionality to resolve #1728Custom linters specify the fixes using the Go analysis structures,
which allow for arbitrary char offsets for fixes; they get converted
into golangci structures, which are line-based. If the conversion is
not possible, the fix is dropped on the floor.
This also potentially partially addresses #1779 for those fixes that can be converted to line-based. For those that cannot, a modification to golangci structures is still necessary.
Current linters that support
SuggestedFixes:goerr113ruleguardstaticchecknlreturnexportlooprefexhaustivegovetThis work is a cherry-pick of Khan@b48d220 by @dbraley and @csilvers
Signed-off-by: Steve Coffman steve@khanacademy.org