internal/lsp/analysis/fillreturns: update fillreturns for new type errs

The error messages produced by go/types related to an incorrect number
of return values have changed, and have been repositioned. Update the
fillreturns analyzer to handle the Go 1.18 form of these errors.

Ideally we'd use error codes here, but can't because they are not
forwarded by go/packages. Added a TODO to revisit when error codes are
exposed.

Change-Id: I4c594fd2fd1cb7423f37ff9c6a57e9490dcbc2a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/367714
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
This commit is contained in:
Robert Findley 2021-11-29 23:01:21 -05:00
parent e212aff8fd
commit f64c0f46fa
7 changed files with 101 additions and 81 deletions

View File

@ -578,7 +578,7 @@ check for constraints that could be simplified to "any"
## **fillreturns**
suggested fixes for "wrong number of return values (want %d, got %d)"
suggest fixes for errors due to an incorrect number of return values
This checker provides suggested fixes for type errors of the
type "wrong number of return values (want %d, got %d)". For example:

View File

@ -62,7 +62,6 @@ func Foo() {
}
func TestFillReturns(t *testing.T) {
t.Skip("temporarily skipped until CL 367196 is submitted")
const files = `
-- go.mod --
module mod.com
@ -79,7 +78,8 @@ func Foo() error {
env.OpenFile("main.go")
var d protocol.PublishDiagnosticsParams
env.Await(OnceMet(
env.DiagnosticAtRegexpWithMessage("main.go", `return`, "wrong number of return values"),
// The error message here changed in 1.18; "return values" covers both forms.
env.DiagnosticAtRegexpWithMessage("main.go", `return`, "return values"),
ReadDiagnostics("main.go", &d),
))
codeActions := env.CodeAction("main.go", d.Diagnostics)

View File

@ -14,7 +14,6 @@ import (
"go/format"
"go/types"
"regexp"
"strconv"
"strings"
"golang.org/x/tools/go/analysis"
@ -23,7 +22,7 @@ import (
"golang.org/x/tools/internal/typeparams"
)
const Doc = `suggested fixes for "wrong number of return values (want %d, got %d)"
const Doc = `suggest fixes for errors due to an incorrect number of return values
This checker provides suggested fixes for type errors of the
type "wrong number of return values (want %d, got %d)". For example:
@ -46,8 +45,6 @@ var Analyzer = &analysis.Analyzer{
RunDespiteErrors: true,
}
var wrongReturnNumRegex = regexp.MustCompile(`wrong number of return values \(want (\d+), got (\d+)\)`)
func run(pass *analysis.Pass) (interface{}, error) {
info := pass.TypesInfo
if info == nil {
@ -58,7 +55,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
outer:
for _, typeErr := range errors {
// Filter out the errors that are not relevant to this analyzer.
if !FixesError(typeErr.Msg) {
if !FixesError(typeErr) {
continue
}
var file *ast.File
@ -79,20 +76,32 @@ outer:
}
typeErrEndPos := analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), typeErr.Pos)
// TODO(rfindley): much of the error handling code below returns, when it
// should probably continue.
// Get the path for the relevant range.
path, _ := astutil.PathEnclosingInterval(file, typeErr.Pos, typeErrEndPos)
if len(path) == 0 {
return nil, nil
}
// Check to make sure the node of interest is a ReturnStmt.
ret, ok := path[0].(*ast.ReturnStmt)
if !ok {
// Find the enclosing return statement.
var ret *ast.ReturnStmt
var retIdx int
for i, n := range path {
if r, ok := n.(*ast.ReturnStmt); ok {
ret = r
retIdx = i
break
}
}
if ret == nil {
return nil, nil
}
// Get the function type that encloses the ReturnStmt.
var enclosingFunc *ast.FuncType
for _, n := range path {
for _, n := range path[retIdx+1:] {
switch node := n.(type) {
case *ast.FuncLit:
enclosingFunc = node.Type
@ -109,7 +118,7 @@ outer:
// Skip any generic enclosing functions, since type parameters don't
// have 0 values.
// TODO(rstambler): We should be able to handle this if the return
// TODO(rfindley): We should be able to handle this if the return
// values are all concrete types.
if tparams := typeparams.ForFuncType(enclosingFunc); tparams != nil && tparams.NumFields() > 0 {
return nil, nil
@ -127,7 +136,8 @@ outer:
return nil, nil
}
// Skip any return statements that contain function calls with multiple return values.
// Skip any return statements that contain function calls with multiple
// return values.
for _, expr := range ret.Results {
e, ok := expr.(*ast.CallExpr)
if !ok {
@ -244,16 +254,23 @@ func matchingTypes(want, got types.Type) bool {
return types.AssignableTo(want, got) || types.ConvertibleTo(want, got)
}
func FixesError(msg string) bool {
matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(msg))
if len(matches) < 3 {
return false
}
if _, err := strconv.Atoi(matches[1]); err != nil {
return false
}
if _, err := strconv.Atoi(matches[2]); err != nil {
return false
}
return true
// Error messages have changed across Go versions. These regexps capture recent
// incarnations.
//
// TODO(rfindley): once error codes are exported and exposed via go/packages,
// use error codes rather than string matching here.
var wrongReturnNumRegexes = []*regexp.Regexp{
regexp.MustCompile(`wrong number of return values \(want (\d+), got (\d+)\)`),
regexp.MustCompile(`too many return values`),
regexp.MustCompile(`not enough return values`),
}
func FixesError(err types.Error) bool {
msg := strings.TrimSpace(err.Msg)
for _, rx := range wrongReturnNumRegexes {
if rx.MatchString(msg) {
return true
}
}
return false
}

View File

@ -13,7 +13,6 @@ import (
)
func Test(t *testing.T) {
t.Skip("temporarily skipped until CL 367196 is submitted and this test is adjusted accordingly")
testdata := analysistest.TestData()
tests := []string{"a"}
if typeparams.Enabled {

View File

@ -25,80 +25,82 @@ func x() error {
return errors.New("foo")
}
// The error messages below changed in 1.18; "return values" covers both forms.
func b() (string, int, error) {
return "", errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return "", errors.New("foo") // want "return values"
}
func c() (string, int, error) {
return 7, errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return 7, errors.New("foo") // want "return values"
}
func d() (string, int, error) {
return "", 7 // want "wrong number of return values \\(want 3, got 2\\)"
return "", 7 // want "return values"
}
func e() (T, error, *bool) {
return (z(http.ListenAndServe))("", nil) // want "wrong number of return values \\(want 3, got 1\\)"
return (z(http.ListenAndServe))("", nil) // want "return values"
}
func preserveLeft() (int, int, error) {
return 1, errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return 1, errors.New("foo") // want "return values"
}
func matchValues() (int, error, string) {
return errors.New("foo"), 3 // want "wrong number of return values \\(want 3, got 2\\)"
return errors.New("foo"), 3 // want "return values"
}
func preventDataOverwrite() (int, string) {
return errors.New("foo") // want "wrong number of return values \\(want 2, got 1\\)"
return errors.New("foo") // want "return values"
}
func closure() (string, error) {
_ = func() (int, error) {
return // want "wrong number of return values \\(want 2, got 0\\)"
return // want "return values"
}
return // want "wrong number of return values \\(want 2, got 0\\)"
return // want "return values"
}
func basic() (uint8, uint16, uint32, uint64, int8, int16, int32, int64, float32, float64, complex64, complex128, byte, rune, uint, int, uintptr, string, bool, error) {
return // want "wrong number of return values \\(want 20, got 0\\)"
return // want "return values"
}
func complex() (*int, []int, [2]int, map[int]int) {
return // want "wrong number of return values \\(want 4, got 0\\)"
return // want "return values"
}
func structsAndInterfaces() (T, url.URL, T1, I, I1, io.Reader, Client, ast2.Stmt) {
return // want "wrong number of return values \\(want 8, got 0\\)"
return // want "return values"
}
func m() (int, error) {
if 1 == 2 {
return // want "wrong number of return values \\(want 2, got 0\\)"
return // want "return values"
} else if 1 == 3 {
return errors.New("foo") // want "wrong number of return values \\(want 2, got 1\\)"
return errors.New("foo") // want "return values"
} else {
return 1 // want "wrong number of return values \\(want 2, got 1\\)"
return 1 // want "return values"
}
return // want "wrong number of return values \\(want 2, got 0\\)"
return // want "return values"
}
func convertibleTypes() (ast2.Expr, int) {
return &ast2.ArrayType{} // want "wrong number of return values \\(want 2, got 1\\)"
return &ast2.ArrayType{} // want "return values"
}
func assignableTypes() (map[string]int, int) {
type X map[string]int
var x X
return x // want "wrong number of return values \\(want 2, got 1\\)"
return x // want "return values"
}
func interfaceAndError() (I, int) {
return errors.New("foo") // want "wrong number of return values \\(want 2, got 1\\)"
return errors.New("foo") // want "return values"
}
func funcOneReturn() (string, error) {
return strconv.Itoa(1) // want "wrong number of return values \\(want 2, got 1\\)"
return strconv.Itoa(1) // want "return values"
}
func funcMultipleReturn() (int, error, string) {
@ -110,16 +112,16 @@ func localFuncMultipleReturn() (string, int, error, string) {
}
func multipleUnused() (int, string, string, string) {
return 3, 4, 5 // want "wrong number of return values \\(want 4, got 3\\)"
return 3, 4, 5 // want "return values"
}
func gotTooMany() int {
if true {
return 0, "" // want "wrong number of return values \\(want 1, got 2\\)"
return 0, "" // want "return values"
} else {
return 1, 0, nil // want "wrong number of return values \\(want 1, got 3\\)"
return 1, 0, nil // want "return values"
}
return 0, 5, false // want "wrong number of return values \\(want 1, got 3\\)"
return 0, 5, false // want "return values"
}
func fillVars() (int, string, ast.Node, bool, error) {
@ -128,10 +130,10 @@ func fillVars() (int, string, ast.Node, bool, error) {
var t bool
if true {
err := errors.New("fail")
return // want "wrong number of return values \\(want 5, got 0\\)"
return // want "return values"
}
n := ast.NewIdent("ident")
int := 3
var b bool
return "" // want "wrong number of return values \\(want 5, got 1\\)"
return "" // want "return values"
}

View File

@ -25,80 +25,82 @@ func x() error {
return errors.New("foo")
}
// The error messages below changed in 1.18; "return values" covers both forms.
func b() (string, int, error) {
return "", 0, errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return "", 0, errors.New("foo") // want "return values"
}
func c() (string, int, error) {
return "", 7, errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return "", 7, errors.New("foo") // want "return values"
}
func d() (string, int, error) {
return "", 7, nil // want "wrong number of return values \\(want 3, got 2\\)"
return "", 7, nil // want "return values"
}
func e() (T, error, *bool) {
return T{}, (z(http.ListenAndServe))("", nil), nil // want "wrong number of return values \\(want 3, got 1\\)"
return T{}, (z(http.ListenAndServe))("", nil), nil // want "return values"
}
func preserveLeft() (int, int, error) {
return 1, 0, errors.New("foo") // want "wrong number of return values \\(want 3, got 2\\)"
return 1, 0, errors.New("foo") // want "return values"
}
func matchValues() (int, error, string) {
return 3, errors.New("foo"), "" // want "wrong number of return values \\(want 3, got 2\\)"
return 3, errors.New("foo"), "" // want "return values"
}
func preventDataOverwrite() (int, string) {
return 0, "", errors.New("foo") // want "wrong number of return values \\(want 2, got 1\\)"
return 0, "", errors.New("foo") // want "return values"
}
func closure() (string, error) {
_ = func() (int, error) {
return 0, nil // want "wrong number of return values \\(want 2, got 0\\)"
return 0, nil // want "return values"
}
return "", nil // want "wrong number of return values \\(want 2, got 0\\)"
return "", nil // want "return values"
}
func basic() (uint8, uint16, uint32, uint64, int8, int16, int32, int64, float32, float64, complex64, complex128, byte, rune, uint, int, uintptr, string, bool, error) {
return 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "", false, nil // want "wrong number of return values \\(want 20, got 0\\)"
return 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "", false, nil // want "return values"
}
func complex() (*int, []int, [2]int, map[int]int) {
return nil, nil, nil, nil // want "wrong number of return values \\(want 4, got 0\\)"
return nil, nil, nil, nil // want "return values"
}
func structsAndInterfaces() (T, url.URL, T1, I, I1, io.Reader, Client, ast2.Stmt) {
return T{}, url.URL{}, T{}, nil, nil, nil, Client{}, nil // want "wrong number of return values \\(want 8, got 0\\)"
return T{}, url.URL{}, T{}, nil, nil, nil, Client{}, nil // want "return values"
}
func m() (int, error) {
if 1 == 2 {
return 0, nil // want "wrong number of return values \\(want 2, got 0\\)"
return 0, nil // want "return values"
} else if 1 == 3 {
return 0, errors.New("foo") // want "wrong number of return values \\(want 2, got 1\\)"
return 0, errors.New("foo") // want "return values"
} else {
return 1, nil // want "wrong number of return values \\(want 2, got 1\\)"
return 1, nil // want "return values"
}
return 0, nil // want "wrong number of return values \\(want 2, got 0\\)"
return 0, nil // want "return values"
}
func convertibleTypes() (ast2.Expr, int) {
return &ast2.ArrayType{}, 0 // want "wrong number of return values \\(want 2, got 1\\)"
return &ast2.ArrayType{}, 0 // want "return values"
}
func assignableTypes() (map[string]int, int) {
type X map[string]int
var x X
return x, 0 // want "wrong number of return values \\(want 2, got 1\\)"
return x, 0 // want "return values"
}
func interfaceAndError() (I, int) {
return errors.New("foo"), 0 // want "wrong number of return values \\(want 2, got 1\\)"
return errors.New("foo"), 0 // want "return values"
}
func funcOneReturn() (string, error) {
return strconv.Itoa(1), nil // want "wrong number of return values \\(want 2, got 1\\)"
return strconv.Itoa(1), nil // want "return values"
}
func funcMultipleReturn() (int, error, string) {
@ -110,16 +112,16 @@ func localFuncMultipleReturn() (string, int, error, string) {
}
func multipleUnused() (int, string, string, string) {
return 3, "", "", "", 4, 5 // want "wrong number of return values \\(want 4, got 3\\)"
return 3, "", "", "", 4, 5 // want "return values"
}
func gotTooMany() int {
if true {
return 0 // want "wrong number of return values \\(want 1, got 2\\)"
return 0 // want "return values"
} else {
return 1 // want "wrong number of return values \\(want 1, got 3\\)"
return 1 // want "return values"
}
return 5 // want "wrong number of return values \\(want 1, got 3\\)"
return 5 // want "return values"
}
func fillVars() (int, string, ast.Node, bool, error) {
@ -128,10 +130,10 @@ func fillVars() (int, string, ast.Node, bool, error) {
var t bool
if true {
err := errors.New("fail")
return eint, s, nil, false, err // want "wrong number of return values \\(want 5, got 0\\)"
return eint, s, nil, false, err // want "return values"
}
n := ast.NewIdent("ident")
int := 3
var b bool
return int, "", n, b, nil // want "wrong number of return values \\(want 5, got 1\\)"
return int, "", n, b, nil // want "return values"
}

View File

@ -563,7 +563,7 @@ var GeneratedAPIJSON = &APIJSON{
},
{
Name: "\"fillreturns\"",
Doc: "suggested fixes for \"wrong number of return values (want %d, got %d)\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n",
Doc: "suggest fixes for errors due to an incorrect number of return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n",
Default: "true",
},
{
@ -1141,7 +1141,7 @@ var GeneratedAPIJSON = &APIJSON{
},
{
Name: "fillreturns",
Doc: "suggested fixes for \"wrong number of return values (want %d, got %d)\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n",
Doc: "suggest fixes for errors due to an incorrect number of return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n",
Default: true,
},
{