From a2dc47679d30b6c496245bafc6a166b46c5fe318 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 24 Oct 2018 15:31:02 +0000 Subject: [PATCH] Revert "imports: support repairing import grouping/ordering" This reverts commit CL 116795 12a7c317e894c0a622b5ed27f0a102fcdd56d1ad. Reason for revert: mangles comments in imports. See comments on issue golang/go#20818. Updates golang/go#20818 Change-Id: Iff82f8dc310dceb982b48d82b26176ea279fef10 Reviewed-on: https://go-review.googlesource.com/c/144339 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Josh Bleecher Snyder --- imports/fix_test.go | 152 ++--------------------------------------- imports/sortimports.go | 14 +++- 2 files changed, 19 insertions(+), 147 deletions(-) diff --git a/imports/fix_test.go b/imports/fix_test.go index bb3d158b8c4..ede39c98949 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -514,6 +514,7 @@ c = fmt.Printf import ( "fmt" + "gu" "github.com/foo/bar" @@ -604,11 +605,15 @@ var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_bar.A, B import ( "fmt" - "io" - "strings" renamed_bar "github.com/foo/bar" + + "io" + . "github.com/foo/baz" + + "strings" + _ "github.com/foo/qux" ) @@ -1044,149 +1049,6 @@ var _ = fmt.Printf `, }, - // golang.org/issue/20818 - { - name: "sort_all_groups", - in: `package p -import ( - "testing" - - "github.com/Sirupsen/logrus" - "context" -) - -var _, _, _ = testing.T, logrus.Entry, context.Context -`, - out: `package p - -import ( - "context" - "testing" - - "github.com/Sirupsen/logrus" -) - -var _, _, _ = testing.T, logrus.Entry, context.Context -`, - }, - - // golang.org/issue/20818 - { - name: "sort_many_groups", - in: `package p -import ( - "testing" - "k8s.io/apimachinery/pkg/api/meta" - - "fmt" - "github.com/pkg/errors" - - "golang.org/x/tools/cover" - - "github.com/sirupsen/logrus" - "context" -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -`, - out: `package p - -import ( - "context" - "fmt" - "testing" - - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/meta" -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -`, - }, - - // golang.org/issue/20818 - { - name: "sort_all_groups_with_blanks_and_comments", - in: `package p -import ( - - - "testing" - "k8s.io/apimachinery/pkg/api/meta" - - // a comment for the "fmt" package (#26921: they are broken, should be fixed) - "fmt" - "github.com/pkg/errors" // some comment - - - "golang.org/x/tools/cover" - - "github.com/sirupsen/logrus" - "context" - - -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -`, - out: `package p - -import ( - "context" - "fmt" - "testing" - - "github.com/pkg/errors" // some comment - "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/meta" // a comment for the "fmt" package (#26921: they are broken, should be fixed) -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -`, - }, - - { - name: "sort_all_groups_with_local_packages", - in: `package p -import ( - "local/foo" - "testing" - "k8s.io/apimachinery/pkg/api/meta" - - "fmt" - "github.com/pkg/errors" - - "github.com/local/bar" - "golang.org/x/tools/cover" - - "github.com/sirupsen/logrus" - "context" -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -var _, _ = foo.Foo, bar.Bar -`, - out: `package p - -import ( - "context" - "fmt" - "testing" - - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/meta" - - "github.com/local/bar" - "local/foo" -) - -var _, _, _, _, _, _ = testing.T, logrus.Entry, context.Context, meta.AnyGroup, fmt.Printf, errors.Frame -var _, _ = foo.Foo, bar.Bar -`, - }, - { name: "cryptorand_preferred_easy_possible", in: `package p diff --git a/imports/sortimports.go b/imports/sortimports.go index 1fc7d8cd6bc..f3dd56c7a6f 100644 --- a/imports/sortimports.go +++ b/imports/sortimports.go @@ -34,8 +34,18 @@ func sortImports(fset *token.FileSet, f *ast.File) { continue } - // Sort and regroup all imports. - sortSpecs(fset, f, d.Specs) + // Identify and sort runs of specs on successive lines. + i := 0 + specs := d.Specs[:0] + for j, s := range d.Specs { + if j > i && fset.Position(s.Pos()).Line > 1+fset.Position(d.Specs[j-1].End()).Line { + // j begins a new run. End this one. + specs = append(specs, sortSpecs(fset, f, d.Specs[i:j])...) + i = j + } + } + specs = append(specs, sortSpecs(fset, f, d.Specs[i:])...) + d.Specs = specs // Deduping can leave a blank line before the rparen; clean that up. if len(d.Specs) > 0 {