1
0
mirror of https://github.com/golang/go synced 2024-11-18 12:44:49 -07:00

imports: drop anything after a non identifier rune in package names

A package name cannot contain a '.' anyway, so this is a mostly likely
internal package name in the presence of a '.' in the import path.
This stops goimports from adding a local alias in places where it is not
needed, for instance in gopkg.in conventions.

Fixes golang/go#29556

Change-Id: I0ab11f2852d7f1dae14457995692760077201c8e
Reviewed-on: https://go-review.googlesource.com/c/157357
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Ian Cottrell 2019-01-10 12:51:27 -05:00
parent 1c3581914d
commit cb89afadce
2 changed files with 48 additions and 13 deletions

View File

@ -23,6 +23,8 @@ import (
"strings"
"sync"
"time"
"unicode"
"unicode/utf8"
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages"
@ -289,7 +291,7 @@ func (p *pass) importIdentifier(imp *importInfo) string {
if known != nil && known.name != "" {
return known.name
}
return importPathToNameBasic(imp.importPath, p.srcDir)
return importPathToAssumedName(imp.importPath)
}
// load reads in everything necessary to run a pass, and reports whether the
@ -389,7 +391,7 @@ func (p *pass) fix() bool {
}
path := strings.Trim(imp.Path.Value, `""`)
ident := p.importIdentifier(&importInfo{importPath: path})
if ident != importPathToNameBasic(path, p.srcDir) {
if ident != importPathToAssumedName(path) {
imp.Name = &ast.Ident{Name: ident, NamePos: imp.Pos()}
}
}
@ -648,7 +650,7 @@ func (r *goPackagesResolver) loadPackageNames(importPaths []string, srcDir strin
if _, ok := names[path]; ok {
continue
}
names[path] = importPathToNameBasic(path, srcDir)
names[path] = importPathToAssumedName(path)
}
return names, nil
@ -741,18 +743,36 @@ func addExternalCandidates(pass *pass, refs references, filename string) error {
return firstErr
}
// importPathToNameBasic assumes the package name is the base of import path,
// except that if the path ends in foo/vN, it assumes the package name is foo.
func importPathToNameBasic(importPath, srcDir string) (packageName string) {
// notIdentifier reports whether ch is an invalid identifier character.
func notIdentifier(ch rune) bool {
return !('a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z' ||
'0' <= ch && ch <= '9' ||
ch == '_' ||
ch >= utf8.RuneSelf && (unicode.IsLetter(ch) || unicode.IsDigit(ch)))
}
// importPathToAssumedName returns the assumed package name of an import path.
// It does this using only string parsing of the import path.
// It picks the last element of the path that does not look like a major
// version, and then picks the valid identifier off the start of that element.
// It is used to determine if a local rename should be added to an import for
// clarity.
// This function could be moved to a standard package and exported if we want
// for use in other tools.
func importPathToAssumedName(importPath string) string {
base := path.Base(importPath)
if strings.HasPrefix(base, "v") {
if _, err := strconv.Atoi(base[1:]); err == nil {
dir := path.Dir(importPath)
if dir != "." {
return path.Base(dir)
base = path.Base(dir)
}
}
}
base = strings.TrimPrefix(base, "go-")
if i := strings.IndexFunc(base, notIdentifier); i >= 0 {
base = base[:i]
}
return base
}

View File

@ -1373,13 +1373,13 @@ var (
// Test for correctly identifying the name of a vendored package when it
// differs from its directory name. In this test, the import line
// "mypkg.com/mypkg.v1" would be removed if goimports wasn't able to detect
// "mypkg.com/mypkg_v1" would be removed if goimports wasn't able to detect
// that the package name is "mypkg".
func TestVendorPackage(t *testing.T) {
const input = `package p
import (
"fmt"
"mypkg.com/mypkg.v1"
"mypkg.com/mypkg_v1"
)
var _, _ = fmt.Print, mypkg.Foo
`
@ -1389,7 +1389,7 @@ var _, _ = fmt.Print, mypkg.Foo
import (
"fmt"
mypkg "mypkg.com/mypkg.v1"
mypkg "mypkg.com/mypkg_v1"
)
var _, _ = fmt.Print, mypkg.Foo
@ -1400,7 +1400,7 @@ var _, _ = fmt.Print, mypkg.Foo
module: packagestest.Module{
Name: "mypkg.com/outpkg",
Files: fm{
"vendor/mypkg.com/mypkg.v1/f.go": "package mypkg\nvar Foo = 123\n",
"vendor/mypkg.com/mypkg_v1/f.go": "package mypkg\nvar Foo = 123\n",
"toformat.go": input,
},
},
@ -1626,28 +1626,43 @@ func TestAddNameToMismatchedImport(t *testing.T) {
const input = `package main
import (
"foo.com/a.thing"
"foo.com/surprise"
"foo.com/v1"
"foo.com/other/v2"
"foo.com/other/v3"
"foo.com/go-thing"
"foo.com/go-wrong"
)
var _, _ = bar.X, v1.Y`
var _ = []interface{}{bar.X, v1.Y, a.A, v2.V2, other.V3, thing.Thing, gow.Wrong}`
const want = `package main
import (
"foo.com/a.thing"
"foo.com/go-thing"
gow "foo.com/go-wrong"
v2 "foo.com/other/v2"
"foo.com/other/v3"
bar "foo.com/surprise"
v1 "foo.com/v1"
)
var _, _ = bar.X, v1.Y
var _ = []interface{}{bar.X, v1.Y, a.A, v2.V2, other.V3, thing.Thing, gow.Wrong}
`
testConfig{
module: packagestest.Module{
Name: "foo.com",
Files: fm{
"a.thing/a.go": "package a \n const A = 1",
"surprise/x.go": "package bar \n const X = 1",
"v1/x.go": "package v1 \n const Y = 1",
"other/v2/y.go": "package v2 \n const V2 = 1",
"other/v3/z.go": "package other \n const V3 = 1",
"go-thing/b.go": "package thing \n const Thing = 1",
"go-wrong/b.go": "package gow \n const Wrong = 1",
"test/t.go": input,
},
},