From c2d7895b1e0ae2a318019c8aa8b7068c3a3adcfc Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Fri, 20 Sep 2013 10:09:16 -0700 Subject: [PATCH] go.tools/go/types: "imported but not used" errors for dot-imports R=adonovan, josharian CC=golang-dev https://golang.org/cl/13795043 --- go/types/objects.go | 27 +++++++------- go/types/resolver.go | 56 +++++++++++++++++++++++------- go/types/testdata/importdecl0a.src | 2 +- go/types/testdata/importdecl0b.src | 12 +++++++ go/types/typexpr.go | 8 +++++ 5 files changed, 78 insertions(+), 27 deletions(-) diff --git a/go/types/objects.go b/go/types/objects.go index 7d55f866..7879fbac 100644 --- a/go/types/objects.go +++ b/go/types/objects.go @@ -31,6 +31,9 @@ type Object interface { // String returns a human-readable string of the object. String() string + // isUsed reports whether the object was marked as 'used'. + isUsed() bool + // setParent sets the parent scope of the object. setParent(*Scope) @@ -66,6 +69,7 @@ type object struct { pkg *Package name string typ Type + used bool } func (obj *object) Parent() *Scope { return obj.parent } @@ -77,6 +81,8 @@ func (obj *object) IsExported() bool { return ast.IsExported(obj.name) } func (obj *object) Id() string { return Id(obj.pkg, obj.name) } func (obj *object) String() string { panic("abstract") } +func (obj *object) isUsed() bool { return obj.used } + func (obj *object) toString(kind string, typ Type) string { var buf bytes.Buffer @@ -123,12 +129,10 @@ func (obj *object) sameId(pkg *Package, name string) bool { // A PkgName represents an imported Go package. type PkgName struct { object - - used bool } func NewPkgName(pos token.Pos, pkg *Package, name string) *PkgName { - return &PkgName{object: object{nil, pos, pkg, name, Typ[Invalid]}} + return &PkgName{object{nil, pos, pkg, name, Typ[Invalid], false}} } func (obj *PkgName) String() string { return obj.toString("package", nil) } @@ -142,7 +146,7 @@ type Const struct { } func NewConst(pos token.Pos, pkg *Package, name string, typ Type, val exact.Value) *Const { - return &Const{object{nil, pos, pkg, name, typ}, val, false} + return &Const{object: object{nil, pos, pkg, name, typ, false}, val: val} } func (obj *Const) String() string { return obj.toString("const", obj.typ) } @@ -154,7 +158,7 @@ type TypeName struct { } func NewTypeName(pos token.Pos, pkg *Package, name string, typ Type) *TypeName { - return &TypeName{object{nil, pos, pkg, name, typ}} + return &TypeName{object{nil, pos, pkg, name, typ, false}} } func (obj *TypeName) String() string { return obj.toString("type", obj.typ.Underlying()) } @@ -165,19 +169,18 @@ type Var struct { anonymous bool // if set, the variable is an anonymous struct field, and name is the type name visited bool // for initialization cycle detection - used bool // if set, the variable was 'used' } func NewVar(pos token.Pos, pkg *Package, name string, typ Type) *Var { - return &Var{object: object{nil, pos, pkg, name, typ}} + return &Var{object: object{nil, pos, pkg, name, typ, false}} } func NewParam(pos token.Pos, pkg *Package, name string, typ Type) *Var { - return &Var{object: object{nil, pos, pkg, name, typ}, used: true} // parameters are always 'used' + return &Var{object: object{nil, pos, pkg, name, typ, true}} // parameters are always 'used' } func NewField(pos token.Pos, pkg *Package, name string, typ Type, anonymous bool) *Var { - return &Var{object: object{nil, pos, pkg, name, typ}, anonymous: anonymous} + return &Var{object: object{nil, pos, pkg, name, typ, false}, anonymous: anonymous} } func (obj *Var) Anonymous() bool { return obj.anonymous } @@ -191,7 +194,7 @@ type Func struct { } func NewFunc(pos token.Pos, pkg *Package, name string, typ Type) *Func { - return &Func{object{nil, pos, pkg, name, typ}} + return &Func{object{nil, pos, pkg, name, typ, false}} } // FullName returns the package- or receiver-type-qualified name of @@ -238,12 +241,10 @@ func (obj *Func) String() string { // A Label represents a declared label. type Label struct { object - - used bool } func NewLabel(pos token.Pos, name string) *Label { - return &Label{object: object{nil, pos, nil, name, nil}} + return &Label{object{nil, pos, nil, name, nil, false}} } func (obj *Label) String() string { return fmt.Sprintf("label %s", obj.Name()) } diff --git a/go/types/resolver.go b/go/types/resolver.go index c1169751..226e5efe 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -145,10 +145,12 @@ func (check *checker) resolveFiles(files []*ast.File) { importer = GcImport } - // only add packages to pkg.imports that have not been seen already - seen := make(map[*Package]bool) + var ( + seenPkgs = make(map[*Package]bool) // imported packages that have been seen already + fileScopes []*Scope // file scope for each file + dotImports []map[*Package]token.Pos // positions of dot-imports for each file + ) - var fileScopes []*Scope // list of file scopes for _, file := range files { // The package identifier denotes the current package, // but there is no corresponding package object. @@ -157,6 +159,7 @@ func (check *checker) resolveFiles(files []*ast.File) { fileScope = NewScope(pkg.scope) check.recordScope(file, fileScope) fileScopes = append(fileScopes, fileScope) + dotImports = append(dotImports, nil) // element (map) is lazily allocated for _, decl := range file.Decls { switch d := decl.(type) { @@ -190,8 +193,8 @@ func (check *checker) resolveFiles(files []*ast.File) { // add package to list of explicit imports // (this functionality is provided as a convenience // for clients; it is not needed for type-checking) - if !seen[imp] { - seen[imp] = true + if !seenPkgs[imp] { + seenPkgs[imp] = true if imp != Unsafe { pkg.imports = append(pkg.imports, imp) } @@ -228,6 +231,14 @@ func (check *checker) resolveFiles(files []*ast.File) { check.recordImplicit(s, obj) } } + // add position to set of dot-import positions for this file + // (this is only needed for "imported but not used" errors) + posSet := dotImports[len(dotImports)-1] + if posSet == nil { + posSet = make(map[*Package]token.Pos) + dotImports[len(dotImports)-1] = posSet + } + posSet[imp] = s.Pos() } else { // declare imported package object in file scope check.declareObj(fileScope, nil, obj) @@ -408,16 +419,35 @@ func (check *checker) resolveFiles(files []*ast.File) { // spec: "It is illegal (...) to directly import a package without referring to // any of its exported identifiers. To import a package solely for its side-effects // (initialization), use the blank identifier as explicit package name." - for _, scope := range fileScopes { - // Unused "blank imports" are automatically ignored - // since _ identifiers are not entered into scopes. + + for i, scope := range fileScopes { + var usedDotImports map[*Package]bool // lazily allocated for _, obj := range scope.elems { - if p, _ := obj.(*PkgName); p != nil && !p.used { - check.errorf(p.pos, "%q imported but not used", p.pkg.path) + switch obj := obj.(type) { + case *PkgName: + // Unused "blank imports" are automatically ignored + // since _ identifiers are not entered into scopes. + if !obj.used { + check.errorf(obj.pos, "%q imported but not used", obj.pkg.path) + } + default: + // All other objects in the file scope must be dot- + // imported. If an object was used, mark its package + // as used. + if obj.isUsed() { + if usedDotImports == nil { + usedDotImports = make(map[*Package]bool) + } + usedDotImports[obj.Pkg()] = true + } + } + } + // Iterate through all dot-imports for this file and + // check if the corresponding package was used. + for pkg, pos := range dotImports[i] { + if !usedDotImports[pkg] { + check.errorf(pos, "%q imported but not used", pkg.path) } - // TODO(gri) dot-imports are not handled for now since we don't - // have mapping from Object to corresponding PkgName through - // which the object was imported. } } diff --git a/go/types/testdata/importdecl0a.src b/go/types/testdata/importdecl0a.src index db87ee5c..c6b82b7e 100644 --- a/go/types/testdata/importdecl0a.src +++ b/go/types/testdata/importdecl0a.src @@ -13,7 +13,7 @@ import ( init /* ERROR "cannot declare init" */ "fmt" // reflect defines a type "flag" which shows up in the gc export data "reflect" - . "reflect" + . /* ERROR "imported but not used" */ "reflect" ) import "math" /* ERROR "imported but not used" */ diff --git a/go/types/testdata/importdecl0b.src b/go/types/testdata/importdecl0b.src index f5fcd368..2073d98c 100644 --- a/go/types/testdata/importdecl0b.src +++ b/go/types/testdata/importdecl0b.src @@ -7,6 +7,18 @@ package importdecl0 import "math" import m "math" +import . "testing" // declares T in file scope +import . /* ERROR "imported but not used" */ "unsafe" +import . "fmt" // declares Println in file scope + // using "math" in this file doesn't affect its use in other files const Pi0 = math.Pi const Pi1 = m.Pi + +type _ T // use "testing" + +func _() func() interface{} { + return func() interface{} { + return Println // use "fmt" + } +} diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 133b3ff1..8d83b4f2 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -51,6 +51,12 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) return case *Const: + // The constant may be dot-imported. Mark it as used so that + // later we can determine if the corresponding dot-imported + // packages was used. Same applies for other objects, below. + // (This code is only used for dot-imports. Without them, we + // would only have to mark Vars.) + obj.used = true if typ == Typ[Invalid] { return } @@ -66,6 +72,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) x.mode = constant case *TypeName: + obj.used = true x.mode = typexpr named, _ := typ.(*Named) if !cycleOk && named != nil && !named.complete { @@ -82,6 +89,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) x.mode = variable case *Func: + obj.used = true // use dot-imported function x.mode = value default: