x/tools/cmd/goimports: support repairing import grouping/ordering

[vasi-stripe]

I'm working on a tool to fix up non-conventional or incorrect imports grouping/ordering, but I'd much rather have this be part of goimports. Would it be welcome?

What goimports does now

When goimports adds or removes an import, it heuristically attempts to find a good place for it. So if I add an os.Open() call, and my imports already look like this:

import (
	"context"
	"testing"

	"github.com/Sirupsen/logrus"
)

Then goimports will put "os" in the proper spot, beween "context" and "testing". That's great!

What goimports does not do

If my imports are not grouped and sorted in the conventional way, goimports will not usually fix the problem. For example, if I was using the old context package, my imports might look like this:

import (
	"testing"

	"github.com/Sirupsen/logrus"
	"golang.org/x/net/context"
)

When I edit the context import to the new package, "context", I have this:

import (
	"testing"

	"github.com/Sirupsen/logrus"
	"context"
)

Running goimports will notice that logrus and context don't make a proper group, so it will separate them:

import (
	"testing"

	"context"

	"github.com/Sirupsen/logrus"
)

But this isn't what we really want. Ideally, it would just re-group and re-order all the imports, to yield:

import (
	"context"
	"testing"

	"github.com/Sirupsen/logrus"
)

Work so far

I have a tool that will globally reorder imports according to the grouping rules, including the associated doc-comments; see below. I could also develop similar funcitonality inside goimports.

My tool is not nearly perfect: It doesn't handle multiple import declarations; it doesn't know about import "C". It's not even clear what should happen to random non-doc-comments inside the import block, eg:

import (
	"github.com/Sirupsen/logrus"

	// When we re-order, what happens to this comment?

	"testing"
)

Automatically grouping and ordering would also be a behavioral change for goimports, so it would have to be behind a flag for now.

[bradfitz]

Improving goimports is welcome.

[meggamind]

Hey can I please pick this issue ? I would need a bit guidance though.

[vasi-stripe]

Here's what I did before: https://github.com/vasi-stripe/gogroup . Note that this is not exactly what we want in goimports, it still has a number of issues.

[vasi-stripe]

So it turns out that goimports already does this, but only in limited cases. If a file is missing an import, and has multiple import decls, they get all merged together and then sorted. The merging seems to lose associated comments, but otherwise this should be possible to repurpose.

See https://github.com/golang/tools/blob/master/go/ast/astutil/imports.go#L151-L173

[wedneyyuri]

Hi @vasi-stripe, in my opinion the goimports should have an option to remove all blank spaces before formatting. Today, if I have imports unordered and separated by spaces the tool doesn't work as expected.

Example (goimports will not do anything):

import (
	"strings"

	"github.com/pkg/errors"

	"net/http"
)

Expected:

import (		
	"net/http"
        "strings"
        "github.com/pkg/errors"
)

[bradfitz]

@wedneyyuri, I disagree with two things there:

(1) "goimports should have an option". We don't like options in code formatters.
(2) that you expect no blank line between the standard library imports and github stuff. goimports is opinionated in that regard.

I agree that the original bug report at the top is a bug, though. It should try to pack things nicely.