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

Internal imports mixed with std imports #117

Closed
lewislbr opened this issue Mar 25, 2021 · 5 comments · Fixed by #199
Closed

Internal imports mixed with std imports #117

lewislbr opened this issue Mar 25, 2021 · 5 comments · Fixed by #199

Comments

@lewislbr
Copy link

lewislbr commented Mar 25, 2021

Hello, thanks for this package, this is what the go fmt should be! 😬

I've recently installed and ran into this issue:

The internal imports are treated as if they were standard library imports and are grouped together alphabetically.

gofumpt version: v0.1.1
Go version: go1.16.2 darwin/amd64

Expected result:

import (
	"stdpackage"

	"internal/foo"
	
	"third-party-package/baz"
)

Actual result:

import (
        "internal/foo"
	"stdpackage"
	
	"third-party-package/baz"
)

Video:

Screen.Recording.2021-03-25.at.19.21.48.mov

Test repo:

gofumpt-test.zip

Additionally, third-party imports are not grouped if they have at least a blank line between them:

Screen.Recording.2021-03-25.at.19.40.00.mov

Let me know if it can be replicated and if I can help.

@lewislbr lewislbr changed the title Internal import mixed with std imports Internal imports mixed with std imports Mar 25, 2021
@mvdan
Copy link
Owner

mvdan commented Apr 5, 2021

This is behaving as intended; any import paths that don't start with a domain name are reserved by the Go toolchain, and as such are considered standard library. See golang/go#32819.

The long-term fix here is to use a proper module path prefix, like foo.bar/internal/foo. It doesn't have to be a real domain, you could use foo.local. Or, for quick things, see golang/go#37641 which explicitly assigns example/ and test/ to third-party modules.

If your objective is to avoid a module path prefix at all costs, then I think your only option is golang/go#44649, though that's a GOPATH transition helper and I'm not sure if it would stick around forever.

@lewislbr
Copy link
Author

lewislbr commented Apr 5, 2021

Ahhh I see. Thanks for the insight.

I see the goimports tool respects the separation from std imports if there is a blank line in between, and even it divides the internal imports (even without a domain) from the third-party ones, so

import (
	"stdpackage"
	
	"third-party-package/baz"
        "internal/foo"
)

becomes

import (
	"stdpackage"

        "internal/foo"
	
	"third-party-package/baz"
)

Would it make sense to replicate that behavior?

@mvdan
Copy link
Owner

mvdan commented Apr 5, 2021

The only reason goimports obeys that is because it doesn't join std imports at all. So it's only better for you in this scenario because it lacks the feature entirely :)

@lewislbr
Copy link
Author

lewislbr commented Apr 5, 2021

Interesting 😬

I guess this can be closed then. Thanks!

@mvdan
Copy link
Owner

mvdan commented Feb 5, 2022

FYI, see #187 (comment); I'm currently thinking we could do better for everyone by looking at the main module's path.

mvdan added a commit that referenced this issue Feb 21, 2022
Add the feature, test it, and document it in the README.
It's enough to mention it in the FAQ, as I expect that most users will
never run into this edge case. And those few that may run into it will
hopefully never need to know how it works or why.

Note that this adds a new option to pass the info from go.mod,
much like LangVersion already did.

Fixes #22.
Fixes #117.
Fixes #120.
Fixes #187.
mvdan added a commit that referenced this issue Feb 21, 2022
Add the feature, test it, and document it in the README.
It's enough to mention it in the FAQ, as I expect that most users will
never run into this edge case. And those few that may run into it will
hopefully never need to know how it works or why.

Note that this adds a new option to pass the info from go.mod,
much like LangVersion already did.

Fixes #22.
Fixes #117.
Fixes #120.
Fixes #187.
mvdan added a commit that referenced this issue Feb 22, 2022
Add the feature, test it, and document it in the README.
It's enough to mention it in the FAQ, as I expect that most users will
never run into this edge case. And those few that may run into it will
hopefully never need to know how it works or why.

Note that this adds a new option to pass the info from go.mod,
much like LangVersion already did.

Fixes #22.
Fixes #117.
Fixes #120.
Fixes #187.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants