go.tools/go/types: enable incremental adding of package files

With this CL, it is now possible to type-check additional
package files to an already type-checked package through
repeated calls to Checker.Files.

Fixes golang/go#7114.

LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/72730043

go/types/api_test.go

@@ -5,6 +5,7 @@
 package types_test
 
 import (
+	"fmt"
 	"go/ast"
 	"go/parser"
 	"go/token"
@@ -444,3 +445,27 @@ func TestInitOrderInfo(t *testing.T) {
 		}
 	}
 }
+
+func TestFiles(t *testing.T) {
+	var sources = []string{
+		"package p; type T struct{}; func (T) m1() {}",
+		"package p; func (T) m2() {}; var _ interface{ m1(); m2() } = T{}",
+		"package p; func (T) m3() {}; var _ interface{ m1(); m2(); m3() } = T{}",
+	}
+
+	var conf Config
+	fset := token.NewFileSet()
+	pkg := NewPackage("p", "p")
+	check := NewChecker(&conf, fset, pkg, nil)
+
+	for i, src := range sources {
+		filename := fmt.Sprintf("sources%d", i)
+		f, err := parser.ParseFile(fset, filename, src, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := check.Files([]*ast.File{f}); err != nil {
+			t.Error(err)
+		}
+	}
+}

go/types/check.go

@@ -151,6 +151,17 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *ch
 // initFiles initializes the files-specific portion of checker.
 // The provided files must all belong to the same package.
 func (check *checker) initFiles(files []*ast.File) {
+	// start with a clean slate (check.Files may be called multiple times)
+	check.files = nil
+	check.fileScopes = nil
+	check.dotImports = nil
+
+	check.firstErr = nil
+	check.methods = nil
+	check.untyped = nil
+	check.funcs = nil
+	check.delayed = nil
+
 	// determine package name, files, and set up file scopes, dotImports maps
 	pkg := check.pkg
 	for _, file := range files {
@@ -171,13 +182,6 @@ func (check *checker) initFiles(files []*ast.File) {
 			// ignore this file
 		}
 	}
-
-	// start with a clean slate (check.Files may be called multiple times)
-	check.firstErr = nil
-	check.methods = nil
-	check.untyped = nil
-	check.funcs = nil
-	check.delayed = nil
 }
 
 // A bailout panic is raised to indicate early termination.
@@ -202,11 +206,12 @@ func (check *checker) Files(files []*ast.File) (err error) {
 
 	check.collectObjects()
 
-	check.packageObjects()
+	objList := objectsOf(check.objMap)
+	check.packageObjects(objList)
 
 	check.functionBodies()
 
-	check.initDependencies()
+	check.initDependencies(objList)
 
 	check.unusedImports()
 

go/types/decl.go

@@ -163,7 +163,7 @@ func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
 }
 
 // underlying returns the underlying type of typ; possibly by following
-// forward chains of named types. Such chains only exist while names types
+// forward chains of named types. Such chains only exist while named types
 // are incomplete.
 func underlying(typ Type) Type {
 	for {
@@ -210,22 +210,29 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []*
 	// any forward chain (they always end in an unnamed type).
 	named.underlying = underlying(named.underlying)
 
-	// type-check signatures of associated methods
+	// check and add associated methods
+	// TODO(gri) It's easy to create pathological cases where the
+	// current approach is incorrect: In general we need to know
+	// and add all methods _before_ type-checking the type.
+	// See http://play.golang.org/p/WMpE0q2wK8
+	check.addMethodDecls(obj)
+}
+
+func (check *checker) addMethodDecls(obj *TypeName) {
+	// get associated methods
 	methods := check.methods[obj.name]
 	if len(methods) == 0 {
 		return // no methods
 	}
+	delete(check.methods, obj.name)
 
-	// spec: "For a base type, the non-blank names of methods bound
-	// to it must be unique."
-	// => use an objset to determine redeclarations
+	// use an objset to check for name conflicts
 	var mset objset
 
 	// spec: "If the base type is a struct type, the non-blank method
 	// and field names must be distinct."
-	// => pre-populate the objset to find conflicts
-	// TODO(gri) consider keeping the objset with the struct instead
-	if t, _ := named.underlying.(*Struct); t != nil {
+	base := obj.typ.(*Named)
+	if t, _ := base.underlying.(*Struct); t != nil {
 		for _, fld := range t.fields {
 			if fld.name != "_" {
 				assert(mset.insert(fld) == nil)
@@ -233,15 +240,25 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []*
 		}
 	}
 
-	// check each method
+	// Checker.Files may be called multiple times; additional package files
+	// may add methods to already type-checked types. Add pre-existing methods
+	// so that we can detect redeclarations.
+	for _, m := range base.methods {
+		assert(m.name != "_")
+		assert(mset.insert(m) == nil)
+	}
+
+	// type-check methods
 	for _, m := range methods {
+		// spec: "For a base type, the non-blank names of methods bound
+		// to it must be unique."
 		if m.name != "_" {
 			if alt := mset.insert(m); alt != nil {
 				switch alt.(type) {
 				case *Var:
 					check.errorf(m.pos, "field and method with the same name %s", m.name)
 				case *Func:
-					check.errorf(m.pos, "method %s already declared for %s", m.name, named)
+					check.errorf(m.pos, "method %s already declared for %s", m.name, base)
 				default:
 					unreachable()
 				}
@@ -249,16 +266,12 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []*
 				continue
 			}
 		}
-		check.recordDef(check.objMap[m].fdecl.Name, m)
 		check.objDecl(m, nil, nil)
-		// Methods with blank _ names cannot be found.
-		// Don't add them to the method list.
+		// methods with blank _ names cannot be found - don't keep them
 		if m.name != "_" {
-			named.methods = append(named.methods, m)
+			base.methods = append(base.methods, m)
 		}
 	}
-
-	delete(check.methods, obj.name) // we don't need them anymore
 }
 
 func (check *checker) funcDecl(obj *Func, decl *declInfo) {

go/types/resolver.go

@@ -305,6 +305,8 @@ func (check *checker) collectObjects() {
 						check.declare(pkg.scope, d.Name, obj)
 					}
 				} else {
+					// method
+					check.recordDef(d.Name, obj)
 					// Associate method with receiver base type name, if possible.
 					// Ignore methods that have an invalid receiver, or a blank _
 					// receiver name. They will be type-checked later, with regular
@@ -339,17 +341,24 @@ func (check *checker) collectObjects() {
 }
 
 // packageObjects typechecks all package objects in check.objMap, but not function bodies.
-func (check *checker) packageObjects() {
+func (check *checker) packageObjects(objList []Object) {
+	// add new methods to already type-checked types (from a prior Checker.Files call)
+	for _, obj := range objList {
+		if obj, _ := obj.(*TypeName); obj != nil && obj.typ != nil {
+			check.addMethodDecls(obj)
+		}
+	}
+
 	// pre-allocate space for type declaration paths so that the underlying array is reused
 	typePath := make([]*TypeName, 0, 8)
 
-	for _, obj := range objectsOf(check.objMap) {
+	for _, obj := range objList {
 		check.objDecl(obj, nil, typePath)
 	}
 
 	// At this point we may have a non-empty check.methods map; this means that not all
 	// entries were deleted at the end of typeDecl because the respective receiver base
-	// types were not declared. In that case, an error was reported when declaring those
+	// types were not found. In that case, an error was reported when declaring those
 	// methods. We can now safely discard this map.
 	check.methods = nil
 }
@@ -362,11 +371,11 @@ func (check *checker) functionBodies() {
 }
 
 // initDependencies computes initialization dependencies.
-func (check *checker) initDependencies() {
+func (check *checker) initDependencies(objList []Object) {
 	// pre-allocate space for initialization paths so that the underlying array is reused
 	initPath := make([]Object, 0, 8)
 
-	for _, obj := range objectsOf(check.objMap) {
+	for _, obj := range objList {
 		switch obj.(type) {
 		case *Const, *Var:
 			if d := check.objMap[obj]; d.hasInitializer() {