1
0
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:
Robert Griesemer 2021-04-27 12:54:39 -07:00
parent ea65a12f89
commit 42812a2fee
4 changed files with 94 additions and 12 deletions

View File

@ -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

View File

@ -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...)
}

View File

@ -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)
}
}

View File

@ -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
}