mirror of
https://github.com/golang/go
synced 2024-11-19 01:04:40 -07:00
e047ae774b
I felt the burn of my laptop on my legs, spinning away while processing
goimports, and felt that it was time to make goimports great again.
Over the past few years goimports fell into a slow state of disrepair
with too many feature additions and no attention to the performance
death by a thousand cuts. This was particularly terrible on OS X with
its lackluster filesystem buffering.
This CL makes goimports stronger, together with various optimizations
and more visibility into what goimports is doing.
* adds more internal documentation
* avoids scanning $GOPATH for answers when running goimports on a file
under $GOROOT (for Go core hackers)
* don't read all $GOROOT & $GOPATH directories' Go code looking for
their package names until much later. Require the package name of
missing imports to be present in the last two directory path
components. Then only try importing them in order from best to
worst (shortest to longest, as before), so we can stop early.
* when adding imports, add names to imports when the imported package name
doesn't match the baes of its import path. For example:
import foo "example.net/foo/v1"
* don't read all *.go files in a package directory once the first file
in a directory has revealed itself to be a package we're not looking
for. For example, if we're looking for the right "client" for "client.Foo",
we used to consider a directory "bar/client" as a candidate and read
all 50 of its *.go files instead of stopping after its first *.go
file had a "package main" line.
* add some fast paths to remove allocations
* add some fast paths to remove disk I/O when looking up the base
package name of a standard library import (of existing imports in a
file, which are very common)
* adds a special case for import "C", to avoid some disk I/O.
* add a -verbose flag to goimports for debugging
On my Mac laptop with a huge $GOPATH, with a test file like:
package foo
import (
"fmt"
"net/http"
)
/*
*/
import "C"
var _ = cloudbilling.New
var _ = http.NewRequest
var _ = client.New
... this took like 10 seconds before, and now 1.3 seconds. (Still
slow; disk-based caching can come later)
Updates golang/go#16367 (goimports is slow)
Updates golang/go#16384 (refactor TestRename is broken on Windows)
Change-Id: I97e85d3016afc9f2ad5501f97babad30c7989183
Reviewed-on: https://go-review.googlesource.com/24941
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
431 lines
9.2 KiB
Go
431 lines
9.2 KiB
Go
// Copyright 2015 The Go Authors. All rights reserved.
|
|
// Use of this source code is governed by a BSD-style
|
|
// licence that can be found in the LICENSE file.
|
|
|
|
package rename
|
|
|
|
import (
|
|
"fmt"
|
|
"go/build"
|
|
"go/token"
|
|
"io/ioutil"
|
|
"path/filepath"
|
|
"reflect"
|
|
"regexp"
|
|
"runtime"
|
|
"strings"
|
|
"testing"
|
|
|
|
"golang.org/x/tools/go/buildutil"
|
|
)
|
|
|
|
func TestErrors(t *testing.T) {
|
|
tests := []struct {
|
|
ctxt *build.Context
|
|
from, to string
|
|
want string // regexp to match error, or "OK"
|
|
}{
|
|
// Simple example.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; type T int`},
|
|
"bar": {`package bar`},
|
|
"main": {`package main
|
|
|
|
import "foo"
|
|
|
|
var _ foo.T
|
|
`},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: `invalid move destination: bar conflicts with directory .go.src.bar`,
|
|
},
|
|
// Subpackage already exists.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; type T int`},
|
|
"foo/sub": {`package sub`},
|
|
"bar/sub": {`package sub`},
|
|
"main": {`package main
|
|
|
|
import "foo"
|
|
|
|
var _ foo.T
|
|
`},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: "invalid move destination: bar; package or subpackage bar/sub already exists",
|
|
},
|
|
// Invalid base name.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; type T int`},
|
|
"main": {`package main
|
|
|
|
import "foo"
|
|
|
|
var _ foo.T
|
|
`},
|
|
}),
|
|
from: "foo", to: "bar-v2.0",
|
|
want: "invalid move destination: bar-v2.0; gomvpkg does not " +
|
|
"support move destinations whose base names are not valid " +
|
|
"go identifiers",
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
ctxt := test.ctxt
|
|
|
|
got := make(map[string]string)
|
|
writeFile = func(filename string, content []byte) error {
|
|
got[filename] = string(content)
|
|
return nil
|
|
}
|
|
moveDirectory = func(from, to string) error {
|
|
for path, contents := range got {
|
|
if strings.HasPrefix(path, from) {
|
|
newPath := strings.Replace(path, from, to, 1)
|
|
delete(got, path)
|
|
got[newPath] = contents
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
err := Move(ctxt, test.from, test.to, "")
|
|
prefix := fmt.Sprintf("-from %q -to %q", test.from, test.to)
|
|
if err == nil {
|
|
t.Errorf("%s: nil error. Expected error: %s", prefix, test.want)
|
|
continue
|
|
}
|
|
matched, err2 := regexp.MatchString(test.want, err.Error())
|
|
if err2 != nil {
|
|
t.Errorf("regexp.MatchString failed %s", err2)
|
|
continue
|
|
}
|
|
if !matched {
|
|
t.Errorf("%s: conflict does not match expectation:\n"+
|
|
"Error: %q\n"+
|
|
"Pattern: %q",
|
|
prefix, err.Error(), test.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestMoves(t *testing.T) {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("broken on Windows; see golang.org/issue/16384")
|
|
}
|
|
tests := []struct {
|
|
ctxt *build.Context
|
|
from, to string
|
|
want map[string]string
|
|
wantWarnings []string
|
|
}{
|
|
// Simple example.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; type T int`},
|
|
"main": {`package main
|
|
|
|
import "foo"
|
|
|
|
var _ foo.T
|
|
`},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{
|
|
"/go/src/main/0.go": `package main
|
|
|
|
import "bar"
|
|
|
|
var _ bar.T
|
|
`,
|
|
"/go/src/bar/0.go": `package bar
|
|
|
|
type T int
|
|
`,
|
|
},
|
|
},
|
|
|
|
// Example with subpackage.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; type T int`},
|
|
"foo/sub": {`package sub; type T int`},
|
|
"main": {`package main
|
|
|
|
import "foo"
|
|
import "foo/sub"
|
|
|
|
var _ foo.T
|
|
var _ sub.T
|
|
`},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{
|
|
"/go/src/main/0.go": `package main
|
|
|
|
import "bar"
|
|
import "bar/sub"
|
|
|
|
var _ bar.T
|
|
var _ sub.T
|
|
`,
|
|
"/go/src/bar/0.go": `package bar
|
|
|
|
type T int
|
|
`,
|
|
"/go/src/bar/sub/0.go": `package sub; type T int`,
|
|
},
|
|
},
|
|
|
|
// References into subpackages
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"foo": {`package foo; import "foo/a"; var _ a.T`},
|
|
"foo/a": {`package a; type T int`},
|
|
"foo/b": {`package b; import "foo/a"; var _ a.T`},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{
|
|
"/go/src/bar/0.go": `package bar
|
|
|
|
import "bar/a"
|
|
|
|
var _ a.T
|
|
`,
|
|
"/go/src/bar/a/0.go": `package a; type T int`,
|
|
"/go/src/bar/b/0.go": `package b
|
|
|
|
import "bar/a"
|
|
|
|
var _ a.T
|
|
`,
|
|
},
|
|
},
|
|
|
|
// External test packages
|
|
{
|
|
ctxt: buildutil.FakeContext(map[string]map[string]string{
|
|
"foo": {
|
|
"0.go": `package foo; type T int`,
|
|
"0_test.go": `package foo_test; import "foo"; var _ foo.T`,
|
|
},
|
|
"baz": {
|
|
"0_test.go": `package baz_test; import "foo"; var _ foo.T`,
|
|
},
|
|
}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{
|
|
"/go/src/bar/0.go": `package bar
|
|
|
|
type T int
|
|
`,
|
|
"/go/src/bar/0_test.go": `package bar_test
|
|
|
|
import "bar"
|
|
|
|
var _ bar.T
|
|
`,
|
|
"/go/src/baz/0_test.go": `package baz_test
|
|
|
|
import "bar"
|
|
|
|
var _ bar.T
|
|
`,
|
|
},
|
|
},
|
|
// package import comments
|
|
{
|
|
ctxt: fakeContext(map[string][]string{"foo": {`package foo // import "baz"`}}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{"/go/src/bar/0.go": `package bar // import "bar"
|
|
`},
|
|
},
|
|
{
|
|
ctxt: fakeContext(map[string][]string{"foo": {`package foo /* import "baz" */`}}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{"/go/src/bar/0.go": `package bar /* import "bar" */
|
|
`},
|
|
},
|
|
{
|
|
ctxt: fakeContext(map[string][]string{"foo": {`package foo // import "baz"`}}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{"/go/src/bar/0.go": `package bar // import "bar"
|
|
`},
|
|
},
|
|
{
|
|
ctxt: fakeContext(map[string][]string{"foo": {`package foo
|
|
// import " this is not an import comment`}}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{"/go/src/bar/0.go": `package bar
|
|
|
|
// import " this is not an import comment
|
|
`},
|
|
},
|
|
{
|
|
ctxt: fakeContext(map[string][]string{"foo": {`package foo
|
|
/* import " this is not an import comment */`}}),
|
|
from: "foo", to: "bar",
|
|
want: map[string]string{"/go/src/bar/0.go": `package bar
|
|
|
|
/* import " this is not an import comment */
|
|
`},
|
|
},
|
|
// Import name conflict generates a warning, not an error.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"x": {},
|
|
"a": {`package a; type A int`},
|
|
"b": {`package b; type B int`},
|
|
"conflict": {`package conflict
|
|
|
|
import "a"
|
|
import "b"
|
|
var _ a.A
|
|
var _ b.B
|
|
`},
|
|
"ok": {`package ok
|
|
import "b"
|
|
var _ b.B
|
|
`},
|
|
}),
|
|
from: "b", to: "x/a",
|
|
want: map[string]string{
|
|
"/go/src/a/0.go": `package a; type A int`,
|
|
"/go/src/ok/0.go": `package ok
|
|
|
|
import "x/a"
|
|
|
|
var _ a.B
|
|
`,
|
|
"/go/src/conflict/0.go": `package conflict
|
|
|
|
import "a"
|
|
import "x/a"
|
|
|
|
var _ a.A
|
|
var _ b.B
|
|
`,
|
|
"/go/src/x/a/0.go": `package a
|
|
|
|
type B int
|
|
`,
|
|
},
|
|
wantWarnings: []string{
|
|
`/go/src/conflict/0.go:4:8: renaming this imported package name "b" to "a"`,
|
|
`/go/src/conflict/0.go:3:8: conflicts with imported package name in same block`,
|
|
`/go/src/conflict/0.go:3:8: skipping update of this file`,
|
|
},
|
|
},
|
|
// Rename with same base name.
|
|
{
|
|
ctxt: fakeContext(map[string][]string{
|
|
"x": {},
|
|
"y": {},
|
|
"x/foo": {`package foo
|
|
|
|
type T int
|
|
`},
|
|
"main": {`package main; import "x/foo"; var _ foo.T`},
|
|
}),
|
|
from: "x/foo", to: "y/foo",
|
|
want: map[string]string{
|
|
"/go/src/y/foo/0.go": `package foo
|
|
|
|
type T int
|
|
`,
|
|
"/go/src/main/0.go": `package main
|
|
|
|
import "y/foo"
|
|
|
|
var _ foo.T
|
|
`,
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
ctxt := test.ctxt
|
|
|
|
got := make(map[string]string)
|
|
// Populate got with starting file set. rewriteFile and moveDirectory
|
|
// will mutate got to produce resulting file set.
|
|
buildutil.ForEachPackage(ctxt, func(importPath string, err error) {
|
|
if err != nil {
|
|
return
|
|
}
|
|
path := filepath.Join("/go/src", importPath, "0.go")
|
|
if !buildutil.FileExists(ctxt, path) {
|
|
return
|
|
}
|
|
f, err := ctxt.OpenFile(path)
|
|
if err != nil {
|
|
t.Errorf("unexpected error opening file: %s", err)
|
|
return
|
|
}
|
|
bytes, err := ioutil.ReadAll(f)
|
|
f.Close()
|
|
if err != nil {
|
|
t.Errorf("unexpected error reading file: %s", err)
|
|
return
|
|
}
|
|
got[path] = string(bytes)
|
|
})
|
|
var warnings []string
|
|
reportError = func(posn token.Position, message string) {
|
|
warnings = append(warnings, posn.String()+": "+message)
|
|
}
|
|
writeFile = func(filename string, content []byte) error {
|
|
got[filename] = string(content)
|
|
return nil
|
|
}
|
|
moveDirectory = func(from, to string) error {
|
|
for path, contents := range got {
|
|
if strings.HasPrefix(path, from) {
|
|
newPath := strings.Replace(path, from, to, 1)
|
|
delete(got, path)
|
|
got[newPath] = contents
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
err := Move(ctxt, test.from, test.to, "")
|
|
prefix := fmt.Sprintf("-from %q -to %q", test.from, test.to)
|
|
if err != nil {
|
|
t.Errorf("%s: unexpected error: %s", prefix, err)
|
|
continue
|
|
}
|
|
|
|
if !reflect.DeepEqual(warnings, test.wantWarnings) {
|
|
t.Errorf("%s: unexpected warnings:\n%s\nwant:\n%s",
|
|
prefix,
|
|
strings.Join(warnings, "\n"),
|
|
strings.Join(test.wantWarnings, "\n"))
|
|
}
|
|
|
|
for file, wantContent := range test.want {
|
|
k := filepath.FromSlash(file)
|
|
gotContent, ok := got[k]
|
|
delete(got, k)
|
|
if !ok {
|
|
// TODO(matloob): some testcases might have files that won't be
|
|
// rewritten
|
|
t.Errorf("%s: file %s not rewritten", prefix, file)
|
|
continue
|
|
}
|
|
if gotContent != wantContent {
|
|
t.Errorf("%s: rewritten file %s does not match expectation; got <<<%s>>>\n"+
|
|
"want <<<%s>>>", prefix, file, gotContent, wantContent)
|
|
}
|
|
}
|
|
// got should now be empty
|
|
for file := range got {
|
|
t.Errorf("%s: unexpected rewrite of file %s", prefix, file)
|
|
}
|
|
}
|
|
}
|