diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 3a5ae5fd21..bac4d74a87 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -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: diff --git a/gopls/internal/regtest/misc/fix_test.go b/gopls/internal/regtest/misc/fix_test.go index 48abccf12c..8318ae557d 100644 --- a/gopls/internal/regtest/misc/fix_test.go +++ b/gopls/internal/regtest/misc/fix_test.go @@ -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) diff --git a/internal/lsp/analysis/fillreturns/fillreturns.go b/internal/lsp/analysis/fillreturns/fillreturns.go index d34baf5cb1..4607f37c09 100644 --- a/internal/lsp/analysis/fillreturns/fillreturns.go +++ b/internal/lsp/analysis/fillreturns/fillreturns.go @@ -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 } diff --git a/internal/lsp/analysis/fillreturns/fillreturns_test.go b/internal/lsp/analysis/fillreturns/fillreturns_test.go index cc0dbd51ef..7ef0d46792 100644 --- a/internal/lsp/analysis/fillreturns/fillreturns_test.go +++ b/internal/lsp/analysis/fillreturns/fillreturns_test.go @@ -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 { diff --git a/internal/lsp/analysis/fillreturns/testdata/src/a/a.go b/internal/lsp/analysis/fillreturns/testdata/src/a/a.go index 44cb25ffa3..7ab0ff167d 100644 --- a/internal/lsp/analysis/fillreturns/testdata/src/a/a.go +++ b/internal/lsp/analysis/fillreturns/testdata/src/a/a.go @@ -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" } diff --git a/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden b/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden index 1435ea09a5..f007a5f374 100644 --- a/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden +++ b/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden @@ -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" } diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index b7c51632d7..8f807a8448 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -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, }, {