]> Cypherpunks.ru repositories - gostls13.git/commitdiff
[dev.regabi] go/types: report unused packages in source order
authorRob Findley <rfindley@google.com>
Thu, 4 Feb 2021 17:24:10 +0000 (12:24 -0500)
committerRobert Findley <rfindley@google.com>
Tue, 9 Feb 2021 03:59:20 +0000 (03:59 +0000)
This is a port of CL 287072 to go/types.

Change-Id: I08f56995f0323c1f238d1b44703a481d393471d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/289720
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/go/types/check.go
src/go/types/resolver.go
src/go/types/testdata/importdecl0/importdecl0b.src
src/go/types/testdata/importdecl1/importdecl1b.src
src/go/types/typexpr.go

index 280792e838ef24bb62849c1b93904dc436770aba..03798587e74fe4aaf68dca36b3714cd114e0e28f 100644 (file)
@@ -69,6 +69,12 @@ type importKey struct {
        path, dir string
 }
 
+// A dotImportKey describes a dot-imported object in the given scope.
+type dotImportKey struct {
+       scope *Scope
+       obj   Object
+}
+
 // A Checker maintains the state of the type checker.
 // It must be created with NewChecker.
 type Checker struct {
@@ -86,8 +92,9 @@ type Checker struct {
        // information collected during type-checking of a set of package files
        // (initialized by Files, valid only for the duration of check.Files;
        // maps and lists are allocated on demand)
-       files            []*ast.File                             // package files
-       unusedDotImports map[*Scope]map[*Package]*ast.ImportSpec // unused dot-imported packages
+       files        []*ast.File               // package files
+       imports      []*PkgName                // list of imported packages
+       dotImportMap map[dotImportKey]*PkgName // maps dot-imported objects to the package they were dot-imported through
 
        firstErr error                 // first error encountered
        methods  map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods
@@ -104,22 +111,6 @@ type Checker struct {
        indent int // indentation for tracing
 }
 
-// addUnusedImport adds the position of a dot-imported package
-// pkg to the map of dot imports for the given file scope.
-func (check *Checker) addUnusedDotImport(scope *Scope, pkg *Package, spec *ast.ImportSpec) {
-       mm := check.unusedDotImports
-       if mm == nil {
-               mm = make(map[*Scope]map[*Package]*ast.ImportSpec)
-               check.unusedDotImports = mm
-       }
-       m := mm[scope]
-       if m == nil {
-               m = make(map[*Package]*ast.ImportSpec)
-               mm[scope] = m
-       }
-       m[pkg] = spec
-}
-
 // addDeclDep adds the dependency edge (check.decl -> to) if check.decl exists
 func (check *Checker) addDeclDep(to Object) {
        from := check.decl
@@ -202,7 +193,8 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Ch
 func (check *Checker) initFiles(files []*ast.File) {
        // start with a clean slate (check.Files may be called multiple times)
        check.files = nil
-       check.unusedDotImports = nil
+       check.imports = nil
+       check.dotImportMap = nil
 
        check.firstErr = nil
        check.methods = nil
@@ -272,10 +264,16 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
        if !check.conf.DisableUnusedImportCheck {
                check.unusedImports()
        }
+       // no longer needed - release memory
+       check.imports = nil
+       check.dotImportMap = nil
 
        check.recordUntyped()
 
        check.pkg.complete = true
+
+       // TODO(rFindley) There's more memory we should release at this point.
+
        return
 }
 
index b637f8b8cacf75793f564660081bc0c741daab1f..cb66871883d907f4c9bcaa117a28e1b47f75f186 100644 (file)
@@ -275,21 +275,26 @@ func (check *Checker) collectObjects() {
                                        }
                                }
 
-                               obj := NewPkgName(d.spec.Pos(), pkg, name, imp)
+                               pkgName := NewPkgName(d.spec.Pos(), pkg, name, imp)
                                if d.spec.Name != nil {
                                        // in a dot-import, the dot represents the package
-                                       check.recordDef(d.spec.Name, obj)
+                                       check.recordDef(d.spec.Name, pkgName)
                                } else {
-                                       check.recordImplicit(d.spec, obj)
+                                       check.recordImplicit(d.spec, pkgName)
                                }
 
                                if path == "C" {
                                        // match cmd/compile (not prescribed by spec)
-                                       obj.used = true
+                                       pkgName.used = true
                                }
 
                                // add import to file scope
+                               check.imports = append(check.imports, pkgName)
                                if name == "." {
+                                       // dot-import
+                                       if check.dotImportMap == nil {
+                                               check.dotImportMap = make(map[dotImportKey]*PkgName)
+                                       }
                                        // merge imported scope with file scope
                                        for _, obj := range imp.scope.elems {
                                                // A package scope may contain non-exported objects,
@@ -303,16 +308,15 @@ func (check *Checker) collectObjects() {
                                                        if alt := fileScope.Insert(obj); alt != nil {
                                                                check.errorf(d.spec.Name, _DuplicateDecl, "%s redeclared in this block", obj.Name())
                                                                check.reportAltDecl(alt)
+                                                       } else {
+                                                               check.dotImportMap[dotImportKey{fileScope, obj}] = pkgName
                                                        }
                                                }
                                        }
-                                       // add position to set of dot-import positions for this file
-                                       // (this is only needed for "imported but not used" errors)
-                                       check.addUnusedDotImport(fileScope, imp, d.spec)
                                } else {
                                        // declare imported package object in file scope
                                        // (no need to provide s.Name since we called check.recordDef earlier)
-                                       check.declare(fileScope, nil, obj, token.NoPos)
+                                       check.declare(fileScope, nil, pkgName, token.NoPos)
                                }
                        case constDecl:
                                // declare all constants
@@ -566,39 +570,30 @@ func (check *Checker) unusedImports() {
        // any of its exported identifiers. To import a package solely for its side-effects
        // (initialization), use the blank identifier as explicit package name."
 
-       // check use of regular imported packages
-       for _, scope := range check.pkg.scope.children /* file scopes */ {
-               for _, obj := range scope.elems {
-                       if obj, ok := obj.(*PkgName); ok {
-                               // Unused "blank imports" are automatically ignored
-                               // since _ identifiers are not entered into scopes.
-                               if !obj.used {
-                                       path := obj.imported.path
-                                       base := pkgName(path)
-                                       if obj.name == base {
-                                               check.softErrorf(obj, _UnusedImport, "%q imported but not used", path)
-                                       } else {
-                                               check.softErrorf(obj, _UnusedImport, "%q imported but not used as %s", path, obj.name)
-                                       }
-                               }
-                       }
-               }
-       }
-
-       // check use of dot-imported packages
-       for _, unusedDotImports := range check.unusedDotImports {
-               for pkg, pos := range unusedDotImports {
-                       check.softErrorf(pos, _UnusedImport, "%q imported but not used", pkg.path)
+       for _, obj := range check.imports {
+               if !obj.used && obj.name != "_" {
+                       check.errorUnusedPkg(obj)
                }
        }
 }
 
-// pkgName returns the package name (last element) of an import path.
-func pkgName(path string) string {
-       if i := strings.LastIndex(path, "/"); i >= 0 {
-               path = path[i+1:]
+func (check *Checker) errorUnusedPkg(obj *PkgName) {
+       // If the package was imported with a name other than the final
+       // import path element, show it explicitly in the error message.
+       // Note that this handles both renamed imports and imports of
+       // packages containing unconventional package declarations.
+       // Note that this uses / always, even on Windows, because Go import
+       // paths always use forward slashes.
+       path := obj.imported.path
+       elem := path
+       if i := strings.LastIndex(elem, "/"); i >= 0 {
+               elem = elem[i+1:]
+       }
+       if obj.name == "" || obj.name == "." || obj.name == elem {
+               check.softErrorf(obj, _UnusedImport, "%q imported but not used", path)
+       } else {
+               check.softErrorf(obj, _UnusedImport, "%q imported but not used as %s", path, obj.name)
        }
-       return path
 }
 
 // dir makes a good-faith attempt to return the directory
index 6844e7098233eb5488c860c03f5a3274f1b467d5..55690423b66ec675506fed232501d3fbd9c67b5c 100644 (file)
@@ -8,7 +8,7 @@ import "math"
 import m "math"
 
 import . "testing" // declares T in file scope
-import . /* ERROR "imported but not used" */ "unsafe"
+import . /* ERROR .unsafe. imported but not used */ "unsafe"
 import . "fmt"     // declares Println in file scope
 
 import (
index ee70bbd8e73f787c8aa3b3d792f5673d0f0aacbb..43a7bcd75396cbb34ab5ab3c1e354f9d1f4642b7 100644 (file)
@@ -4,7 +4,7 @@
 
 package importdecl1
 
-import . /* ERROR "imported but not used" */ "unsafe"
+import . /* ERROR .unsafe. imported but not used */ "unsafe"
 
 type B interface {
        A
index 311a970051ad41e8aeb24fdbf49e5b577e755bb1..6e89ccb02743b77ad3869c666a918c7d19cdc180 100644 (file)
@@ -51,12 +51,12 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, wantType bool)
        }
        assert(typ != nil)
 
-       // The object may be dot-imported: If so, remove its package from
-       // the map of unused dot imports for the respective file scope.
+       // The object may have been dot-imported.
+       // If so, mark the respective package as used.
        // (This code is only needed for dot-imports. Without them,
        // we only have to mark variables, see *Var case below).
-       if pkg := obj.Pkg(); pkg != check.pkg && pkg != nil {
-               delete(check.unusedDotImports[scope], pkg)
+       if pkgName := check.dotImportMap[dotImportKey{scope, obj}]; pkgName != nil {
+               pkgName.used = true
        }
 
        switch obj := obj.(type) {