diff options
Diffstat (limited to 'internal/lsp/analysis')
53 files changed, 1703 insertions, 141 deletions
diff --git a/internal/lsp/analysis/fillreturns/fillreturns.go b/internal/lsp/analysis/fillreturns/fillreturns.go index 94accef62..4607f37c0 100644 --- a/internal/lsp/analysis/fillreturns/fillreturns.go +++ b/internal/lsp/analysis/fillreturns/fillreturns.go @@ -14,15 +14,15 @@ import ( "go/format" "go/types" "regexp" - "strconv" "strings" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/internal/analysisinternal" + "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: @@ -45,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 { @@ -57,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 @@ -78,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 @@ -106,6 +116,14 @@ outer: continue } + // Skip any generic enclosing functions, since type parameters don't + // have 0 values. + // 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 + } + // Find the function declaration that encloses the ReturnStmt. var outer *ast.FuncDecl for _, p := range path { @@ -118,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 { @@ -235,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 +// 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 true + return false } diff --git a/internal/lsp/analysis/fillreturns/fillreturns_test.go b/internal/lsp/analysis/fillreturns/fillreturns_test.go index d1ad6566d..7ef0d4679 100644 --- a/internal/lsp/analysis/fillreturns/fillreturns_test.go +++ b/internal/lsp/analysis/fillreturns/fillreturns_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/fillreturns" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, fillreturns.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, fillreturns.Analyzer, tests...) } diff --git a/internal/lsp/analysis/fillreturns/testdata/src/a/a.go b/internal/lsp/analysis/fillreturns/testdata/src/a/a.go index 44cb25ffa..7ab0ff167 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 1435ea09a..f007a5f37 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/analysis/fillreturns/testdata/src/a/typeparams/a.go b/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go new file mode 100644 index 000000000..8454bd2ce --- /dev/null +++ b/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go @@ -0,0 +1,5 @@ +package fillreturns + +func hello[T any]() int { + return +} diff --git a/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden b/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden new file mode 100644 index 000000000..8454bd2ce --- /dev/null +++ b/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden @@ -0,0 +1,5 @@ +package fillreturns + +func hello[T any]() int { + return +} diff --git a/internal/lsp/analysis/fillstruct/fillstruct.go b/internal/lsp/analysis/fillstruct/fillstruct.go index 36a63a1f6..a4dd8ccb8 100644 --- a/internal/lsp/analysis/fillstruct/fillstruct.go +++ b/internal/lsp/analysis/fillstruct/fillstruct.go @@ -13,6 +13,7 @@ import ( "go/format" "go/token" "go/types" + "strings" "unicode" "golang.org/x/tools/go/analysis" @@ -21,6 +22,7 @@ import ( "golang.org/x/tools/go/ast/inspector" "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/typeparams" ) const Doc = `note incomplete struct initializations @@ -65,6 +67,14 @@ func run(pass *analysis.Pass) (interface{}, error) { return } + // Ignore types that have type parameters for now. + // TODO: support type params. + if typ, ok := typ.(*types.Named); ok { + if tparams := typeparams.ForNamed(typ); tparams != nil && tparams.Len() > 0 { + return + } + } + // Find reference to the type declaration of the struct being initialized. for { p, ok := typ.Underlying().(*types.Pointer) @@ -87,13 +97,25 @@ func run(pass *analysis.Pass) (interface{}, error) { } var fillable bool + var fillableFields []string for i := 0; i < fieldCount; i++ { field := obj.Field(i) // Ignore fields that are not accessible in the current package. if field.Pkg() != nil && field.Pkg() != pass.Pkg && !field.Exported() { continue } + // Ignore structs containing fields that have type parameters for now. + // TODO: support type params. + if typ, ok := field.Type().(*types.Named); ok { + if tparams := typeparams.ForNamed(typ); tparams != nil && tparams.Len() > 0 { + return + } + } + if _, ok := field.Type().(*typeparams.TypeParam); ok { + return + } fillable = true + fillableFields = append(fillableFields, fmt.Sprintf("%s: %s", field.Name(), field.Type().String())) } if !fillable { return @@ -105,7 +127,21 @@ func run(pass *analysis.Pass) (interface{}, error) { case *ast.SelectorExpr: name = fmt.Sprintf("%s.%s", typ.X, typ.Sel.Name) default: - name = "anonymous struct" + totalFields := len(fillableFields) + maxLen := 20 + // Find the index to cut off printing of fields. + var i, fieldLen int + for i = range fillableFields { + if fieldLen > maxLen { + break + } + fieldLen += len(fillableFields[i]) + } + fillableFields = fillableFields[:i] + if i < totalFields { + fillableFields = append(fillableFields, "...") + } + name = fmt.Sprintf("anonymous struct { %s }", strings.Join(fillableFields, ", ")) } pass.Report(analysis.Diagnostic{ Message: fmt.Sprintf("Fill %s", name), diff --git a/internal/lsp/analysis/fillstruct/fillstruct_test.go b/internal/lsp/analysis/fillstruct/fillstruct_test.go index 34c9923e5..51a516cdf 100644 --- a/internal/lsp/analysis/fillstruct/fillstruct_test.go +++ b/internal/lsp/analysis/fillstruct/fillstruct_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/fillstruct" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, fillstruct.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.Run(t, testdata, fillstruct.Analyzer, tests...) } diff --git a/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go b/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..90290613d --- /dev/null +++ b/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go @@ -0,0 +1,41 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fillstruct + +type emptyStruct[A any] struct{} + +var _ = emptyStruct[int]{} + +type basicStruct[T any] struct { + foo T +} + +var _ = basicStruct[int]{} + +type fooType[T any] T + +type twoArgStruct[F, B any] struct { + foo fooType[F] + bar fooType[B] +} + +var _ = twoArgStruct[string, int]{} + +var _ = twoArgStruct[int, string]{ + bar: "bar", +} + +type nestedStruct struct { + bar string + basic basicStruct[int] +} + +var _ = nestedStruct{} + +func _[T any]() { + type S struct{ t T } + x := S{} + _ = x +} diff --git a/internal/lsp/analysis/infertypeargs/infertypeargs.go b/internal/lsp/analysis/infertypeargs/infertypeargs.go new file mode 100644 index 000000000..119de50ce --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/infertypeargs.go @@ -0,0 +1,31 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package infertypeargs defines an analyzer that checks for explicit function +// arguments that could be inferred. +package infertypeargs + +import ( + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" +) + +const Doc = `check for unnecessary type arguments in call expressions + +Explicit type arguments may be omitted from call expressions if they can be +inferred from function arguments, or from other type arguments: + + func f[T any](T) {} + + func _() { + f[string]("foo") // string could be inferred + } +` + +var Analyzer = &analysis.Analyzer{ + Name: "infertypeargs", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} diff --git a/internal/lsp/analysis/infertypeargs/infertypeargs_test.go b/internal/lsp/analysis/infertypeargs/infertypeargs_test.go new file mode 100644 index 000000000..2957f46e3 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/infertypeargs_test.go @@ -0,0 +1,23 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package infertypeargs_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/infertypeargs" + "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testenv.NeedsGo1Point(t, 13) + if !typeparams.Enabled { + t.Skip("type params are not enabled") + } + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, infertypeargs.Analyzer, "a") +} diff --git a/internal/lsp/analysis/infertypeargs/run_go117.go b/internal/lsp/analysis/infertypeargs/run_go117.go new file mode 100644 index 000000000..bc5c29b51 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/run_go117.go @@ -0,0 +1,16 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !go1.18 +// +build !go1.18 + +package infertypeargs + +import "golang.org/x/tools/go/analysis" + +// This analyzer only relates to go1.18+, and uses the types.CheckExpr API that +// was added in Go 1.13. +func run(pass *analysis.Pass) (interface{}, error) { + return nil, nil +} diff --git a/internal/lsp/analysis/infertypeargs/run_go118.go b/internal/lsp/analysis/infertypeargs/run_go118.go new file mode 100644 index 000000000..66457429a --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/run_go118.go @@ -0,0 +1,111 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build go1.18 +// +build go1.18 + +package infertypeargs + +import ( + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/internal/typeparams" +) + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + inspect.Preorder(nodeFilter, func(node ast.Node) { + call := node.(*ast.CallExpr) + x, lbrack, indices, rbrack := typeparams.UnpackIndexExpr(call.Fun) + ident := calledIdent(x) + if ident == nil || len(indices) == 0 { + return // no explicit args, nothing to do + } + + // Confirm that instantiation actually occurred at this ident. + idata, ok := typeparams.GetInstances(pass.TypesInfo)[ident] + if !ok { + return // something went wrong, but fail open + } + instance := idata.Type + + // Start removing argument expressions from the right, and check if we can + // still infer the call expression. + required := len(indices) // number of type expressions that are required + for i := len(indices) - 1; i >= 0; i-- { + var fun ast.Expr + if i == 0 { + // No longer an index expression: just use the parameterized operand. + fun = x + } else { + fun = typeparams.PackIndexExpr(x, lbrack, indices[:i], indices[i-1].End()) + } + newCall := &ast.CallExpr{ + Fun: fun, + Lparen: call.Lparen, + Args: call.Args, + Ellipsis: call.Ellipsis, + Rparen: call.Rparen, + } + info := new(types.Info) + typeparams.InitInstanceInfo(info) + if err := types.CheckExpr(pass.Fset, pass.Pkg, call.Pos(), newCall, info); err != nil { + // Most likely inference failed. + break + } + newIData := typeparams.GetInstances(info)[ident] + newInstance := newIData.Type + if !types.Identical(instance, newInstance) { + // The inferred result type does not match the original result type, so + // this simplification is not valid. + break + } + required = i + } + if required < len(indices) { + var start, end token.Pos + var edit analysis.TextEdit + if required == 0 { + start, end = lbrack, rbrack+1 // erase the entire index + edit = analysis.TextEdit{Pos: start, End: end} + } else { + start = indices[required].Pos() + end = rbrack + // erase from end of last arg to include last comma & white-spaces + edit = analysis.TextEdit{Pos: indices[required-1].End(), End: end} + } + pass.Report(analysis.Diagnostic{ + Pos: start, + End: end, + Message: "unnecessary type arguments", + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "simplify type arguments", + TextEdits: []analysis.TextEdit{edit}, + }}, + }) + } + }) + + return nil, nil +} + +func calledIdent(x ast.Expr) *ast.Ident { + switch x := x.(type) { + case *ast.Ident: + return x + case *ast.SelectorExpr: + return x.Sel + } + return nil +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go new file mode 100644 index 000000000..1c3d88ba1 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go @@ -0,0 +1,20 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the infertyepargs checker. + +package a + +func f[T any](T) {} + +func g[T any]() T { var x T; return x } + +func h[P interface{ ~*T }, T any]() {} + +func _() { + f[string]("hello") // want "unnecessary type arguments" + f[int](2) // want "unnecessary type arguments" + _ = g[int]() + h[*int, int]() // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden new file mode 100644 index 000000000..72348ff77 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden @@ -0,0 +1,20 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the infertyepargs checker. + +package a + +func f[T any](T) {} + +func g[T any]() T { var x T; return x } + +func h[P interface{ ~*T }, T any]() {} + +func _() { + f("hello") // want "unnecessary type arguments" + f(2) // want "unnecessary type arguments" + _ = g[int]() + h[*int]() // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go new file mode 100644 index 000000000..fc1f763df --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go @@ -0,0 +1,12 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import "a/imported" + +func _() { + var x int + imported.F[int](x) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden new file mode 100644 index 000000000..6099545bb --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden @@ -0,0 +1,12 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import "a/imported" + +func _() { + var x int + imported.F(x) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go new file mode 100644 index 000000000..f0610a8b4 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go @@ -0,0 +1,7 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package imported + +func F[T any](T) {} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go new file mode 100644 index 000000000..c304f1d0d --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go @@ -0,0 +1,26 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// We should not suggest removing type arguments if doing so would change the +// resulting type. + +package a + +func id[T any](t T) T { return t } + +var _ = id[int](1) // want "unnecessary type arguments" +var _ = id[string]("foo") // want "unnecessary type arguments" +var _ = id[int64](2) + +func pair[T any](t T) (T, T) { return t, t } + +var _, _ = pair[int](3) // want "unnecessary type arguments" +var _, _ = pair[int64](3) + +func noreturn[T any](t T) {} + +func _() { + noreturn[int64](4) + noreturn[int](4) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden new file mode 100644 index 000000000..93c6f707c --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden @@ -0,0 +1,26 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// We should not suggest removing type arguments if doing so would change the +// resulting type. + +package a + +func id[T any](t T) T { return t } + +var _ = id(1) // want "unnecessary type arguments" +var _ = id("foo") // want "unnecessary type arguments" +var _ = id[int64](2) + +func pair[T any](t T) (T, T) { return t, t } + +var _, _ = pair(3) // want "unnecessary type arguments" +var _, _ = pair[int64](3) + +func noreturn[T any](t T) {} + +func _() { + noreturn[int64](4) + noreturn(4) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/nonewvars/nonewvars_test.go b/internal/lsp/analysis/nonewvars/nonewvars_test.go index 3983bc523..dc58ab0ff 100644 --- a/internal/lsp/analysis/nonewvars/nonewvars_test.go +++ b/internal/lsp/analysis/nonewvars/nonewvars_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/nonewvars" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, nonewvars.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, nonewvars.Analyzer, tests...) } diff --git a/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go b/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go new file mode 100644 index 000000000..b381c9c09 --- /dev/null +++ b/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go @@ -0,0 +1,6 @@ +package nonewvars + +func hello[T any]() int { + var z T + z := 1 // want "no new variables on left side of :=" +} diff --git a/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go.golden b/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go.golden new file mode 100644 index 000000000..3a5117301 --- /dev/null +++ b/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go.golden @@ -0,0 +1,6 @@ +package nonewvars + +func hello[T any]() int { + var z T + z = 1 // want "no new variables on left side of :=" +} diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues.go b/internal/lsp/analysis/noresultvalues/noresultvalues.go index 0e6b26f8b..b9f21f313 100644 --- a/internal/lsp/analysis/noresultvalues/noresultvalues.go +++ b/internal/lsp/analysis/noresultvalues/noresultvalues.go @@ -10,6 +10,7 @@ import ( "bytes" "go/ast" "go/format" + "strings" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -17,10 +18,11 @@ import ( "golang.org/x/tools/internal/analysisinternal" ) -const Doc = `suggested fixes for "no result values expected" +const Doc = `suggested fixes for unexpected return values This checker provides suggested fixes for type errors of the -type "no result values expected". For example: +type "no result values expected" or "too many return values". +For example: func z() { return nil } will turn into func z() { return } @@ -83,5 +85,6 @@ func run(pass *analysis.Pass) (interface{}, error) { } func FixesError(msg string) bool { - return msg == "no result values expected" + return msg == "no result values expected" || + strings.HasPrefix(msg, "too many return values") && strings.Contains(msg, "want ()") } diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues_test.go b/internal/lsp/analysis/noresultvalues/noresultvalues_test.go index 6b9451bf2..12198a1c1 100644 --- a/internal/lsp/analysis/noresultvalues/noresultvalues_test.go +++ b/internal/lsp/analysis/noresultvalues/noresultvalues_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/noresultvalues" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, noresultvalues.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, noresultvalues.Analyzer, tests...) } diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go index 30265a42f..3daa7f7c7 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go +++ b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go @@ -4,6 +4,6 @@ package noresultvalues -func x() { return nil } // want "no result values expected" +func x() { return nil } // want `no result values expected|too many return values` -func y() { return nil, "hello" } // want "no result values expected" +func y() { return nil, "hello" } // want `no result values expected|too many return values` diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden index 6b29cefa3..5e93aa413 100644 --- a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden +++ b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden @@ -4,6 +4,6 @@ package noresultvalues -func x() { return } // want "no result values expected" +func x() { return } // want `no result values expected|too many return values` -func y() { return } // want "no result values expected" +func y() { return } // want `no result values expected|too many return values` diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go new file mode 100644 index 000000000..f8aa43665 --- /dev/null +++ b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go @@ -0,0 +1,6 @@ +package noresult + +func hello[T any]() { + var z T + return z // want `no result values expected|too many return values` +} diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden new file mode 100644 index 000000000..963e3f4e1 --- /dev/null +++ b/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden @@ -0,0 +1,6 @@ +package noresult + +func hello[T any]() { + var z T + return // want `no result values expected|too many return values` +} diff --git a/internal/lsp/analysis/simplifyslice/simplifyslice_test.go b/internal/lsp/analysis/simplifyslice/simplifyslice_test.go index 91db76ae0..cff6267c6 100644 --- a/internal/lsp/analysis/simplifyslice/simplifyslice_test.go +++ b/internal/lsp/analysis/simplifyslice/simplifyslice_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/simplifyslice" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, simplifyslice.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, simplifyslice.Analyzer, tests...) } diff --git a/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go b/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..69db3100a --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go @@ -0,0 +1,39 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +// +//go:build go1.18 +// +build go1.18 + +package testdata + +type List[E any] []E + +// TODO(suzmue): add a test for generic slice expressions when https://github.com/golang/go/issues/48618 is closed. +// type S interface{ ~[]int } + +var ( + a [10]byte + b [20]float32 + p List[int] + + _ = p[0:] + _ = p[1:10] + _ = p[2:len(p)] // want "unneeded: len\\(p\\)" + _ = p[3:(len(p))] + _ = p[len(a) : len(p)-1] + _ = p[0:len(b)] + _ = p[2:len(p):len(p)] + + _ = p[:] + _ = p[:10] + _ = p[:len(p)] // want "unneeded: len\\(p\\)" + _ = p[:(len(p))] + _ = p[:len(p)-1] + _ = p[:len(b)] + _ = p[:len(p):len(p)] +) + +func foo[E any](a List[E]) { + _ = a[0:len(a)] // want "unneeded: len\\(a\\)" +} diff --git a/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go.golden b/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go.golden new file mode 100644 index 000000000..99ca9e447 --- /dev/null +++ b/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go.golden @@ -0,0 +1,39 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +// +//go:build go1.18 +// +build go1.18 + +package testdata + +type List[E any] []E + +// TODO(suzmue): add a test for generic slice expressions when https://github.com/golang/go/issues/48618 is closed. +// type S interface{ ~[]int } + +var ( + a [10]byte + b [20]float32 + p List[int] + + _ = p[0:] + _ = p[1:10] + _ = p[2:] // want "unneeded: len\\(p\\)" + _ = p[3:(len(p))] + _ = p[len(a) : len(p)-1] + _ = p[0:len(b)] + _ = p[2:len(p):len(p)] + + _ = p[:] + _ = p[:10] + _ = p[:] // want "unneeded: len\\(p\\)" + _ = p[:(len(p))] + _ = p[:len(p)-1] + _ = p[:len(b)] + _ = p[:len(p):len(p)] +) + +func foo[E any](a List[E]) { + _ = a[0:] // want "unneeded: len\\(a\\)" +} diff --git a/internal/lsp/analysis/stubmethods/stubmethods.go b/internal/lsp/analysis/stubmethods/stubmethods.go new file mode 100644 index 000000000..c2a4138fa --- /dev/null +++ b/internal/lsp/analysis/stubmethods/stubmethods.go @@ -0,0 +1,351 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package stubmethods + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/token" + "go/types" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/internal/typesinternal" +) + +const Doc = `stub methods analyzer + +This analyzer generates method stubs for concrete types +in order to implement a target interface` + +var Analyzer = &analysis.Analyzer{ + Name: "stubmethods", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, err := range analysisinternal.GetTypeErrors(pass) { + ifaceErr := strings.Contains(err.Msg, "missing method") || strings.HasPrefix(err.Msg, "cannot convert") + if !ifaceErr { + continue + } + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= err.Pos && err.Pos < f.End() { + file = f + break + } + } + if file == nil { + continue + } + // Get the end position of the error. + _, _, endPos, ok := typesinternal.ReadGo116ErrorData(err) + if !ok { + var buf bytes.Buffer + if err := format.Node(&buf, pass.Fset, file); err != nil { + continue + } + endPos = analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos) + } + path, _ := astutil.PathEnclosingInterval(file, err.Pos, endPos) + si := GetStubInfo(pass.TypesInfo, path, err.Pos) + if si == nil { + continue + } + qf := RelativeToFiles(si.Concrete.Obj().Pkg(), file, nil, nil) + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: endPos, + Message: fmt.Sprintf("Implement %s", types.TypeString(si.Interface.Type(), qf)), + }) + } + return nil, nil +} + +// StubInfo represents a concrete type +// that wants to stub out an interface type +type StubInfo struct { + // Interface is the interface that the client wants to implement. + // When the interface is defined, the underlying object will be a TypeName. + // Note that we keep track of types.Object instead of types.Type in order + // to keep a reference to the declaring object's package and the ast file + // in the case where the concrete type file requires a new import that happens to be renamed + // in the interface file. + // TODO(marwan-at-work): implement interface literals. + Interface types.Object + Concrete *types.Named + Pointer bool +} + +// GetStubInfo determines whether the "missing method error" +// can be used to deduced what the concrete and interface types are. +func GetStubInfo(ti *types.Info, path []ast.Node, pos token.Pos) *StubInfo { + for _, n := range path { + switch n := n.(type) { + case *ast.ValueSpec: + return fromValueSpec(ti, n, pos) + case *ast.ReturnStmt: + // An error here may not indicate a real error the user should know about, but it may. + // Therefore, it would be best to log it out for debugging/reporting purposes instead of ignoring + // it. However, event.Log takes a context which is not passed via the analysis package. + // TODO(marwan-at-work): properly log this error. + si, _ := fromReturnStmt(ti, pos, path, n) + return si + case *ast.AssignStmt: + return fromAssignStmt(ti, n, pos) + } + } + return nil +} + +// fromReturnStmt analyzes a "return" statement to extract +// a concrete type that is trying to be returned as an interface type. +// +// For example, func() io.Writer { return myType{} } +// would return StubInfo with the interface being io.Writer and the concrete type being myType{}. +func fromReturnStmt(ti *types.Info, pos token.Pos, path []ast.Node, rs *ast.ReturnStmt) (*StubInfo, error) { + returnIdx := -1 + for i, r := range rs.Results { + if pos >= r.Pos() && pos <= r.End() { + returnIdx = i + } + } + if returnIdx == -1 { + return nil, fmt.Errorf("pos %d not within return statement bounds: [%d-%d]", pos, rs.Pos(), rs.End()) + } + concObj, pointer := concreteType(rs.Results[returnIdx], ti) + if concObj == nil || concObj.Obj().Pkg() == nil { + return nil, nil + } + ef := enclosingFunction(path, ti) + if ef == nil { + return nil, fmt.Errorf("could not find the enclosing function of the return statement") + } + iface := ifaceType(ef.Results.List[returnIdx].Type, ti) + if iface == nil { + return nil, nil + } + return &StubInfo{ + Concrete: concObj, + Pointer: pointer, + Interface: iface, + }, nil +} + +// fromValueSpec returns *StubInfo from a variable declaration such as +// var x io.Writer = &T{} +func fromValueSpec(ti *types.Info, vs *ast.ValueSpec, pos token.Pos) *StubInfo { + var idx int + for i, vs := range vs.Values { + if pos >= vs.Pos() && pos <= vs.End() { + idx = i + break + } + } + + valueNode := vs.Values[idx] + ifaceNode := vs.Type + callExp, ok := valueNode.(*ast.CallExpr) + // if the ValueSpec is `var _ = myInterface(...)` + // as opposed to `var _ myInterface = ...` + if ifaceNode == nil && ok && len(callExp.Args) == 1 { + ifaceNode = callExp.Fun + valueNode = callExp.Args[0] + } + concObj, pointer := concreteType(valueNode, ti) + if concObj == nil || concObj.Obj().Pkg() == nil { + return nil + } + ifaceObj := ifaceType(ifaceNode, ti) + if ifaceObj == nil { + return nil + } + return &StubInfo{ + Concrete: concObj, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// fromAssignStmt returns *StubInfo from a variable re-assignment such as +// var x io.Writer +// x = &T{} +func fromAssignStmt(ti *types.Info, as *ast.AssignStmt, pos token.Pos) *StubInfo { + idx := -1 + var lhs, rhs ast.Expr + // Given a re-assignment interface conversion error, + // the compiler error shows up on the right hand side of the expression. + // For example, x = &T{} where x is io.Writer highlights the error + // under "&T{}" and not "x". + for i, hs := range as.Rhs { + if pos >= hs.Pos() && pos <= hs.End() { + idx = i + break + } + } + if idx == -1 { + return nil + } + // Technically, this should never happen as + // we would get a "cannot assign N values to M variables" + // before we get an interface conversion error. Nonetheless, + // guard against out of range index errors. + if idx >= len(as.Lhs) { + return nil + } + lhs, rhs = as.Lhs[idx], as.Rhs[idx] + ifaceObj := ifaceType(lhs, ti) + if ifaceObj == nil { + return nil + } + concType, pointer := concreteType(rhs, ti) + if concType == nil || concType.Obj().Pkg() == nil { + return nil + } + return &StubInfo{ + Concrete: concType, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// RelativeToFiles returns a types.Qualifier that formats package names +// according to the files where the concrete and interface types are defined. +// +// This is similar to types.RelativeTo except if a file imports the package with a different name, +// then it will use it. And if the file does import the package but it is ignored, +// then it will return the original name. It also prefers package names in ifaceFile in case +// an import is missing from concFile but is present in ifaceFile. +// +// Additionally, if missingImport is not nil, the function will be called whenever the concFile +// is presented with a package that is not imported. This is useful so that as types.TypeString is +// formatting a function signature, it is identifying packages that will need to be imported when +// stubbing an interface. +func RelativeToFiles(concPkg *types.Package, concFile, ifaceFile *ast.File, missingImport func(name, path string)) types.Qualifier { + return func(other *types.Package) string { + if other == concPkg { + return "" + } + + // Check if the concrete file already has the given import, + // if so return the default package name or the renamed import statement. + for _, imp := range concFile.Imports { + impPath, _ := strconv.Unquote(imp.Path.Value) + isIgnored := imp.Name != nil && (imp.Name.Name == "." || imp.Name.Name == "_") + if impPath == other.Path() && !isIgnored { + importName := other.Name() + if imp.Name != nil { + importName = imp.Name.Name + } + return importName + } + } + + // If the concrete file does not have the import, check if the package + // is renamed in the interface file and prefer that. + var importName string + if ifaceFile != nil { + for _, imp := range ifaceFile.Imports { + impPath, _ := strconv.Unquote(imp.Path.Value) + isIgnored := imp.Name != nil && (imp.Name.Name == "." || imp.Name.Name == "_") + if impPath == other.Path() && !isIgnored { + if imp.Name != nil && imp.Name.Name != concPkg.Name() { + importName = imp.Name.Name + } + break + } + } + } + + if missingImport != nil { + missingImport(importName, other.Path()) + } + + // Up until this point, importName must stay empty when calling missingImport, + // otherwise we'd end up with `import time "time"` which doesn't look idiomatic. + if importName == "" { + importName = other.Name() + } + return importName + } +} + +// ifaceType will try to extract the types.Object that defines +// the interface given the ast.Expr where the "missing method" +// or "conversion" errors happen. +func ifaceType(n ast.Expr, ti *types.Info) types.Object { + tv, ok := ti.Types[n] + if !ok { + return nil + } + typ := tv.Type + named, ok := typ.(*types.Named) + if !ok { + return nil + } + _, ok = named.Underlying().(*types.Interface) + if !ok { + return nil + } + // Interfaces defined in the "builtin" package return nil a Pkg(). + // But they are still real interfaces that we need to make a special case for. + // Therefore, protect gopls from panicking if a new interface type was added in the future. + if named.Obj().Pkg() == nil && named.Obj().Name() != "error" { + return nil + } + return named.Obj() +} + +// concreteType tries to extract the *types.Named that defines +// the concrete type given the ast.Expr where the "missing method" +// or "conversion" errors happened. If the concrete type is something +// that cannot have methods defined on it (such as basic types), this +// method will return a nil *types.Named. The second return parameter +// is a boolean that indicates whether the concreteType was defined as a +// pointer or value. +func concreteType(n ast.Expr, ti *types.Info) (*types.Named, bool) { + tv, ok := ti.Types[n] + if !ok { + return nil, false + } + typ := tv.Type + ptr, isPtr := typ.(*types.Pointer) + if isPtr { + typ = ptr.Elem() + } + named, ok := typ.(*types.Named) + if !ok { + return nil, false + } + return named, isPtr +} + +// enclosingFunction returns the signature and type of the function +// enclosing the given position. +func enclosingFunction(path []ast.Node, info *types.Info) *ast.FuncType { + for _, node := range path { + switch t := node.(type) { + case *ast.FuncDecl: + if _, ok := info.Defs[t.Name]; ok { + return t.Type + } + case *ast.FuncLit: + if _, ok := info.Types[t]; ok { + return t.Type + } + } + } + return nil +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go new file mode 100644 index 000000000..ecf00ecfc --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go @@ -0,0 +1,13 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func channels(s string) { + undefinedChannels(c()) // want "undeclared name: undefinedChannels" +} + +func c() (<-chan string, chan string) { + return make(<-chan string), make(chan string) +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go new file mode 100644 index 000000000..ab7b2ba5c --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go @@ -0,0 +1,10 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func consecutiveParams() { + var s string + undefinedConsecutiveParams(s, s) // want "undeclared name: undefinedConsecutiveParams" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go new file mode 100644 index 000000000..341a9d2a4 --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go @@ -0,0 +1,10 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func errorParam() { + var err error + undefinedErrorParam(err) // want "undeclared name: undefinedErrorParam" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go new file mode 100644 index 000000000..ab82463d0 --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go @@ -0,0 +1,11 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +type T struct{} + +func literals() { + undefinedLiterals("hey compiler", T{}, &T{}) // want "undeclared name: undefinedLiterals" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go new file mode 100644 index 000000000..9a543821e --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go @@ -0,0 +1,11 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +import "time" + +func operation() { + undefinedOperation(10 * time.Second) // want "undeclared name: undefinedOperation" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go new file mode 100644 index 000000000..9ed09a27f --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go @@ -0,0 +1,10 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func selector() { + m := map[int]bool{} + undefinedSelector(m[1]) // want "undeclared name: undefinedSelector" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go new file mode 100644 index 000000000..d741c68f6 --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go @@ -0,0 +1,9 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func slice() { + undefinedSlice([]int{1, 2}) // want "undeclared name: undefinedSlice" +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go new file mode 100644 index 000000000..3148e8f4d --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go @@ -0,0 +1,13 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func tuple() { + undefinedTuple(b()) // want "undeclared name: undefinedTuple" +} + +func b() (string, error) { + return "", nil +} diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go b/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go new file mode 100644 index 000000000..98f77a43c --- /dev/null +++ b/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go @@ -0,0 +1,11 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package undeclared + +func uniqueArguments() { + var s string + var i int + undefinedUniqueArguments(s, i, s) // want "undeclared name: undefinedUniqueArguments" +} diff --git a/internal/lsp/analysis/undeclaredname/undeclared.go b/internal/lsp/analysis/undeclaredname/undeclared.go index df24d1d97..22b552c37 100644 --- a/internal/lsp/analysis/undeclaredname/undeclared.go +++ b/internal/lsp/analysis/undeclaredname/undeclared.go @@ -10,9 +10,11 @@ import ( "bytes" "fmt" "go/ast" + "go/format" "go/token" "go/types" "strings" + "unicode" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/ast/astutil" @@ -23,8 +25,17 @@ import ( const Doc = `suggested fixes for "undeclared name: <>" This checker provides suggested fixes for type errors of the -type "undeclared name: <>". It will insert a new statement: -"<> := ".` +type "undeclared name: <>". It will either insert a new statement, +such as: + +"<> := " + +or a new function declaration, such as: + +func <>(inferred parameters) { + panic("implement me!") +} +` var Analyzer = &analysis.Analyzer{ Name: string(analysisinternal.UndeclaredName), @@ -38,69 +49,94 @@ const undeclaredNamePrefix = "undeclared name: " func run(pass *analysis.Pass) (interface{}, error) { for _, err := range analysisinternal.GetTypeErrors(pass) { - if !FixesError(err.Msg) { - continue - } - name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix) - var file *ast.File - for _, f := range pass.Files { - if f.Pos() <= err.Pos && err.Pos < f.End() { - file = f - break - } - } - if file == nil { - continue - } + runForError(pass, err) + } + return nil, nil +} - // Get the path for the relevant range. - path, _ := astutil.PathEnclosingInterval(file, err.Pos, err.Pos) - if len(path) < 2 { - continue - } - ident, ok := path[0].(*ast.Ident) - if !ok || ident.Name != name { - continue - } - // Skip selector expressions because it might be too complex - // to try and provide a suggested fix for fields and methods. - if _, ok := path[1].(*ast.SelectorExpr); ok { - continue - } - // TODO(golang.org/issue/34644): Handle call expressions with suggested - // fixes to create a function. - if _, ok := path[1].(*ast.CallExpr); ok { - continue +func runForError(pass *analysis.Pass, err types.Error) { + if !strings.HasPrefix(err.Msg, undeclaredNamePrefix) { + return + } + name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix) + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= err.Pos && err.Pos < f.End() { + file = f + break } - tok := pass.Fset.File(file.Pos()) - if tok == nil { - continue + } + if file == nil { + return + } + + // Get the path for the relevant range. + path, _ := astutil.PathEnclosingInterval(file, err.Pos, err.Pos) + if len(path) < 2 { + return + } + ident, ok := path[0].(*ast.Ident) + if !ok || ident.Name != name { + return + } + + // Undeclared quick fixes only work in function bodies. + inFunc := false + for i := range path { + if _, inFunc = path[i].(*ast.FuncDecl); inFunc { + if i == 0 { + return + } + if _, isBody := path[i-1].(*ast.BlockStmt); !isBody { + return + } + break } - offset := pass.Fset.Position(err.Pos).Offset - end := tok.Pos(offset + len(name)) - pass.Report(analysis.Diagnostic{ - Pos: err.Pos, - End: end, - Message: err.Msg, - }) } - return nil, nil + if !inFunc { + return + } + // Skip selector expressions because it might be too complex + // to try and provide a suggested fix for fields and methods. + if _, ok := path[1].(*ast.SelectorExpr); ok { + return + } + tok := pass.Fset.File(file.Pos()) + if tok == nil { + return + } + offset := pass.Fset.Position(err.Pos).Offset + end := tok.Pos(offset + len(name)) + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: end, + Message: err.Msg, + }) } -func SuggestedFix(fset *token.FileSet, rng span.Range, content []byte, file *ast.File, _ *types.Package, _ *types.Info) (*analysis.SuggestedFix, error) { +func SuggestedFix(fset *token.FileSet, rng span.Range, content []byte, file *ast.File, pkg *types.Package, info *types.Info) (*analysis.SuggestedFix, error) { pos := rng.Start // don't use the end path, _ := astutil.PathEnclosingInterval(file, pos, pos) if len(path) < 2 { - return nil, fmt.Errorf("") + return nil, fmt.Errorf("no expression found") } ident, ok := path[0].(*ast.Ident) if !ok { - return nil, fmt.Errorf("") + return nil, fmt.Errorf("no identifier found") + } + + // Check for a possible call expression, in which case we should add a + // new function declaration. + if len(path) > 1 { + if _, ok := path[1].(*ast.CallExpr); ok { + return newFunctionDeclaration(path, file, pkg, info, fset) + } } + // Get the place to insert the new statement. insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(path) if insertBeforeStmt == nil { - return nil, fmt.Errorf("") + return nil, fmt.Errorf("could not locate insertion point") } insertBefore := fset.Position(insertBeforeStmt.Pos()).Offset @@ -111,6 +147,7 @@ func SuggestedFix(fset *token.FileSet, rng span.Range, content []byte, file *ast if nl := bytes.LastIndex(contentBeforeStmt, []byte("\n")); nl != -1 { indent = string(contentBeforeStmt[nl:]) } + // Create the new local variable statement. newStmt := fmt.Sprintf("%s := %s", ident.Name, indent) return &analysis.SuggestedFix{ @@ -123,6 +160,181 @@ func SuggestedFix(fset *token.FileSet, rng span.Range, content []byte, file *ast }, nil } -func FixesError(msg string) bool { - return strings.HasPrefix(msg, undeclaredNamePrefix) +func newFunctionDeclaration(path []ast.Node, file *ast.File, pkg *types.Package, info *types.Info, fset *token.FileSet) (*analysis.SuggestedFix, error) { + if len(path) < 3 { + return nil, fmt.Errorf("unexpected set of enclosing nodes: %v", path) + } + ident, ok := path[0].(*ast.Ident) + if !ok { + return nil, fmt.Errorf("no name for function declaration %v (%T)", path[0], path[0]) + } + call, ok := path[1].(*ast.CallExpr) + if !ok { + return nil, fmt.Errorf("no call expression found %v (%T)", path[1], path[1]) + } + + // Find the enclosing function, so that we can add the new declaration + // below. + var enclosing *ast.FuncDecl + for _, n := range path { + if n, ok := n.(*ast.FuncDecl); ok { + enclosing = n + break + } + } + // TODO(rstambler): Support the situation when there is no enclosing + // function. + if enclosing == nil { + return nil, fmt.Errorf("no enclosing function found: %v", path) + } + + pos := enclosing.End() + + var paramNames []string + var paramTypes []types.Type + // keep track of all param names to later ensure uniqueness + nameCounts := map[string]int{} + for _, arg := range call.Args { + typ := info.TypeOf(arg) + if typ == nil { + return nil, fmt.Errorf("unable to determine type for %s", arg) + } + + switch t := typ.(type) { + // this is the case where another function call returning multiple + // results is used as an argument + case *types.Tuple: + n := t.Len() + for i := 0; i < n; i++ { + name := typeToArgName(t.At(i).Type()) + nameCounts[name]++ + + paramNames = append(paramNames, name) + paramTypes = append(paramTypes, types.Default(t.At(i).Type())) + } + + default: + // does the argument have a name we can reuse? + // only happens in case of a *ast.Ident + var name string + if ident, ok := arg.(*ast.Ident); ok { + name = ident.Name + } + + if name == "" { + name = typeToArgName(typ) + } + + nameCounts[name]++ + + paramNames = append(paramNames, name) + paramTypes = append(paramTypes, types.Default(typ)) + } + } + + for n, c := range nameCounts { + // Any names we saw more than once will need a unique suffix added + // on. Reset the count to 1 to act as the suffix for the first + // occurrence of that name. + if c >= 2 { + nameCounts[n] = 1 + } else { + delete(nameCounts, n) + } + } + + params := &ast.FieldList{} + + for i, name := range paramNames { + if suffix, repeats := nameCounts[name]; repeats { + nameCounts[name]++ + name = fmt.Sprintf("%s%d", name, suffix) + } + + // only worth checking after previous param in the list + if i > 0 { + // if type of parameter at hand is the same as the previous one, + // add it to the previous param list of identifiers so to have: + // (s1, s2 string) + // and not + // (s1 string, s2 string) + if paramTypes[i] == paramTypes[i-1] { + params.List[len(params.List)-1].Names = append(params.List[len(params.List)-1].Names, ast.NewIdent(name)) + continue + } + } + + params.List = append(params.List, &ast.Field{ + Names: []*ast.Ident{ + ast.NewIdent(name), + }, + Type: analysisinternal.TypeExpr(fset, file, pkg, paramTypes[i]), + }) + } + + decl := &ast.FuncDecl{ + Name: ast.NewIdent(ident.Name), + Type: &ast.FuncType{ + Params: params, + // TODO(rstambler): Also handle result parameters here. + }, + Body: &ast.BlockStmt{ + List: []ast.Stmt{ + &ast.ExprStmt{ + X: &ast.CallExpr{ + Fun: ast.NewIdent("panic"), + Args: []ast.Expr{ + &ast.BasicLit{ + Value: `"unimplemented"`, + }, + }, + }, + }, + }, + }, + } + + b := bytes.NewBufferString("\n\n") + if err := format.Node(b, fset, decl); err != nil { + return nil, err + } + return &analysis.SuggestedFix{ + Message: fmt.Sprintf("Create function \"%s\"", ident.Name), + TextEdits: []analysis.TextEdit{{ + Pos: pos, + End: pos, + NewText: b.Bytes(), + }}, + }, nil +} +func typeToArgName(ty types.Type) string { + s := types.Default(ty).String() + + switch t := ty.(type) { + case *types.Basic: + // use first letter in type name for basic types + return s[0:1] + case *types.Slice: + // use element type to decide var name for slices + return typeToArgName(t.Elem()) + case *types.Array: + // use element type to decide var name for arrays + return typeToArgName(t.Elem()) + case *types.Chan: + return "ch" + } + + s = strings.TrimFunc(s, func(r rune) bool { + return !unicode.IsLetter(r) + }) + + if s == "error" { + return "err" + } + + // remove package (if present) + // and make first letter lowercase + a := []rune(s[strings.LastIndexByte(s, '.')+1:]) + a[0] = unicode.ToLower(a[0]) + return string(a) } diff --git a/internal/lsp/analysis/unusedparams/testdata/src/a/a.go b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go index 248ecfc0e..23e4122c4 100644 --- a/internal/lsp/analysis/unusedparams/testdata/src/a/a.go +++ b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Go Authors. All rights reserved. +// Copyright 2022 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. diff --git a/internal/lsp/analysis/unusedparams/testdata/src/a/a.go.golden b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go.golden new file mode 100644 index 000000000..e28a6bdea --- /dev/null +++ b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go.golden @@ -0,0 +1,55 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import ( + "bytes" + "fmt" + "net/http" +) + +type parent interface { + n(f bool) +} + +type yuh struct { + a int +} + +func (y *yuh) n(f bool) { + for i := 0; i < 10; i++ { + fmt.Println(i) + } +} + +func a(i1 int, _ int, i3 int) int { // want "potentially unused parameter: 'i2'" + i3 += i1 + _ = func(_ int) int { // want "potentially unused parameter: 'z'" + _ = 1 + return 1 + } + return i3 +} + +func b(_ bytes.Buffer) { // want "potentially unused parameter: 'c'" + _ = 1 +} + +func z(_ http.ResponseWriter, _ *http.Request) { // want "potentially unused parameter: 'h'" + fmt.Println("Before") +} + +func l(h http.Handler) http.Handler { + return http.HandlerFunc(z) +} + +func mult(a, _ int) int { // want "potentially unused parameter: 'b'" + a += 1 + return a +} + +func y(a int) { + panic("yo") +} diff --git a/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go b/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..93af2681b --- /dev/null +++ b/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go @@ -0,0 +1,55 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package typeparams + +import ( + "bytes" + "fmt" + "net/http" +) + +type parent[T any] interface { + n(f T) +} + +type yuh[T any] struct { + a T +} + +func (y *yuh[int]) n(f bool) { + for i := 0; i < 10; i++ { + fmt.Println(i) + } +} + +func a[T comparable](i1 int, i2 T, i3 int) int { // want "potentially unused parameter: 'i2'" + i3 += i1 + _ = func(z int) int { // want "potentially unused parameter: 'z'" + _ = 1 + return 1 + } + return i3 +} + +func b[T any](c bytes.Buffer) { // want "potentially unused parameter: 'c'" + _ = 1 +} + +func z[T http.ResponseWriter](h T, _ *http.Request) { // want "potentially unused parameter: 'h'" + fmt.Println("Before") +} + +func l(h http.Handler) http.Handler { + return http.HandlerFunc(z[http.ResponseWriter]) +} + +func mult(a, b int) int { // want "potentially unused parameter: 'b'" + a += 1 + return a +} + +func y[T any](a T) { + panic("yo") +} diff --git a/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go.golden b/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go.golden new file mode 100644 index 000000000..c86bf289a --- /dev/null +++ b/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go.golden @@ -0,0 +1,55 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package typeparams + +import ( + "bytes" + "fmt" + "net/http" +) + +type parent[T any] interface { + n(f T) +} + +type yuh[T any] struct { + a T +} + +func (y *yuh[int]) n(f bool) { + for i := 0; i < 10; i++ { + fmt.Println(i) + } +} + +func a[T comparable](i1 int, _ T, i3 int) int { // want "potentially unused parameter: 'i2'" + i3 += i1 + _ = func(_ int) int { // want "potentially unused parameter: 'z'" + _ = 1 + return 1 + } + return i3 +} + +func b[T any](_ bytes.Buffer) { // want "potentially unused parameter: 'c'" + _ = 1 +} + +func z[T http.ResponseWriter](_ T, _ *http.Request) { // want "potentially unused parameter: 'h'" + fmt.Println("Before") +} + +func l(h http.Handler) http.Handler { + return http.HandlerFunc(z[http.ResponseWriter]) +} + +func mult(a, _ int) int { // want "potentially unused parameter: 'b'" + a += 1 + return a +} + +func y[T any](a T) { + panic("yo") +} diff --git a/internal/lsp/analysis/unusedparams/unusedparams.go b/internal/lsp/analysis/unusedparams/unusedparams.go index e6f22746d..4c933c8fb 100644 --- a/internal/lsp/analysis/unusedparams/unusedparams.go +++ b/internal/lsp/analysis/unusedparams/unusedparams.go @@ -131,13 +131,20 @@ func run(pass *analysis.Pass) (interface{}, error) { start, end = u.ident.Pos(), u.ident.End() } // TODO(golang/go#36602): Add suggested fixes to automatically - // remove the unused parameter. To start, just remove it from the - // function declaration. Later, remove it from every use of this + // remove the unused parameter from every use of this // function. pass.Report(analysis.Diagnostic{ Pos: start, End: end, Message: fmt.Sprintf("potentially unused parameter: '%s'", u.ident.Name), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: `Replace with "_"`, + TextEdits: []analysis.TextEdit{{ + Pos: u.ident.Pos(), + End: u.ident.End(), + NewText: []byte("_"), + }}, + }}, }) } }) diff --git a/internal/lsp/analysis/unusedparams/unusedparams_test.go b/internal/lsp/analysis/unusedparams/unusedparams_test.go index 907f71c8d..dff17c95e 100644 --- a/internal/lsp/analysis/unusedparams/unusedparams_test.go +++ b/internal/lsp/analysis/unusedparams/unusedparams_test.go @@ -9,9 +9,14 @@ import ( "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/internal/lsp/analysis/unusedparams" + "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, unusedparams.Analyzer, "a") + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, unusedparams.Analyzer, tests...) } diff --git a/internal/lsp/analysis/useany/testdata/src/a/a.go b/internal/lsp/analysis/useany/testdata/src/a/a.go new file mode 100644 index 000000000..22d693150 --- /dev/null +++ b/internal/lsp/analysis/useany/testdata/src/a/a.go @@ -0,0 +1,25 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the useany checker. + +package a + +type Any interface{} + +func _[T interface{}]() {} // want "could use \"any\" for this empty interface" +func _[X any, T interface{}]() {} // want "could use \"any\" for this empty interface" +func _[any interface{}]() {} // want "could use \"any\" for this empty interface" +func _[T Any]() {} // want "could use \"any\" for this empty interface" +func _[T interface{ int | interface{} }]() {} // want "could use \"any\" for this empty interface" +func _[T interface{ int | Any }]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} + +type _[T interface{}] int // want "could use \"any\" for this empty interface" +type _[X any, T interface{}] int // want "could use \"any\" for this empty interface" +type _[any interface{}] int // want "could use \"any\" for this empty interface" +type _[T Any] int // want "could use \"any\" for this empty interface" +type _[T interface{ int | interface{} }] int // want "could use \"any\" for this empty interface" +type _[T interface{ int | Any }] int // want "could use \"any\" for this empty interface" +type _[T any] int diff --git a/internal/lsp/analysis/useany/testdata/src/a/a.go.golden b/internal/lsp/analysis/useany/testdata/src/a/a.go.golden new file mode 100644 index 000000000..efd8fd640 --- /dev/null +++ b/internal/lsp/analysis/useany/testdata/src/a/a.go.golden @@ -0,0 +1,25 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the useany checker. + +package a + +type Any interface{} + +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[X any, T any]() {} // want "could use \"any\" for this empty interface" +func _[any interface{}]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} + +type _[T any] int // want "could use \"any\" for this empty interface" +type _[X any, T any] int // want "could use \"any\" for this empty interface" +type _[any interface{}] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int diff --git a/internal/lsp/analysis/useany/useany.go b/internal/lsp/analysis/useany/useany.go new file mode 100644 index 000000000..73e2f7633 --- /dev/null +++ b/internal/lsp/analysis/useany/useany.go @@ -0,0 +1,102 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package useany defines an Analyzer that checks for usage of interface{} in +// constraints, rather than the predeclared any. +package useany + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/internal/typeparams" +) + +const Doc = `check for constraints that could be simplified to "any"` + +var Analyzer = &analysis.Analyzer{ + Name: "useany", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + universeAny := types.Universe.Lookup("any") + if universeAny == nil { + // Go <= 1.17. Nothing to check. + return nil, nil + } + + nodeFilter := []ast.Node{ + (*ast.TypeSpec)(nil), + (*ast.FuncType)(nil), + } + + inspect.Preorder(nodeFilter, func(node ast.Node) { + var tparams *ast.FieldList + switch node := node.(type) { + case *ast.TypeSpec: + tparams = typeparams.ForTypeSpec(node) + case *ast.FuncType: + tparams = typeparams.ForFuncType(node) + default: + panic(fmt.Sprintf("unexpected node type %T", node)) + } + if tparams.NumFields() == 0 { + return + } + + for _, field := range tparams.List { + typ := pass.TypesInfo.Types[field.Type].Type + if typ == nil { + continue // something is wrong, but not our concern + } + iface, ok := typ.Underlying().(*types.Interface) + if !ok { + continue // invalid constraint + } + + // If the constraint is the empty interface, offer a fix to use 'any' + // instead. + if iface.Empty() { + id, _ := field.Type.(*ast.Ident) + if id != nil && pass.TypesInfo.Uses[id] == universeAny { + continue + } + + diag := analysis.Diagnostic{ + Pos: field.Type.Pos(), + End: field.Type.End(), + Message: `could use "any" for this empty interface`, + } + + // Only suggest a fix to 'any' if we actually resolve the predeclared + // any in this scope. + if scope := pass.TypesInfo.Scopes[node]; scope != nil { + if _, any := scope.LookupParent("any", token.NoPos); any == universeAny { + diag.SuggestedFixes = []analysis.SuggestedFix{{ + Message: `use "any"`, + TextEdits: []analysis.TextEdit{{ + Pos: field.Type.Pos(), + End: field.Type.End(), + NewText: []byte("any"), + }}, + }} + } + } + + pass.Report(diag) + } + } + }) + return nil, nil +} diff --git a/internal/lsp/analysis/useany/useany_test.go b/internal/lsp/analysis/useany/useany_test.go new file mode 100644 index 000000000..535d91526 --- /dev/null +++ b/internal/lsp/analysis/useany/useany_test.go @@ -0,0 +1,21 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package useany_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/useany" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + if !typeparams.Enabled { + t.Skip("type params are not enabled") + } + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, useany.Analyzer, "a") +} |