go.tools/go/types: fewer guarantees in case of type errors

Provide fewer guarantees regarding the collected result
type information in types.Info if there are type errors.

Specifically, don't record nil objects in the Objects map
in case of a declaration error; instead, simply omit them.

R=adonovan
TBR=adonovan
CC=golang-dev
https://golang.org/cl/12553043

go/types/api.go

@@ -89,29 +89,28 @@ type Config struct {
 	Sizeof func(Type) int64
 }
 
-// Info holds result type information for a package.
+// Info holds result type information for a type-checked package.
+// Only the information for which a map is provided is collected.
+// If the package has type errors, the collected information may
+// be incomplete.
 type Info struct {
-	// If Types != nil, it records the expression and corresponding type
-	// for each expression that is type-checked. Identifiers declaring a
-	// variable are recorded in Objects, not Types.
+	// Types maps expressions to their types. Identifiers on the
+	// lhs of declarations are collected in Objects, not Types.
 	Types map[ast.Expr]Type
 
-	// If Values != nil, it records the expression and corresponding value
-	// for each constant expression that is type-checked.
+	// Values maps constant expressions to their values.
 	Values map[ast.Expr]exact.Value
 
-	// If Objects != nil, it records the identifier and corresponding object
-	// for each identifier that is type-checked (including package names,
-	// dots "." of dot-imports, and blank "_" identifiers). For identifiers
-	// that are not declared (due to an error, or because they are symbolic
-	// as the t in t := x.(type) of a type switch header), the corresponding
-	// object is nil.
+	// Objects maps identifiers to their corresponding objects (including
+	// package names, dots "." of dot-imports, and blank "_" identifiers).
+	// For identifiers that do not denote objects (e.g., blank identifiers
+	// on the lhs of assignments, or symbolic variables t in t := x.(type)
+	// of type switch headers), the corresponding objects are nil.
 	// BUG(gri) Label identifiers in break, continue, or goto statements
-	// are not recorded.
+	// are not yet mapped.
 	Objects map[*ast.Ident]Object
 
-	// If Implicits != nil, it records the node and corresponding object for
-	// each node that is type-checked and that implicitly declared an object.
+	// Implicits maps nodes to their implicitly declared objects, if any.
 	// The following node and object types may appear:
 	//
 	//	node               declared object
@@ -122,10 +121,7 @@ type Info struct {
 	//
 	Implicits map[ast.Node]Object
 
-	// If Selections != nil, it records the selector expression and corresponding
-	// selection, i.e., package object (qualified identifier), struct field (field
-	// selector), or method (method expression or value) for each selector expression
-	// that is type-checked.
+	// Selections maps selector expressions to their corresponding selections.
 	Selections map[*ast.SelectorExpr]*Selection
 }
 

go/types/assignments.go

@@ -306,7 +306,9 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
 				// declare new variable
 				obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil)
 			}
-			check.recordObject(ident, obj) // obj may be nil
+			if obj != nil {
+				check.recordObject(ident, obj)
+			}
 		} else {
 			check.errorf(lhs.Pos(), "cannot declare %s", lhs)
 		}

go/types/call.go

@@ -169,7 +169,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 	// can only appear in qualified identifiers which are mapped to
 	// selector expressions.
 	if ident, ok := e.X.(*ast.Ident); ok {
-		if pkg, ok := check.topScope.LookupParent(ident.Name).(*Package); ok {
+		if pkg, _ := check.topScope.LookupParent(ident.Name).(*Package); pkg != nil {
 			check.recordObject(ident, pkg)
 			exp := pkg.scope.Lookup(sel)
 			if exp == nil {

go/types/gcimporter.go

@@ -107,7 +107,6 @@ func GcImportData(imports map[string]*Package, filename, id string, data *bufio.
 // corresponding package object to the imports map, and returns the object.
 // Local import paths are interpreted relative to the current working directory.
 // The imports map must contains all packages already imported.
-// GcImport satisfies the ast.Importer signature.
 //
 func GcImport(imports map[string]*Package, path string) (pkg *Package, err error) {
 	if path == "unsafe" {

go/types/lookup.go

@@ -236,7 +236,7 @@ func MissingMethod(typ Type, T *Interface, static bool) (method *Func, wrongType
 		return
 	}
 
-	// TODO(gri) Consider using methods sets here. Might be more efficient.
+	// TODO(gri) Consider using method sets here. Might be more efficient.
 
 	if ityp, _ := typ.Underlying().(*Interface); ityp != nil {
 		for _, m := range T.methods {

go/types/resolver.go

@@ -27,7 +27,7 @@ func (check *checker) declareObj(scope *Scope, id *ast.Ident, obj Object) {
 	if alt := scope.Insert(obj); alt != nil {
 		check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name())
 		check.reportAltDecl(alt)
-		obj = nil
+		return
 	}
 	if id != nil {
 		check.recordObject(id, obj)
@@ -38,7 +38,7 @@ func (check *checker) declareFld(oset *objset, id *ast.Ident, obj Object) {
 	if alt := oset.insert(obj); alt != nil {
 		check.errorf(obj.Pos(), "%s redeclared", obj.Name())
 		check.reportAltDecl(alt)
-		obj = nil
+		return
 	}
 	if id != nil {
 		check.recordObject(id, obj)
@@ -131,7 +131,6 @@ func (check *checker) resolveFiles(files []*ast.File) {
 		// spec: "A package-scope or file-scope identifier with name init
 		// may only be declared to be a function with this (func()) signature."
 		if ident.Name == "init" {
-			check.recordObject(ident, nil)
 			check.errorf(ident.Pos(), "cannot declare init - must be func")
 			return
 		}
@@ -529,8 +528,6 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk
 
 	// check each method
 	for _, m := range methods {
-		ident := check.objMap[m].fdecl.Name
-
 		if alt := mset.insert(m); alt != nil {
 			switch alt.(type) {
 			case *Var:
@@ -541,19 +538,14 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk
 				unreachable()
 			}
 			check.reportAltDecl(alt)
-			m = nil
+			continue
 		}
-
-		// If the method is valid, type-check its signature,
-		// and collect it with the named base type.
-		if m != nil {
-			check.recordObject(ident, m)
-			check.objDecl(m, nil, true)
-			// Methods with blank _ names cannot be found.
-			// Don't add them to the method list.
-			if m.name != "_" {
-				named.methods = append(named.methods, m)
-			}
+		check.recordObject(check.objMap[m].fdecl.Name, m)
+		check.objDecl(m, nil, true)
+		// Methods with blank _ names cannot be found.
+		// Don't add them to the method list.
+		if m.name != "_" {
+			named.methods = append(named.methods, m)
 		}
 	}
 

go/types/typexpr.go

@@ -23,7 +23,6 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool)
 	x.expr = e
 
 	obj := check.topScope.LookupParent(e.Name)
-	check.recordObject(e, obj)
 	if obj == nil {
 		if e.Name == "_" {
 			check.errorf(e.Pos(), "cannot use _ as value or type")
@@ -32,6 +31,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool)
 		}
 		return
 	}
+	check.recordObject(e, obj)
 
 	typ := obj.Type()
 	if typ == nil {