mirror of
https://github.com/golang/go
synced 2024-11-22 15:54:52 -07:00
types2: disambiguate package qualifiers in error messages
This is a port of the go/types CL https://golang.org/cl/313035 with minor adjustments (use of package syntax rather than go/ast). Change-Id: I89410efb3d27be85fdbe827f966c2c91ee5693b5 Reviewed-on: https://go-review.googlesource.com/c/go/+/314410 Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
This commit is contained in:
parent
ea65a12f89
commit
42812a2fee
@ -87,7 +87,16 @@ type Checker struct {
|
||||
impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
|
||||
posMap map[*Interface][]syntax.Pos // maps interface types to lists of embedded interface positions
|
||||
typMap map[string]*Named // maps an instantiated named type hash to a *Named type
|
||||
pkgCnt map[string]int // counts number of imported packages with a given name (for better error messages)
|
||||
|
||||
// pkgPathMap maps package names to the set of distinct import paths we've
|
||||
// seen for that name, anywhere in the import graph. It is used for
|
||||
// disambiguating package names in error messages.
|
||||
//
|
||||
// pkgPathMap is allocated lazily, so that we don't pay the price of building
|
||||
// it on the happy path. seenPkgMap tracks the packages that we've already
|
||||
// walked.
|
||||
pkgPathMap map[string]map[string]bool
|
||||
seenPkgMap map[*Package]bool
|
||||
|
||||
// information collected during type-checking of a set of package files
|
||||
// (initialized by Files, valid only for the duration of check.Files;
|
||||
@ -181,7 +190,6 @@ func NewChecker(conf *Config, pkg *Package, info *Info) *Checker {
|
||||
impMap: make(map[importKey]*Package),
|
||||
posMap: make(map[*Interface][]syntax.Pos),
|
||||
typMap: make(map[string]*Named),
|
||||
pkgCnt: make(map[string]int),
|
||||
}
|
||||
}
|
||||
|
||||
@ -271,9 +279,6 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) {
|
||||
print("== unusedImports ==")
|
||||
check.unusedImports()
|
||||
}
|
||||
// no longer needed - release memory
|
||||
check.imports = nil
|
||||
check.dotImportMap = nil
|
||||
|
||||
print("== recordUntyped ==")
|
||||
check.recordUntyped()
|
||||
@ -285,6 +290,12 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) {
|
||||
|
||||
check.pkg.complete = true
|
||||
|
||||
// no longer needed - release memory
|
||||
check.imports = nil
|
||||
check.dotImportMap = nil
|
||||
check.pkgPathMap = nil
|
||||
check.seenPkgMap = nil
|
||||
|
||||
// TODO(gri) There's more memory we should release at this point.
|
||||
|
||||
return
|
||||
|
@ -108,8 +108,13 @@ func sprintf(qf Qualifier, format string, args ...interface{}) string {
|
||||
func (check *Checker) qualifier(pkg *Package) string {
|
||||
// Qualify the package unless it's the package being type-checked.
|
||||
if pkg != check.pkg {
|
||||
if check.pkgPathMap == nil {
|
||||
check.pkgPathMap = make(map[string]map[string]bool)
|
||||
check.seenPkgMap = make(map[*Package]bool)
|
||||
check.markImports(pkg)
|
||||
}
|
||||
// If the same package name was used by multiple packages, display the full path.
|
||||
if check.pkgCnt[pkg.name] > 1 {
|
||||
if len(check.pkgPathMap[pkg.name]) > 1 {
|
||||
return strconv.Quote(pkg.path)
|
||||
}
|
||||
return pkg.name
|
||||
@ -117,6 +122,26 @@ func (check *Checker) qualifier(pkg *Package) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// markImports recursively walks pkg and its imports, to record unique import
|
||||
// paths in pkgPathMap.
|
||||
func (check *Checker) markImports(pkg *Package) {
|
||||
if check.seenPkgMap[pkg] {
|
||||
return
|
||||
}
|
||||
check.seenPkgMap[pkg] = true
|
||||
|
||||
forName, ok := check.pkgPathMap[pkg.name]
|
||||
if !ok {
|
||||
forName = make(map[string]bool)
|
||||
check.pkgPathMap[pkg.name] = forName
|
||||
}
|
||||
forName[pkg.path] = true
|
||||
|
||||
for _, imp := range pkg.imports {
|
||||
check.markImports(imp)
|
||||
}
|
||||
}
|
||||
|
||||
func (check *Checker) sprintf(format string, args ...interface{}) string {
|
||||
return sprintf(check.qualifier, format, args...)
|
||||
}
|
||||
|
@ -474,7 +474,7 @@ func TestIssue34151(t *testing.T) {
|
||||
}
|
||||
|
||||
bast := mustParse(t, bsrc)
|
||||
conf := Config{Importer: importHelper{a}}
|
||||
conf := Config{Importer: importHelper{pkg: a}}
|
||||
b, err := conf.Check(bast.PkgName.Value, []*syntax.File{bast}, nil)
|
||||
if err != nil {
|
||||
t.Errorf("package %s failed to typecheck: %v", b.Name(), err)
|
||||
@ -482,14 +482,18 @@ func TestIssue34151(t *testing.T) {
|
||||
}
|
||||
|
||||
type importHelper struct {
|
||||
pkg *Package
|
||||
pkg *Package
|
||||
fallback Importer
|
||||
}
|
||||
|
||||
func (h importHelper) Import(path string) (*Package, error) {
|
||||
if path != h.pkg.Path() {
|
||||
if path == h.pkg.Path() {
|
||||
return h.pkg, nil
|
||||
}
|
||||
if h.fallback == nil {
|
||||
return nil, fmt.Errorf("got package path %q; want %q", path, h.pkg.Path())
|
||||
}
|
||||
return h.pkg, nil
|
||||
return h.fallback.Import(path)
|
||||
}
|
||||
|
||||
// TestIssue34921 verifies that we don't update an imported type's underlying
|
||||
@ -513,7 +517,7 @@ func TestIssue34921(t *testing.T) {
|
||||
var pkg *Package
|
||||
for _, src := range sources {
|
||||
f := mustParse(t, src)
|
||||
conf := Config{Importer: importHelper{pkg}}
|
||||
conf := Config{Importer: importHelper{pkg: pkg}}
|
||||
res, err := conf.Check(f.PkgName.Value, []*syntax.File{f}, nil)
|
||||
if err != nil {
|
||||
t.Errorf("%q failed to typecheck: %v", src, err)
|
||||
@ -568,3 +572,41 @@ func TestIssue44515(t *testing.T) {
|
||||
t.Errorf("got %q; want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssue43124(t *testing.T) {
|
||||
// All involved packages have the same name (template). Error messages should
|
||||
// disambiguate between text/template and html/template by printing the full
|
||||
// path.
|
||||
const (
|
||||
asrc = `package a; import "text/template"; func F(template.Template) {}; func G(int) {}`
|
||||
bsrc = `package b; import ("a"; "html/template"); func _() { a.F(template.Template{}) }`
|
||||
csrc = `package c; import ("a"; "html/template"); func _() { a.G(template.Template{}) }`
|
||||
)
|
||||
|
||||
a, err := pkgFor("a", asrc, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("package a failed to typecheck: %v", err)
|
||||
}
|
||||
conf := Config{Importer: importHelper{pkg: a, fallback: defaultImporter()}}
|
||||
|
||||
// Packages should be fully qualified when there is ambiguity within the
|
||||
// error string itself.
|
||||
bast := mustParse(t, bsrc)
|
||||
_, err = conf.Check(bast.PkgName.Value, []*syntax.File{bast}, nil)
|
||||
if err == nil {
|
||||
t.Fatal("package b had no errors")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "text/template") || !strings.Contains(err.Error(), "html/template") {
|
||||
t.Errorf("type checking error for b does not disambiguate package template: %q", err)
|
||||
}
|
||||
|
||||
// ...and also when there is any ambiguity in reachable packages.
|
||||
cast := mustParse(t, csrc)
|
||||
_, err = conf.Check(cast.PkgName.Value, []*syntax.File{cast}, nil)
|
||||
if err == nil {
|
||||
t.Fatal("package c had no errors")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "html/template") {
|
||||
t.Errorf("type checking error for c does not disambiguate package template: %q", err)
|
||||
}
|
||||
}
|
||||
|
@ -179,7 +179,11 @@ func (check *Checker) importPackage(pos syntax.Pos, path, dir string) *Package {
|
||||
// package should be complete or marked fake, but be cautious
|
||||
if imp.complete || imp.fake {
|
||||
check.impMap[key] = imp
|
||||
check.pkgCnt[imp.name]++
|
||||
// Once we've formatted an error message once, keep the pkgPathMap
|
||||
// up-to-date on subsequent imports.
|
||||
if check.pkgPathMap != nil {
|
||||
check.markImports(imp)
|
||||
}
|
||||
return imp
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user