-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
x/tools/gopls: organizeImports disabled when gofumpt is used #44136
Comments
I took another look at this, and I think the problem isn't so much that I don't understand why |
The presence of the disabled attribute is a glitch in our JSON serialization, which I believe has been fixed at tip. VS Code, at least, ignores it if the reason is empty. I believe this behavior is gofumpt working as designed. The import path cc @mvdan in case I'm missing something. |
Vim-go also ignores the disabled attribute for now, though I was thinking about changing that. The When configuring, notice the
Adjusting imports honors that, but then I can modify vim-go to adjust imports after formatting as a workaround, though. |
FWIW, I've adjusted vim-go to organize imports after formatting on save so that the local imports will meet user's expectations. |
What @heschik said is right. #32819 is the issue to canonically document that module names without dots in the first path element are reserved. Arguably, tools like You might also be interested in #37641.
|
As far as I can tell there is not much to be done from the gopls side here. In my opinion, if a user chooses to enable gofumpt, gopls should always preserve gofumpt formatting. In that sense, the fact that organize imports is doing its usual grouping before being overriden by formatting might be a bug. (Though not one I would bother fixing.) I'll leave this bug open for further discussion if you like, but to me this seems like something better suited for the gofumpt tracker. |
Fair. I have a workaround implemented in vim-go already (organize imports after formatting). |
I'm surprised you arrived at that conclusion. It is one of the reasons, but not the only one. For example, the issue says:
The issue is also about reserving the paths entirely, so any distinction about the reasons seems unnecessary. It's entirely reasonable for a tool to assume that such a path is not from an external module, given that it's reserved. |
That's of course your decision for vim-go, but it seems a bit confusing from the user's point of view; if they enabled gofumpt, they should get its formatting. I get your workaround as a temporary workaround, but not as a proper long-term solution :) |
Yeah, the issue does say that, but module names without dots are entirely valid and always have been. Yes, it's possible to collide with stdlib, but the idea that module names without dots are reserved seems like a new rule that would catch a lot of people off guard and potentially create a lot of work for Go users unnecessarily.
Is it reserved now, though? I work with modules regularly that don't have dots without any ill effects. Everyone one I've shown that issue to shakes their head when they see it. I'm happy to take this convo to that issue if it's useful for you.
|
Yes, your points seem to refer mostly to that issue and not this one. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I enabled gofumpt and a local imports value for gopls and tried to organize imports with gopls.
What did you expect to see?
The imports organized and the local modules put in their own section.
What did you see instead?
The imports were not organized.
Here are the relevant logs when gofumpt is not enabled (the last log is response to the
codeAction/organizeImports
request):gopls logs
And here they are when gofumpt is enabled (the last log is response to the
codeAction/organizeImports
request):gopls logs
This was originally reported on fatih/vim-go#3151
The text was updated successfully, but these errors were encountered: