cmd/gofmt: formatting should sort and cluster imports in a standard way

[jsgoupil]

What version of Go are you using (go version)?

go version go1.16.5 windows/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

The order of the imports is not something that "matters" to developers, yet the developers will comment during code reviews to order them the right way. Why? There is literally no reason for having them in a specific order other than the "look" of them.

Furthermore, by having them manually fiddled with, it causes avoidable merge issues.

Currently, the imports are ordered but by block only. If I separate my imports in different blocks, they will be all left untouched. It will take the solo import lines and sort them correctly. So there is some kind of logic applied so it "looks" good, but it should go all the way opiniated and not allow the dev to order them the way they want.

What did you expect to see?

import (
	"flag"
	"fmt"
	"log"
	"net/http"
	"os"
	"strings"
	"time"

	_ "github.com/fake-a/package"
	"github.com/fake-b/package"
	fakeZ "github.com/fake-z/package"
}

What did you see instead?

import (
	"fmt"
	"log"

	"flag"
	"net/http"
	"os"
	"strings"
	"time"

	fakeZ "github.com/fake-z/package"

	_ "github.com/fake-a/package"
	"github.com/fake-b/package"
}

[jfesler]

100% agree on the idea that the imports should be forcefully formatted to the One True Way. I don't know that I actually care about what way that is; only that I don't want to argue over it.

This exact issue is why our team has a different formatter, that wraps the go fmt library and then reformats the imports to the team standard.

[apparentlymart]

Based on your code examples, it seems like you're implicitly proposing a particular algorithm but haven't stated exactly what it is. Here's how I understand the algorithm implied by your example; please correct me if this doesn't match what you intended.

• Take the set of all ImportSpec in the file and then categorize them into two disjoint sets: packages from the standard library, and all other packages.
• Generate a single import declaration which groups together all of the import specs. In it:
  • First, write out the packages from the standard library entries in lexical order, one per line with no blank lines.
  • If both sets are non-empty, write one blank line to separate them.
  • Finally, write out the all other packages entries in lexical order, one per line with no blank lines.

I expect that in the above the definition of "in the standard library" is probably the most debatable sub-question, though presumably the Go toolchain already has some sense of that in order to avoid attempting external module package installation for those, and so I'd expect it to follow the same rules.

(FWIW, I don't have a strong opinion on whether or not to do this, just wanted to attempt to clarify what exactly the proposal is.)

[jsgoupil]

I'm the same as @jfesler
I wouldn't infer my algorithm here. Just come up with one and I won't argue. I don't care about the order but only a true forced order.

I chatted a bit on a Gopher slack and people definitely use different libraries to fix this problem. So maybe look at what is being done in these libraries.

[apparentlymart]

Fair enough! I guess then my previous comment is an example of one possible algorithm that fmt could enforce.

Some other questions that come to my mind when I think about this are:

• If there are comments adjacent to or inside any of the existing import declarations, what should happen? Do they get preserved, and if so does the presence of the comments have any impact on the chosen ordering?
• Are the ImportSpecs ordered by the import path or by the package name? Does that decision vary depending on whether there's an explicit package name or not?
• Is "packages belonging to the same module as the current file" another category worthy of having a separately-sorted list of import specifications, distinct from both packages from the standard library and all other packages?

Again, I don't have a strong opinion either way on whether to do this, but it seems like whether it would be accepted depends at least in part on finding reasonable answers to these questions that can produce reasonable results for most existing source files, and so I'm thinking here about the sorts of edge cases I've seen which seem to have motivated "non-standard" import ordering in source files I regularly interact with. Unfortunately the burden of introducing "one true way" is having to decide what that way is, even if you don't actually care very much! (which is also true for me) 😀

[beoran]

It is sometimes necessary to order them "the way the dev wants" in order to make sure the init() functions of the imported packages are called in the right way with the right order.

This is for example needed for Gui libraries in which drawing is only allowed on the main OS thread, and where the GUI llbrary has to call LockOsThread() in init() to be able to execute on that thread, before any other package, especially if that package also uses LocksOsThread().

That's why go fmt currently leaves separate blocks separate, to keep this possible.

[jsgoupil]

@beoran This is quite interesting what you bring up here. I hope this doesn't prevent this issues to go forward anyway.
If you think about it, if you have

import "flag"
import "fmt"

import (
	"log"
)

Then format, you get

import (
	"flag"
	"fmt"
	"log"
)

But if you had ordered "fmt" prior because you "think?!" you have an init() that has to run before flag:

import "fmt"
import "flag"

import (
	"log"
)

You still get

import (
	"flag"
	"fmt"
	"log"
)

So now, being able to order ONLY within an import () block rather than an import line, sounds even more like a hidden feature. And from what I heard (my knowledge of Go is still limited), people say using init() being an anti-pattern.

As mentioned, I'm happy you bring this up so that we are aware of what is going on, but I hope a consensus is reached.
People seem to want this to be fixed, but most do not care on how.

[beoran]

I also don't care how this is solved but there must remain a way to allow manual ordering as well.

And init() can sometiles be an anti-pattern. But init() is essential in some specific cases, such as the example of a GUI or game library, that must lock its goroutine to the primary thread, before main is called.

[ianlancetaylor]

See some previous discussion at #9922 and #18905.