go/types: port recent x/tools/go/types fixes

The main change is:

golang.org/cl/10800  add pos parameter to Eval; remove New, EvalNode

followed by several cleanups/follow-up fixes:

golang.org/cl/10992  remove global vars in test
golang.org/cl/10994  remove unused scope parameter from NewSignature
golang.org/cl/10995  provide full source file extent to file scope
golang.org/cl/10996  comment fix in resolver.go
golang.org/cl/11004  updated cmd/vet
golang.org/cl/11042  be robust in the presence of incorrect/missing position info

Fixes golang#9980.

Change-Id: Id4aff688f6a399f76bf92b84c7e793b8da8baa48
Reviewed-on: https://go-review.googlesource.com/11122
Reviewed-by: Alan Donovan <[email protected]>

src/cmd/vet/shadow.go

@@ -212,7 +212,7 @@ func checkShadowing(f *File, ident *ast.Ident) {
 	}
 	// obj.Parent.Parent is the surrounding scope. If we can find another declaration
 	// starting from there, we have a shadowed identifier.
-	_, shadowed := obj.Parent().Parent().LookupParent(obj.Name())
+	_, shadowed := obj.Parent().Parent().LookupParent(obj.Name(), obj.Pos())
 	if shadowed == nil {
 		return
 	}

src/cmd/vet/unused.go

@@ -31,7 +31,7 @@ func init() {
 }
 
 // func() string
-var sigNoArgsStringResult = types.NewSignature(nil, nil, nil,
+var sigNoArgsStringResult = types.NewSignature(nil, nil,
 	types.NewTuple(types.NewVar(token.NoPos, nil, "", types.Typ[types.String])),
 	false)
 

src/go/internal/gcimporter/gcimporter.go

@@ -568,7 +568,7 @@ func (p *parser) parseSignature(recv *types.Var) *types.Signature {
 		}
 	}
 
-	return types.NewSignature(nil, recv, types.NewTuple(params...), types.NewTuple(results...), isVariadic)
+	return types.NewSignature(recv, types.NewTuple(params...), types.NewTuple(results...), isVariadic)
 }
 
 // InterfaceType = "interface" "{" [ MethodList ] "}" .

src/go/types/assignments.go

@@ -161,7 +161,7 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
 	var v *Var
 	var v_used bool
 	if ident != nil {
-		if _, obj := check.scope.LookupParent(ident.Name); obj != nil {
+		if _, obj := check.scope.LookupParent(ident.Name, token.NoPos); obj != nil {
 			v, _ = obj.(*Var)
 			if v != nil {
 				v_used = v.used
@@ -314,8 +314,13 @@ func (check *Checker) shortVarDecl(pos token.Pos, lhs, rhs []ast.Expr) {
 
 	// declare new variables
 	if len(newVars) > 0 {
+		// spec: "The scope of a constant or variable identifier declared inside
+		// a function begins at the end of the ConstSpec or VarSpec (ShortVarDecl
+		// for short variable declarations) and ends at the end of the innermost
+		// containing block."
+		scopePos := rhs[len(rhs)-1].End()
 		for _, obj := range newVars {
-			check.declare(scope, nil, obj) // recordObject already called
+			check.declare(scope, nil, obj, scopePos) // recordObject already called
 		}
 	} else {
 		check.softErrorf(pos, "no new variables on left side of :=")

src/go/types/call.go

@@ -280,7 +280,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 {
-		_, obj := check.scope.LookupParent(ident.Name)
+		_, obj := check.scope.LookupParent(ident.Name, check.pos)
 		if pkg, _ := obj.(*PkgName); pkg != nil {
 			assert(pkg.pkg == check.pkg)
 			check.recordUse(ident, pkg)

src/go/types/check.go

@@ -83,6 +83,7 @@ type Checker struct {
 	// context within which the current object is type-checked
 	// (valid only for the duration of type-checking a specific object)
 	context
+	pos token.Pos // if valid, identifiers are looked up as if at position pos (used by Eval)
 
 	// debugging
 	indent int // indentation for tracing

src/go/types/decl.go

@@ -19,7 +19,7 @@ func (check *Checker) reportAltDecl(obj Object) {
 	}
 }
 
-func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object) {
+func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token.Pos) {
 	// spec: "The blank identifier, represented by the underscore
 	// character _, may be used in a declaration like any other
 	// identifier but the declaration does not introduce a new
@@ -30,6 +30,7 @@ func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object) {
 			check.reportAltDecl(alt)
 			return
 		}
+		obj.setScopePos(pos)
 	}
 	if id != nil {
 		check.recordDef(id, obj)
@@ -347,8 +348,13 @@ func (check *Checker) declStmt(decl ast.Decl) {
 
 					check.arityMatch(s, last)
 
+					// spec: "The scope of a constant or variable identifier declared
+					// inside a function begins at the end of the ConstSpec or VarSpec
+					// (ShortVarDecl for short variable declarations) and ends at the
+					// end of the innermost containing block."
+					scopePos := s.End()
 					for i, name := range s.Names {
-						check.declare(check.scope, name, lhs[i])
+						check.declare(check.scope, name, lhs[i], scopePos)
 					}
 
 				case token.VAR:
@@ -394,8 +400,10 @@ func (check *Checker) declStmt(decl ast.Decl) {
 
 					// declare all variables
 					// (only at this point are the variable scopes (parents) set)
+					scopePos := s.End() // see constant declarations
 					for i, name := range s.Names {
-						check.declare(check.scope, name, lhs0[i])
+						// see constant declarations
+						check.declare(check.scope, name, lhs0[i], scopePos)
 					}
 
 				default:
@@ -404,7 +412,11 @@ func (check *Checker) declStmt(decl ast.Decl) {
 
 			case *ast.TypeSpec:
 				obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Name, nil)
-				check.declare(check.scope, s.Name, obj)
+				// spec: "The scope of a type identifier declared inside a function
+				// begins at the identifier in the TypeSpec and ends at the end of
+				// the innermost containing block."
+				scopePos := s.Name.Pos()
+				check.declare(check.scope, s.Name, obj, scopePos)
 				check.typeDecl(obj, s.Type, nil, nil)
 
 			default:

src/go/types/eval.go

@@ -2,44 +2,30 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// This file implements New, Eval and EvalNode.
-
 package types
 
 import (
 	"fmt"
-	"go/ast"
 	"go/parser"
 	"go/token"
 )
 
-// New is a convenience function to create a new type from a given
-// expression or type literal string evaluated in Universe scope.
-// New(str) is shorthand for Eval(str, nil, nil), but only returns
-// the type result, and panics in case of an error.
-// Position info for objects in the result type is undefined.
-//
-func New(str string) Type {
-	tv, err := Eval(str, nil, nil)
-	if err != nil {
-		panic(err)
-	}
-	return tv.Type
-}
-
 // Eval returns the type and, if constant, the value for the
-// expression or type literal string str evaluated in scope.
-// If the expression contains function literals, the function
-// bodies are ignored (though they must be syntactically correct).
+// expression expr, evaluated at position pos of package pkg,
+// which must have been derived from type-checking an AST with
+// complete position information relative to the provided file
+// set.
+//
+// If the expression contains function literals, their bodies
+// are ignored (i.e., the bodies are not type-checked).
 //
 // If pkg == nil, the Universe scope is used and the provided
-// scope is ignored. Otherwise, the scope must belong to the
-// package (either the package scope, or nested within the
-// package scope).
+// position pos is ignored. If pkg != nil, and pos is invalid,
+// the package scope is used. Otherwise, pos must belong to the
+// package.
 //
-// An error is returned if the scope is incorrect, the string
-// has syntax errors, or if it cannot be evaluated in the scope.
-// Position info for objects in the result type is undefined.
+// An error is returned if pos is not within the package or
+// if the node cannot be evaluated.
 //
 // Note: Eval should not be used instead of running Check to compute
 // types and values, but in addition to Check. Eval will re-evaluate
@@ -48,48 +34,54 @@ func New(str string) Type {
 // level untyped constants will return an untyped type rather then the
 // respective context-specific type.
 //
-func Eval(str string, pkg *Package, scope *Scope) (TypeAndValue, error) {
-	node, err := parser.ParseExpr(str)
-	if err != nil {
-		return TypeAndValue{}, err
-	}
-
-	// Create a file set that looks structurally identical to the
-	// one created by parser.ParseExpr for correct error positions.
-	fset := token.NewFileSet()
-	fset.AddFile("", len(str), fset.Base()).SetLinesForContent([]byte(str))
-
-	return EvalNode(fset, node, pkg, scope)
-}
-
-// EvalNode is like Eval but instead of string it accepts
-// an expression node and respective file set.
-//
-// An error is returned if the scope is incorrect
-// if the node cannot be evaluated in the scope.
-//
-func EvalNode(fset *token.FileSet, node ast.Expr, pkg *Package, scope *Scope) (tv TypeAndValue, err error) {
-	// verify package/scope relationship
+func Eval(fset *token.FileSet, pkg *Package, pos token.Pos, expr string) (tv TypeAndValue, err error) {
+	// determine scope
+	var scope *Scope
 	if pkg == nil {
 		scope = Universe
+		pos = token.NoPos
+	} else if !pos.IsValid() {
+		scope = pkg.scope
 	} else {
-		s := scope
-		for s != nil && s != pkg.scope {
-			s = s.parent
+		// The package scope extent (position information) may be
+		// incorrect (files spread accross a wide range of fset
+		// positions) - ignore it and just consider its children
+		// (file scopes).
+		for _, fscope := range pkg.scope.children {
+			if scope = fscope.Innermost(pos); scope != nil {
+				break
+			}
 		}
-		// s == nil || s == pkg.scope
-		if s == nil {
-			return TypeAndValue{}, fmt.Errorf("scope does not belong to package %s", pkg.name)
+		if scope == nil || debug {
+			s := scope
+			for s != nil && s != pkg.scope {
+				s = s.parent
+			}
+			// s == nil || s == pkg.scope
+			if s == nil {
+				return TypeAndValue{}, fmt.Errorf("no position %s found in package %s", fset.Position(pos), pkg.name)
+			}
 		}
 	}
 
+	// parse expressions
+	// BUG(gri) In case of type-checking errors below, the type checker
+	//          doesn't have the correct file set for expr. The correct
+	//          solution requires a ParseExpr that uses the incoming
+	//          file set fset.
+	node, err := parser.ParseExpr(expr)
+	if err != nil {
+		return TypeAndValue{}, err
+	}
+
 	// initialize checker
 	check := NewChecker(nil, fset, pkg, nil)
 	check.scope = scope
+	check.pos = pos
 	defer check.handleBailout(&err)
 
 	// evaluate node
 	var x operand
 	check.rawExpr(&x, node, nil)
-	return TypeAndValue{x.mode, x.typ, x.val}, nil
+	return TypeAndValue{x.mode, x.typ, x.val}, err
 }