go.tools/go/types: don't type-check methods twice

- Centralize "visited" check in check.objDecl.
- Assert that object-specific declX functions
are only called with objects that have no
type associated with them, yet.
- Added test case.

Thanks to Richard Musiol (neelance@gmail.com) for
finding the bug and providing an easy test case.

For a discussion of the bug see the issue.

Fixes golang/go#7245.

TBR=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/59210043

go/types/decl.go

@@ -34,7 +34,23 @@ func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) {
 // objDecl type-checks the declaration of obj in its respective file scope.
 // See typeDecl for the details on def and cycleOk.
 func (check *checker) objDecl(obj Object, def *Named, cycle []*TypeName) {
+	if obj.Type() != nil {
+		return // already checked - nothing to do
+	}
+
+	if trace {
+		check.trace(obj.Pos(), "-- resolving %s", obj.Name())
+	}
+
 	d := check.objMap[obj]
+	if debug && d == nil {
+		if check.objMap == nil {
+			check.dump("%s: %s should have been declared (we are inside a function)", obj.Pos(), obj)
+			unreachable()
+		}
+		check.dump("%s: %s should have been forward-declared", obj.Pos(), obj)
+		unreachable()
+	}
 
 	// adjust file scope for current object
 	oldScope := check.topScope
@@ -68,6 +84,8 @@ func (check *checker) objDecl(obj Object, def *Named, cycle []*TypeName) {
 }
 
 func (check *checker) constDecl(obj *Const, typ, init ast.Expr) {
+	assert(obj.typ == nil)
+
 	// TODO(gri) consider using the same cycle detection as for types
 	// so that we can print the actual cycle in case of an error
 	if obj.visited {
@@ -105,6 +123,8 @@ func (check *checker) constDecl(obj *Const, typ, init ast.Expr) {
 
 // TODO(gri) document arguments
 func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
+	assert(obj.typ == nil)
+
 	// TODO(gri) consider using the same cycle detection as for types
 	// so that we can print the actual cycle in case of an error
 	if obj.visited {
@@ -176,7 +196,7 @@ func (n *Named) setUnderlying(typ Type) {
 }
 
 func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycle []*TypeName) {
-	assert(obj.Type() == nil)
+	assert(obj.typ == nil)
 
 	// type declarations cannot use iota
 	assert(check.iota == nil)
@@ -262,6 +282,8 @@ type funcInfo struct {
 }
 
 func (check *checker) funcDecl(obj *Func, info *declInfo) {
+	assert(obj.typ == nil)
+
 	// func declarations cannot use iota
 	assert(check.iota == nil)
 
@@ -347,6 +369,19 @@ func (check *checker) declStmt(decl ast.Decl) {
 							}
 						}
 						check.varDecl(obj, lhs, s.Type, init)
+						if len(s.Values) == 1 {
+							// If we have a single lhs variable we are done either way.
+							// If we have a single rhs expression, it must be a multi-
+							// valued expression, in which case handling the first lhs
+							// variable will cause all lhs variables to have a type
+							// assigned, and we are done as well.
+							if debug {
+								for _, obj := range lhs0 {
+									assert(obj.typ != nil)
+								}
+							}
+							break
+						}
 					}
 
 					check.arityMatch(s, nil)

go/types/issues_test.go

@@ -117,3 +117,30 @@ func f() int {
 		t.Errorf("got %d CallExprs; want 2", n)
 	}
 }
+
+func TestIssue7245(t *testing.T) {
+	src := `
+package p
+func (T) m() (res bool) { return }
+type T struct{} // receiver type after method declaration
+`
+	f, err := parser.ParseFile(fset, "", src, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var conf Config
+	objects := make(map[*ast.Ident]Object)
+	_, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Objects: objects})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	m := f.Decls[0].(*ast.FuncDecl)
+	res1 := objects[m.Name].(*Func).Type().(*Signature).Results().At(0)
+	res2 := objects[m.Type.Results.List[0].Names[0]].(*Var)
+
+	if res1 != res2 {
+		t.Errorf("got %s (%p) != %s (%p)", res1, res2, res1, res2)
+	}
+}

go/types/resolver.go

@@ -358,13 +358,8 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	check.objMap = objMap                 // indicate that we are checking package-level declarations (objects may not have a type yet)
 	emptyCycle := make([]*TypeName, 0, 8) // re-use the same underlying array for cycle detection
 	for _, obj := range objectsOf(check.objMap) {
-		if obj.Type() == nil {
-			if trace {
-				check.trace(obj.Pos(), "-- resolving %s", obj.Name())
-			}
 		check.objDecl(obj, nil, emptyCycle)
 	}
-	}
 	emptyCycle = nil   // not needed anymore
 	check.objMap = nil // not needed anymore
 

go/types/typexpr.go

@@ -34,16 +34,8 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycle []*TypeN
 	}
 	check.recordObject(e, obj)
 
-	typ := obj.Type()
-	if typ == nil {
-		// object type not yet determined
-		if check.objMap == nil {
-			check.dump("%s: %s should have been declared (we are inside a function)", e.Pos(), e)
-			unreachable()
-		}
 	check.objDecl(obj, def, cycle)
-		typ = obj.Type()
+	typ := obj.Type()
-	}
 	assert(typ != nil)
 
 	switch obj := obj.(type) {