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

Group imports as std, external, internal #37

Closed
Southclaws opened this issue Oct 22, 2019 · 5 comments
Closed

Group imports as std, external, internal #37

Southclaws opened this issue Oct 22, 2019 · 5 comments

Comments

@Southclaws
Copy link

Southclaws commented Oct 22, 2019

A convention I and coworkers often follow is a further separation of imports by relationship to the current project. So anything from the app/library itself is grouped separately so you can easily see all the external dependencies without them getting mixed with internal dependencies.

import (
	"bytes"
	"encoding/json"
	"fmt"
	"io"
	"net/http"

	"github.com/go-chi/chi"
	"github.com/pkg/errors"
	"github.com/lib/pq"

	"github.com/our-company-org/our-project/stuff"
	"github.com/our-company-org/our-project/internal/other-stuff"
	"github.com/our-company-org/our-project/internal/secret/generics"
)

It would be neat for gofumpt to read the current package from go.mod — which in the example above, would be github.com/our-company-org/our-project — then group any package with that prefix at the bottom of the import block.

Maybe this is too specific though, I'm not sure if anyone else does this.

@mvdan
Copy link
Owner

mvdan commented Oct 22, 2019

We already do part of this. We force all standard library imports to be in the first group.

I thought about extending the logic to also enforce that the last group contain all the imports from within the same module. However, I think that would be too risky, for two reasons:

  1. The tool is not module-aware at all, at the moment. Note that you can run gofumpt foo.go, and that file may not belong to any module. There are also other fun problems, like what would happen if one runs cd module1; gofumpt ../module2/foo.go. Would this use module1 or module2? Either way, this would make the tool more complex, and slower, as we'd have to run something like go list -m -json.

  2. I'm not convinced that everyone follows this pattern. It certainly is common, and enforcing it could be good, but I also imagine it's going to make some users upset. I only want to enforce what 99% of users already do, and is considered as part of "idiomatic go" formatting.

@Southclaws
Copy link
Author

Makes sense, I didn't think of the complexities of inside/outside modules and forgot about the lack of module-awareness!

@mvdan mvdan closed this as completed Nov 5, 2019
mvdan added a commit that referenced this issue Mar 18, 2020
If an empty line existed between any std imports, we would leave it
there. Instead, remove it.

To keep the rule from being too aggressive, treat imports that have
explicit names or comments as non-std.

Improves #8.
Updates #37.
@mvdan
Copy link
Owner

mvdan commented Mar 18, 2020

The heuristic here was a bit lax; I've made it a bit more aggressive. See the commit pushed to master just above.

@dnwe
Copy link

dnwe commented Nov 2, 2020

@mvdan FYI I did notice that there is also https://github.com/daixiang0/gci in this space now

@mvdan
Copy link
Owner

mvdan commented Nov 2, 2020

Thanks! This discussion has now moved to #38 for the most part, which is open. I've left a couple of design proposals near the end of the thread.

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

No branches or pull requests

3 participants