From 2f6d3820501b34ce530be8193789659c18db0867 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 24 Nov 2021 16:48:00 -0800 Subject: [PATCH] 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 Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Robert Findley --- src/go/types/assignments.go | 82 ++++++++++++++++++++++--- src/go/types/call.go | 25 ++++++-- src/go/types/check_test.go | 2 +- src/go/types/decl.go | 2 +- src/go/types/stmt.go | 2 +- src/go/types/testdata/check/errors.src | 28 +++++---- src/go/types/testdata/check/expr3.src | 12 ++-- src/go/types/testdata/check/stmt0.src | 14 ++--- src/go/types/testdata/check/vardecl.src | 2 +- 9 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index 8e9724e911d..a556e5e017c 100644 --- a/src/go/types/assignments.go +++ b/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 = "" + 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) diff --git a/src/go/types/call.go b/src/go/types/call.go index 940c0ff468f..280ed05d1b4 100644 --- a/src/go/types/call.go +++ b/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 } diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index a3be47e371c..e296d13be91 100644 --- a/src/go/types/check_test.go +++ b/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, diff --git a/src/go/types/decl.go b/src/go/types/decl.go index c85087018cd..2c51329be93 100644 --- a/src/go/types/decl.go +++ b/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. diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go index ee7d4e4cf1e..06c9d3175db 100644 --- a/src/go/types/stmt.go +++ b/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") diff --git a/src/go/types/testdata/check/errors.src b/src/go/types/testdata/check/errors.src index ff929217c4c..7cdc5fb5fff 100644 --- a/src/go/types/testdata/check/errors.src +++ b/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. diff --git a/src/go/types/testdata/check/expr3.src b/src/go/types/testdata/check/expr3.src index 5117a0373b8..b8f96dc6113 100644 --- a/src/go/types/testdata/check/expr3.src +++ b/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" */ diff --git a/src/go/types/testdata/check/stmt0.src b/src/go/types/testdata/check/stmt0.src index 2cce0b59b2b..c7a718de70e 100644 --- a/src/go/types/testdata/check/stmt0.src +++ b/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 diff --git a/src/go/types/testdata/check/vardecl.src b/src/go/types/testdata/check/vardecl.src index 54f5ef1e10d..787f7878f14 100644 --- a/src/go/types/testdata/check/vardecl.src +++ b/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.