go/types: report types for mismatched call and return statements

This is a port of CL 364874 from types2 to go/types with various
adjustments:

- the error position for "not enough arguments" in calls is the
  closing ) rather than the position of the last provided argument

- the ERROR comments in tests are positioned accordingly

- the reg. expression for matching error strings accepts newlines
  for the . pattern (added s flag)

For #48834.
For #48835.

Change-Id: I64362ecf605bcf9d89b8dc121432e0131bd5da1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/367196
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

src/go/types/assignments.go

@@ -9,7 +9,7 @@ package types
 import (
 	"fmt"
 	"go/ast"
-	"go/token"
+	"strings"
 )
 
 // assignment reports whether x can be assigned to a variable of type T,
@@ -238,6 +238,58 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
 	return x.typ
 }
 
+// operandTypes returns the list of types for the given operands.
+func operandTypes(list []*operand) (res []Type) {
+	for _, x := range list {
+		res = append(res, x.typ)
+	}
+	return res
+}
+
+// varTypes returns the list of types for the given variables.
+func varTypes(list []*Var) (res []Type) {
+	for _, x := range list {
+		res = append(res, x.typ)
+	}
+	return res
+}
+
+// typesSummary returns a string of the form "(t1, t2, ...)" where the
+// ti's are user-friendly string representations for the given types.
+// If variadic is set and the last type is a slice, its string is of
+// the form "...E" where E is the slice's element type.
+func (check *Checker) typesSummary(list []Type, variadic bool) string {
+	var res []string
+	for i, t := range list {
+		var s string
+		switch {
+		case t == nil:
+			fallthrough // should not happend but be cautious
+		case t == Typ[Invalid]:
+			s = "<T>"
+		case isUntyped(t):
+			if isNumeric(t) {
+				// Do not imply a specific type requirement:
+				// "have number, want float64" is better than
+				// "have untyped int, want float64" or
+				// "have int, want float64".
+				s = "number"
+			} else {
+				// If we don't have a number, omit the "untyped" qualifier
+				// for compactness.
+				s = strings.Replace(t.(*Basic).name, "untyped ", "", -1)
+			}
+		case variadic && i == len(list)-1:
+			s = check.sprintf("...%s", t.(*Slice).elem)
+		}
+		if s == "" {
+			s = check.sprintf("%s", t)
+		}
+		res = append(res, s)
+	}
+	return "(" + strings.Join(res, ", ") + ")"
+}
+
 func (check *Checker) assignError(rhs []ast.Expr, nvars, nvals int) {
 	measure := func(x int, unit string) string {
 		s := fmt.Sprintf("%d %s", x, unit)
@@ -260,10 +312,10 @@ func (check *Checker) assignError(rhs []ast.Expr, nvars, nvals int) {
 	check.errorf(rhs0, _WrongAssignCount, "assignment mismatch: %s but %s", vars, vals)
 }
 
-// If returnPos is valid, initVars is called to type-check the assignment of
-// return expressions, and returnPos is the position of the return statement.
-func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnPos token.Pos) {
-	rhs, commaOk := check.exprList(origRHS, len(lhs) == 2 && !returnPos.IsValid())
+// If returnStmt != nil, initVars is called to type-check the assignment
+// of return expressions, and returnStmt is the the return statement.
+func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnStmt ast.Stmt) {
+	rhs, commaOk := check.exprList(origRHS, len(lhs) == 2 && returnStmt == nil)
 
 	if len(lhs) != len(rhs) {
 		// invalidate lhs
@@ -279,8 +331,20 @@ func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnPos token.P
 				return
 			}
 		}
-		if returnPos.IsValid() {
-			check.errorf(atPos(returnPos), _WrongResultCount, "wrong number of return values (want %d, got %d)", len(lhs), len(rhs))
+		if returnStmt != nil {
+			var at positioner = returnStmt
+			qualifier := "not enough"
+			if len(rhs) > len(lhs) {
+				at = rhs[len(lhs)].expr // report at first extra value
+				qualifier = "too many"
+			} else if len(rhs) > 0 {
+				at = rhs[len(rhs)-1].expr // report at last value
+			}
+			check.errorf(at, _WrongResultCount, "%s return values\n\thave %s\n\twant %s",
+				qualifier,
+				check.typesSummary(operandTypes(rhs), false),
+				check.typesSummary(varTypes(lhs), false),
+			)
 			return
 		}
 		if compilerErrorMessages {
@@ -292,7 +356,7 @@ func (check *Checker) initVars(lhs []*Var, origRHS []ast.Expr, returnPos token.P
 	}
 
 	context := "assignment"
-	if returnPos.IsValid() {
+	if returnStmt != nil {
 		context = "return statement"
 	}
 
@@ -404,7 +468,7 @@ func (check *Checker) shortVarDecl(pos positioner, lhs, rhs []ast.Expr) {
 		}
 	}
 
-	check.initVars(lhsVars, rhs, token.NoPos)
+	check.initVars(lhsVars, rhs, nil)
 
 	// process function literals in rhs expressions before scope changes
 	check.processDelayed(top)

src/go/types/call.go

@@ -355,12 +355,25 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, targs []Type
 	}
 
 	// check argument count
-	switch {
-	case nargs < npars:
-		check.errorf(inNode(call, call.Rparen), _WrongArgCount, "not enough arguments in call to %s", call.Fun)
-		return
-	case nargs > npars:
-		check.errorf(args[npars], _WrongArgCount, "too many arguments in call to %s", call.Fun) // report at first extra argument
+	if nargs != npars {
+		var at positioner = call
+		qualifier := "not enough"
+		if nargs > npars {
+			at = args[npars].expr // report at first extra argument
+			qualifier = "too many"
+		} else {
+			at = atPos(call.Rparen) // report at closing )
+		}
+		// take care of empty parameter lists represented by nil tuples
+		var params []*Var
+		if sig.params != nil {
+			params = sig.params.vars
+		}
+		check.errorf(at, _WrongArgCount, "%s arguments in call to %s\n\thave %s\n\twant %s",
+			qualifier, call.Fun,
+			check.typesSummary(operandTypes(args), false),
+			check.typesSummary(varTypes(params), sig.variadic),
+		)
 		return
 	}
 

src/go/types/check_test.go

@@ -51,7 +51,7 @@ var (
 var fset = token.NewFileSet()
 
 // Positioned errors are of the form filename:line:column: message .
-var posMsgRx = regexp.MustCompile(`^(.*:[0-9]+:[0-9]+): *(.*)`)
+var posMsgRx = regexp.MustCompile(`^(.*:[0-9]+:[0-9]+): *(?s)(.*)`)
 
 // splitError splits an error's error message into a position string
 // and the actual error message. If there's no position information,

src/go/types/decl.go

@@ -602,7 +602,7 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) {
 		}
 	}
 
-	check.initVars(lhs, []ast.Expr{init}, token.NoPos)
+	check.initVars(lhs, []ast.Expr{init}, nil)
 }
 
 // isImportedConstraint reports whether typ is an imported type constraint.

src/go/types/stmt.go

@@ -519,7 +519,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
 				}
 			} else {
 				// return has results or result parameters are unnamed
-				check.initVars(res.vars, s.Results, s.Return)
+				check.initVars(res.vars, s.Results, s)
 			}
 		} else if len(s.Results) > 0 {
 			check.error(s.Results[0], _WrongResultCount, "no result values expected")

src/go/types/testdata/check/errors.src

@@ -8,32 +8,38 @@ package errors
 // (matching messages are regular expressions, hence the \'s).
 func f(x int, m map[string]int) {
 	// no values
-	_ = f /* ERROR "f\(0, m\) \(no value\) used as value" */ (0, m)
+	_ = f /* ERROR f\(0, m\) \(no value\) used as value */ (0, m)
 
 	// built-ins
-	_ = println /* ERROR "println \(built-in\) must be called" */
+	_ = println // ERROR println \(built-in\) must be called
 
 	// types
-	_ = complex128 /* ERROR "complex128 \(type\) is not an expression" */
+	_ = complex128 // ERROR complex128 \(type\) is not an expression
 
 	// constants
 	const c1 = 991
 	const c2 float32 = 0.5
-	0 /* ERROR "0 \(untyped int constant\) is not used" */
-	c1 /* ERROR "c1 \(untyped int constant 991\) is not used" */
-	c2 /* ERROR "c2 \(constant 0.5 of type float32\) is not used" */
-	c1 /* ERROR "c1 \+ c2 \(constant 991.5 of type float32\) is not used" */ + c2
+	const c3 = "foo"
+	0 // ERROR 0 \(untyped int constant\) is not used
+	0.5 // ERROR 0.5 \(untyped float constant\) is not used
+	"foo" // ERROR "foo" \(untyped string constant\) is not used
+	c1 // ERROR c1 \(untyped int constant 991\) is not used
+	c2 // ERROR c2 \(constant 0.5 of type float32\) is not used
+	c1 /* ERROR c1 \+ c2 \(constant 991.5 of type float32\) is not used */ + c2
+	c3 // ERROR c3 \(untyped string constant "foo"\) is not used
 
 	// variables
-	x /* ERROR "x \(variable of type int\) is not used" */
+	x // ERROR x \(variable of type int\) is not used
 
 	// values
-	x /* ERROR "x != x \(untyped bool value\) is not used" */ != x
-	x /* ERROR "x \+ x \(value of type int\) is not used" */ + x
+	nil // ERROR nil is not used
+	( /* ERROR \(\*int\)\(nil\) \(value of type \*int\) is not used */ *int)(nil)
+	x /* ERROR x != x \(untyped bool value\) is not used */ != x
+	x /* ERROR x \+ x \(value of type int\) is not used */ + x
 
 	// value, ok's
 	const s = "foo"
-	m /* ERROR "m\[s\] \(map index expression of type int\) is not used" */ [s]
+	m /* ERROR m\[s\] \(map index expression of type int\) is not used */ [s]
 }
 
 // Valid ERROR comments can have a variety of forms.

src/go/types/testdata/check/expr3.src

@@ -493,20 +493,20 @@ func _calls() {
 	f1(0)
 	f1(x)
 	f1(10.0)
-	f1() /* ERROR "not enough arguments" */
-	f1(x, y /* ERROR "too many arguments" */ )
+	f1() /* ERROR "not enough arguments in call to f1\n\thave \(\)\n\twant \(int\)" */
+	f1(x, y /* ERROR "too many arguments in call to f1\n\thave \(int, float32\)\n\twant \(int\)" */ )
 	f1(s /* ERROR "cannot use .* in argument" */ )
 	f1(x ... /* ERROR "cannot use ..." */ )
 	f1(g0 /* ERROR "used as value" */ ())
 	f1(g1())
-	f1(g2 /* ERROR "too many arguments" */ ())
+	f1(g2 /* ERROR "too many arguments in call to f1\n\thave \(float32, string\)\n\twant \(int\)" */ ())
 
-	f2() /* ERROR "not enough arguments" */
-	f2(3.14) /* ERROR "not enough arguments" */
+	f2() /* ERROR "not enough arguments in call to f2\n\thave \(\)\n\twant \(float32, string\)" */
+	f2(3.14) /* ERROR "not enough arguments in call to f2\n\thave \(number\)\n\twant \(float32, string\)" */
 	f2(3.14, "foo")
 	f2(x /* ERROR "cannot use .* in argument" */ , "foo")
 	f2(g0 /* ERROR "used as value" */ ())
-	f2(g1()) /* ERROR "not enough arguments" */
+	f2(g1()) /* ERROR "not enough arguments in call to f2\n\thave \(int\)\n\twant \(float32, string\)" */
 	f2(g2())
 
 	fs() /* ERROR "not enough arguments" */

src/go/types/testdata/check/stmt0.src

@@ -29,10 +29,10 @@ func assignments0() (int, int) {
 
 	a, b, c = <- /* ERROR "cannot assign [1-9]+ values to [1-9]+ variables" */ ch
 
-	return /* ERROR "wrong number of return values" */
-	return /* ERROR "wrong number of return values" */ 1
+	return /* ERROR "not enough return values\n\thave \(\)\n\twant \(int, int\)" */
+	return 1 /* ERROR "not enough return values\n\thave \(number\)\n\twant \(int, int\)" */
 	return 1, 2
-	return /* ERROR "wrong number of return values" */ 1, 2, 3
+	return 1, 2, 3 /* ERROR "too many return values\n\thave \(number, number, number\)\n\twant \(int, int\)" */
 }
 
 func assignments1() {
@@ -81,7 +81,7 @@ func assignments1() {
 	// test cases for issue 5500
 	_ = func() (int, bool) {
 		var m map[int]int
-		return /* ERROR "wrong number of return values" */ m[0]
+		return m /* ERROR "not enough return values" */ [0]
 	}
 
 	g := func(int, bool){}
@@ -380,15 +380,15 @@ func returns0() {
 
 func returns1(x float64) (int, *float64) {
 	return 0, &x
-	return /* ERROR wrong number of return values */
+	return /* ERROR not enough return values */
 	return "foo" /* ERROR "cannot .* in return statement" */, x /* ERROR "cannot use .* in return statement" */
-	return /* ERROR wrong number of return values */ 0, &x, 1
+	return 0, &x, 1 /* ERROR too many return values */
 }
 
 func returns2() (a, b int) {
 	return
 	return 1, "foo" /* ERROR cannot use .* in return statement */
-	return /* ERROR wrong number of return values */ 1, 2, 3
+	return 1, 2, 3 /* ERROR too many return values */
 	{
 		type a int
 		return 1, 2

src/go/types/testdata/check/vardecl.src

@@ -175,7 +175,7 @@ func _() {
 
 func _() int {
 	var x, y int
-	return /* ERROR wrong number of return values */ x, y
+	return x, y /* ERROR too many return values */
 }
 
 // Short variable declarations must declare at least one new non-blank variable.