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

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

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
+	var (
-	seen := make(map[*Package]bool)
+		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] {
+						if !seenPkgs[imp] {
-							seen[imp] = true
+							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
+	for i, scope := range fileScopes {
-		// since _ identifiers are not entered into scopes.
+		var usedDotImports map[*Package]bool // lazily allocated
 		for _, obj := range scope.elems {
-			if p, _ := obj.(*PkgName); p != nil && !p.used {
+			switch obj := obj.(type) {
-				check.errorf(p.pos, "%q imported but not used", p.pkg.path)
+			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.
 		}
 	}
 

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" */

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"
+	}
+}

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: