Skip to content
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

Open
stamblerre opened this issue Jul 13, 2020 · 7 comments
Open

support fixes as individual analyzers #78

stamblerre opened this issue Jul 13, 2020 · 7 comments
Labels

Comments

@stamblerre
Copy link
Contributor

This is a follow-up on golang/go#39805. We have integrated gofumpt as an opt-in formatter, but this approach means that gofumpt will be less discoverable and easy-to-use. The best way to integrate gofumpt into gopls 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.

@mvdan
Copy link
Owner

mvdan commented Jul 15, 2020

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.

@stamblerre
Copy link
Contributor Author

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 gofumpt fixes" and then the user would accept the full formatting with that one fix.

@mvdan
Copy link
Owner

mvdan commented Jul 15, 2020

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 go/format.

@stamblerre
Copy link
Contributor Author

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.

@mvdan
Copy link
Owner

mvdan commented Jul 15, 2020

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.

@SVilgelm
Copy link

SVilgelm commented Mar 8, 2023

@mvdan what is the stats on implementing the Analyzer? It would really helps me in my project of custom linter

@SVilgelm
Copy link

@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
https://github.com/sv-tools/gochecker/blob/main/analyzers/gofumpt/analyzer.go

@mvdan mvdan mentioned this issue Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants