-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
support fixes as individual analyzers #78
Comments
Thanks for filing this issue! I indeed want to improve the situation here. Just to clarify - do we need one analizer per "rule"? My worry is that that would require one entirely separate AST walk per rule, which would probably add quite a bit of cost. From what you can see in the README, we already have a dozen or so of these, and the number will only increase over time. |
If we want to show granular diagnostics and allow people to accept fixes individually, then yes, we would need one analyzer per rule. An alternative would be to keep it at 1 analyzer, but then the diagnostic would show up like "accept all |
Hmm. Maybe I could group them by general features; one analyzer to remove or add empty lines, one to make syntax more consistent, one to fix the imports, etc. That seems like the best of both worlds. Having 30 analyzers would be too expensive, and probably annoying to the end user. And having just one analyzer with everything does not bring any benefit over the current API similar to |
I agree with this - one analyzer is not a viable approach, and a warning for an extra line isn't the best user experience. If you can categorize them, that sounds good, but I wonder how the different diagnostics will work. One other thing - you could do it all in one analyzer and just have it report different diagnostics/fixes in different cases, I think. |
Yeah, maybe I'll try a single analyzer as the first refactor and see how well it works. One https://godoc.org/golang.org/x/tools/go/analysis#SuggestedFix for each fix we encounter, and the message can tell the user what rule is being applied. |
@mvdan what is the stats on implementing the Analyzer? It would really helps me in my project of custom linter |
@mvdan I have created a simple wrapper to provide an analyzer with suggested fixes, if you are interesting, I can contribute it to this project |
This is a follow-up on golang/go#39805. We have integrated
gofumpt
as an opt-in formatter, but this approach means thatgofumpt
will be less discoverable and easy-to-use. The best way to integrategofumpt
intogopls
would be through diagnostics and suggested fixes, so that users could apply fixes granularly and aren't required to apply fixes only on save (or manual format).If you would be willing to create go/analysis.Analyzer's for each of
gofumpt
's fixes, we would probably be able to enable it by default and have users opt-in to get the fixes on-save. I understand this is a big undertaking, so there's no particular rush or time pressure here.The text was updated successfully, but these errors were encountered: