go.tools/go/types: implement shadowed return parameter restriction

Also:
- slightly better error messages for return statements
- more AST validity checking of parameter lists
- use secondary error message for clarifying message in type switch errors

Fixes golang/go#7469.

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

go/types/assignments.go

@@ -101,7 +101,8 @@ func (check *checker) initConst(lhs *Const, x *operand) {
 	lhs.val = x.val
 }
 
-func (check *checker) initVar(lhs *Var, x *operand) Type {
+// If result is set, lhs is a function result parameter and x is a return result.
+func (check *checker) initVar(lhs *Var, x *operand, result bool) Type {
 	if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] {
 		if lhs.typ == nil {
 			lhs.typ = Typ[Invalid]
@@ -126,7 +127,12 @@ func (check *checker) initVar(lhs *Var, x *operand) Type {
 
 	if !check.assignment(x, lhs.typ) {
 		if x.mode != invalid {
-			check.errorf(x.pos(), "cannot initialize variable %s (type %s) with %s", lhs.Name(), lhs.typ, x)
+			if result {
+				// don't refer to lhs.name because it may be an anonymous result parameter
+				check.errorf(x.pos(), "cannot return %s as value of type %s", x, lhs.typ)
+			} else {
+				check.errorf(x.pos(), "cannot initialize %s with %s", lhs, x)
+			}
 		}
 		return nil
 	}
@@ -223,7 +229,7 @@ func (check *checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos)
 		var a [2]Type
 		for i := range a {
 			get(&x, i)
-			a[i] = check.initVar(lhs[i], &x)
+			a[i] = check.initVar(lhs[i], &x, returnPos.IsValid())
 		}
 		check.recordCommaOkTypes(rhs[0], a)
 		return
@@ -231,7 +237,7 @@ func (check *checker) initVars(lhs []*Var, rhs []ast.Expr, returnPos token.Pos)
 
 	for i, lhs := range lhs {
 		get(&x, i)
-		check.initVar(lhs, &x)
+		check.initVar(lhs, &x, returnPos.IsValid())
 	}
 }
 

go/types/decl.go

@@ -146,7 +146,7 @@ func (check *checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
 		assert(lhs == nil || lhs[0] == obj)
 		var x operand
 		check.expr(&x, init)
-		check.initVar(obj, &x)
+		check.initVar(obj, &x, false)
 		return
 	}
 

go/types/stmt.go

@@ -187,7 +187,7 @@ L:
 			if T == nil && t == nil || T != nil && t != nil && Identical(T, t) {
 				// talk about "case" rather than "type" because of nil case
 				check.errorf(e.Pos(), "duplicate case in type switch")
-				check.errorf(pos, "previous case %s", T)
+				check.errorf(pos, "\tprevious case %s", T) // secondary error, \t indented
 				continue L
 			}
 		}
@@ -320,24 +320,27 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 		check.suspendedCall("defer", s.Call)
 
 	case *ast.ReturnStmt:
-		sig := check.sig
-		if n := sig.results.Len(); n > 0 {
-			// determine if the function has named results
-			named := false
-			lhs := make([]*Var, n)
-			for i, res := range sig.results.vars {
-				if res.name != "" {
-					// a blank (_) result parameter is a named result
-					named = true
+		res := check.sig.results
+		if res.Len() > 0 {
+			// function returns results
+			// (if one, say the first, result parameter is named, all of them are named)
+			if len(s.Results) == 0 && res.vars[0].name != "" {
+				// spec: "Implementation restriction: A compiler may disallow an empty expression
+				// list in a "return" statement if a different entity (constant, type, or variable)
+				// with the same name as a result parameter is in scope at the place of the return."
+				for _, obj := range res.vars {
+					if alt := check.scope.LookupParent(obj.name); alt != nil && alt != obj {
+						check.errorf(s.Pos(), "result parameter %s not in scope at return", obj.name)
+						check.errorf(alt.Pos(), "\tinner declaration of %s", obj)
+						// ok to continue
+					}
 				}
-				lhs[i] = res
-			}
-			if len(s.Results) > 0 || !named {
-				check.initVars(lhs, s.Results, s.Return)
-				return
+			} else {
+				// return has results or result parameters are unnamed
+				check.initVars(res.vars, s.Results, s.Return)
 			}
 		} else if len(s.Results) > 0 {
-			check.errorf(s.Pos(), "no result values expected")
+			check.errorf(s.Results[0].Pos(), "no result values expected")
 		}
 
 	case *ast.BranchStmt:
@@ -691,7 +694,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) {
 				x.mode = value
 				x.expr = lhs // we don't have a better rhs expression to use here
 				x.typ = rhs[i]
-				check.initVar(obj, &x)
+				check.initVar(obj, &x, false)
 			}
 
 			// declare variables

go/types/testdata/decls1.src

@@ -69,6 +69,7 @@ var (
 	t18 float64 = math.Pi * 10.0
 	t19 int = t1 /* ERROR "cannot call" */ ()
 	t20 int = f0 /* ERROR "no value" */ ()
+	t21 int = a /* ERROR "cannot initialize" */
 )
 
 // Various more complex expressions

go/types/testdata/stmt0.src

@@ -357,6 +357,37 @@ func continues() {
 	}
 }
 
+func returns0() {
+	return
+	return 0 /* ERROR no result values expected */
+}
+
+func returns1(x float64) (int, *float64) {
+	return 0, &x
+	return /* ERROR wrong number of return values */
+	return "foo" /* ERROR "cannot convert" */, x /* ERROR "cannot return" */
+	return /* ERROR wrong number of return values */ 0, &x, 1
+}
+
+func returns2() (a, b int) {
+	return
+	return 1, "foo" /* ERROR cannot convert */
+	return /* ERROR wrong number of return values */ 1, 2, 3
+	{
+		type a int
+		return 1, 2
+		return /* ERROR a not in scope at return */
+	}
+}
+
+func returns3() (_ int) {
+	return
+	{
+		var _ int // blank (_) identifiers never shadow since they are in no scope
+		return
+	}
+}
+
 func switches0() {
 	var x int
 
@@ -566,13 +597,13 @@ func typeswitch2() {
 
 func typeswitch3(x interface{}) {
 	switch x.(type) {
-	case int /* ERROR previous case int */ :
+	case int:
 	case float64:
 	case int /* ERROR duplicate case */ :
 	}
 
 	switch x.(type) {
-	case nil /* ERROR previous case <nil> */ /* ERROR previous case <nil> */ :
+	case nil:
 	case int:
 	case nil /* ERROR duplicate case */ , nil /* ERROR duplicate case */ :
 	}
@@ -580,7 +611,7 @@ func typeswitch3(x interface{}) {
 	type F func(int)
 	switch x.(type) {
 	case nil:
-	case int, func /* ERROR previous case */ (int):
+	case int, func(int):
 	case float32, func /* ERROR duplicate case */ (x int):
 	case F:
 	}

go/types/typexpr.go

@@ -382,6 +382,7 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 		return
 	}
 
+	var named, anonymous bool
 	for i, field := range list.List {
 		ftype := field.Type
 		if t, _ := ftype.(*ast.Ellipsis); t != nil {
@@ -399,18 +400,29 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 		if len(field.Names) > 0 {
 			// named parameter
 			for _, name := range field.Names {
+				if name.Name == "" {
+					check.invalidAST(name.Pos(), "anonymous parameter")
+					// ok to continue
+				}
 				par := NewParam(name.Pos(), check.pkg, name.Name, typ)
 				check.declare(scope, name, par)
 				params = append(params, par)
 			}
+			named = true
 		} else {
 			// anonymous parameter
 			par := NewParam(ftype.Pos(), check.pkg, "", typ)
 			check.recordImplicit(field, par)
 			params = append(params, par)
+			anonymous = true
 		}
 	}
 
+	if named && anonymous {
+		check.invalidAST(list.Pos(), "list contains both named and anonymous parameters")
+		// ok to continue
+	}
+
 	// For a variadic function, change the last parameter's type from T to []T.
 	if variadic && len(params) > 0 {
 		last := params[len(params)-1]