diff options
Diffstat (limited to 'gopls/internal/lsp/analysis')
82 files changed, 5167 insertions, 0 deletions
diff --git a/gopls/internal/lsp/analysis/embeddirective/embeddirective.go b/gopls/internal/lsp/analysis/embeddirective/embeddirective.go new file mode 100644 index 000000000..1b504f7cb --- /dev/null +++ b/gopls/internal/lsp/analysis/embeddirective/embeddirective.go @@ -0,0 +1,58 @@ +// 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 embeddirective defines an Analyzer that validates import for //go:embed directive. +package embeddirective + +import ( + "go/ast" + "strings" + + "golang.org/x/tools/go/analysis" +) + +const Doc = `check for //go:embed directive import + +This analyzer checks that the embed package is imported when source code contains //go:embed comment directives. +The embed package must be imported for //go:embed directives to function.import _ "embed".` + +var Analyzer = &analysis.Analyzer{ + Name: "embed", + Doc: Doc, + Requires: []*analysis.Analyzer{}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, f := range pass.Files { + com := hasEmbedDirectiveComment(f) + if com != nil { + assertEmbedImport(pass, com, f) + } + } + return nil, nil +} + +// Check if the comment contains //go:embed directive. +func hasEmbedDirectiveComment(f *ast.File) *ast.Comment { + for _, cg := range f.Comments { + for _, c := range cg.List { + if strings.HasPrefix(c.Text, "//go:embed ") { + return c + } + } + } + return nil +} + +// Verifies that "embed" import exists for //go:embed directive. +func assertEmbedImport(pass *analysis.Pass, com *ast.Comment, f *ast.File) { + for _, imp := range f.Imports { + if "\"embed\"" == imp.Path.Value { + return + } + } + pass.Report(analysis.Diagnostic{Pos: com.Pos(), End: com.Pos() + 10, Message: "The \"embed\" package must be imported when using go:embed directives."}) +} diff --git a/gopls/internal/lsp/analysis/embeddirective/embeddirective_test.go b/gopls/internal/lsp/analysis/embeddirective/embeddirective_test.go new file mode 100644 index 000000000..1165c0bf6 --- /dev/null +++ b/gopls/internal/lsp/analysis/embeddirective/embeddirective_test.go @@ -0,0 +1,22 @@ +// 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 embeddirective + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests) + } + + analysistest.RunWithSuggestedFixes(t, testdata, Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/a.go b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/a.go new file mode 100644 index 000000000..4203f6ce2 --- /dev/null +++ b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/a.go @@ -0,0 +1,13 @@ +package a + +import ( + "fmt" +) + +//go:embed embedText // want "The \"embed\" package must be imported when using go:embed directives" +var s string + +// This is main function +func main() { + fmt.Println(s) +} diff --git a/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/b.go b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/b.go new file mode 100644 index 000000000..c8c701e66 --- /dev/null +++ b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/b.go @@ -0,0 +1,14 @@ +package a + +import ( + _ "embed" + "fmt" +) + +//go:embed embedText // ok +var s string + +// This is main function +func main() { + fmt.Println(s) +} diff --git a/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/embedText b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/embedText new file mode 100644 index 000000000..5e1c309da --- /dev/null +++ b/gopls/internal/lsp/analysis/embeddirective/testdata/src/a/embedText @@ -0,0 +1 @@ +Hello World
\ No newline at end of file diff --git a/gopls/internal/lsp/analysis/fillreturns/fillreturns.go b/gopls/internal/lsp/analysis/fillreturns/fillreturns.go new file mode 100644 index 000000000..c8146df2d --- /dev/null +++ b/gopls/internal/lsp/analysis/fillreturns/fillreturns.go @@ -0,0 +1,279 @@ +// 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 fillreturns defines an Analyzer that will attempt to +// automatically fill in a return statement that has missing +// values with zero value elements. +package fillreturns + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/types" + "regexp" + "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/fuzzy" + "golang.org/x/tools/internal/typeparams" +) + +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: + func m() (int, string, *bool, error) { + return + } +will turn into + func m() (int, string, *bool, error) { + return 0, "", nil, nil + } + +This functionality is similar to https://github.com/sqs/goreturns. +` + +var Analyzer = &analysis.Analyzer{ + Name: "fillreturns", + Doc: Doc, + Requires: []*analysis.Analyzer{}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + info := pass.TypesInfo + if info == nil { + return nil, fmt.Errorf("nil TypeInfo") + } + +outer: + for _, typeErr := range pass.TypeErrors { + // Filter out the errors that are not relevant to this analyzer. + if !FixesError(typeErr) { + continue + } + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= typeErr.Pos && typeErr.Pos <= f.End() { + file = f + break + } + } + if file == nil { + continue + } + + // Get the end position of the error. + // (This heuristic assumes that the buffer is formatted, + // at least up to the end position of the error.) + var buf bytes.Buffer + if err := format.Node(&buf, pass.Fset, file); err != nil { + continue + } + 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 + } + + // 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[retIdx+1:] { + switch node := n.(type) { + case *ast.FuncLit: + enclosingFunc = node.Type + case *ast.FuncDecl: + enclosingFunc = node.Type + } + if enclosingFunc != nil { + break + } + } + if enclosingFunc == nil || enclosingFunc.Results == nil { + 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 { + if p, ok := p.(*ast.FuncDecl); ok { + outer = p + break + } + } + if outer == nil { + return nil, nil + } + + // Skip any return statements that contain function calls with multiple + // return values. + for _, expr := range ret.Results { + e, ok := expr.(*ast.CallExpr) + if !ok { + continue + } + if tup, ok := info.TypeOf(e).(*types.Tuple); ok && tup.Len() > 1 { + continue outer + } + } + + // Duplicate the return values to track which values have been matched. + remaining := make([]ast.Expr, len(ret.Results)) + copy(remaining, ret.Results) + + fixed := make([]ast.Expr, len(enclosingFunc.Results.List)) + + // For each value in the return function declaration, find the leftmost element + // in the return statement that has the desired type. If no such element exists, + // fill in the missing value with the appropriate "zero" value. + // Beware that type information may be incomplete. + var retTyps []types.Type + for _, ret := range enclosingFunc.Results.List { + retTyp := info.TypeOf(ret.Type) + if retTyp == nil { + return nil, nil + } + retTyps = append(retTyps, retTyp) + } + matches := analysisinternal.MatchingIdents(retTyps, file, ret.Pos(), info, pass.Pkg) + for i, retTyp := range retTyps { + var match ast.Expr + var idx int + for j, val := range remaining { + if t := info.TypeOf(val); t == nil || !matchingTypes(t, retTyp) { + continue + } + if !analysisinternal.IsZeroValue(val) { + match, idx = val, j + break + } + // If the current match is a "zero" value, we keep searching in + // case we find a non-"zero" value match. If we do not find a + // non-"zero" value, we will use the "zero" value. + match, idx = val, j + } + + if match != nil { + fixed[i] = match + remaining = append(remaining[:idx], remaining[idx+1:]...) + } else { + names, ok := matches[retTyp] + if !ok { + return nil, fmt.Errorf("invalid return type: %v", retTyp) + } + // Find the identifier most similar to the return type. + // If no identifier matches the pattern, generate a zero value. + if best := fuzzy.BestMatch(retTyp.String(), names); best != "" { + fixed[i] = ast.NewIdent(best) + } else if zero := analysisinternal.ZeroValue(file, pass.Pkg, retTyp); zero != nil { + fixed[i] = zero + } else { + return nil, nil + } + } + } + + // Remove any non-matching "zero values" from the leftover values. + var nonZeroRemaining []ast.Expr + for _, expr := range remaining { + if !analysisinternal.IsZeroValue(expr) { + nonZeroRemaining = append(nonZeroRemaining, expr) + } + } + // Append leftover return values to end of new return statement. + fixed = append(fixed, nonZeroRemaining...) + + newRet := &ast.ReturnStmt{ + Return: ret.Pos(), + Results: fixed, + } + + // Convert the new return statement AST to text. + var newBuf bytes.Buffer + if err := format.Node(&newBuf, pass.Fset, newRet); err != nil { + return nil, err + } + + pass.Report(analysis.Diagnostic{ + Pos: typeErr.Pos, + End: typeErrEndPos, + Message: typeErr.Msg, + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Fill in return values", + TextEdits: []analysis.TextEdit{{ + Pos: ret.Pos(), + End: ret.End(), + NewText: newBuf.Bytes(), + }}, + }}, + }) + } + return nil, nil +} + +func matchingTypes(want, got types.Type) bool { + if want == got || types.Identical(want, got) { + return true + } + // Code segment to help check for untyped equality from (golang/go#32146). + if rhs, ok := want.(*types.Basic); ok && rhs.Info()&types.IsUntyped > 0 { + if lhs, ok := got.Underlying().(*types.Basic); ok { + return rhs.Info()&types.IsConstType == lhs.Info()&types.IsConstType + } + } + return types.AssignableTo(want, got) || types.ConvertibleTo(want, got) +} + +// 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/gopls/internal/lsp/analysis/fillreturns/fillreturns_test.go b/gopls/internal/lsp/analysis/fillreturns/fillreturns_test.go new file mode 100644 index 000000000..1f7627551 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillreturns/fillreturns_test.go @@ -0,0 +1,22 @@ +// 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 fillreturns_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/fillreturns" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, fillreturns.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go new file mode 100644 index 000000000..7ab0ff167 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go @@ -0,0 +1,139 @@ +// 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 fillreturns + +import ( + "errors" + "go/ast" + ast2 "go/ast" + "io" + "net/http" + . "net/http" + "net/url" + "strconv" +) + +type T struct{} +type T1 = T +type I interface{} +type I1 = I +type z func(string, http.Handler) error + +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 "return values" +} + +func c() (string, int, error) { + return 7, errors.New("foo") // want "return values" +} + +func d() (string, int, error) { + return "", 7 // want "return values" +} + +func e() (T, error, *bool) { + return (z(http.ListenAndServe))("", nil) // want "return values" +} + +func preserveLeft() (int, int, error) { + return 1, errors.New("foo") // want "return values" +} + +func matchValues() (int, error, string) { + return errors.New("foo"), 3 // want "return values" +} + +func preventDataOverwrite() (int, string) { + return errors.New("foo") // want "return values" +} + +func closure() (string, error) { + _ = func() (int, error) { + return // want "return values" + } + 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 "return values" +} + +func complex() (*int, []int, [2]int, map[int]int) { + return // want "return values" +} + +func structsAndInterfaces() (T, url.URL, T1, I, I1, io.Reader, Client, ast2.Stmt) { + return // want "return values" +} + +func m() (int, error) { + if 1 == 2 { + return // want "return values" + } else if 1 == 3 { + return errors.New("foo") // want "return values" + } else { + return 1 // want "return values" + } + return // want "return values" +} + +func convertibleTypes() (ast2.Expr, int) { + return &ast2.ArrayType{} // want "return values" +} + +func assignableTypes() (map[string]int, int) { + type X map[string]int + var x X + return x // want "return values" +} + +func interfaceAndError() (I, int) { + return errors.New("foo") // want "return values" +} + +func funcOneReturn() (string, error) { + return strconv.Itoa(1) // want "return values" +} + +func funcMultipleReturn() (int, error, string) { + return strconv.Atoi("1") +} + +func localFuncMultipleReturn() (string, int, error, string) { + return b() +} + +func multipleUnused() (int, string, string, string) { + return 3, 4, 5 // want "return values" +} + +func gotTooMany() int { + if true { + return 0, "" // want "return values" + } else { + return 1, 0, nil // want "return values" + } + return 0, 5, false // want "return values" +} + +func fillVars() (int, string, ast.Node, bool, error) { + eint := 0 + s := "a" + var t bool + if true { + err := errors.New("fail") + return // want "return values" + } + n := ast.NewIdent("ident") + int := 3 + var b bool + return "" // want "return values" +} diff --git a/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden new file mode 100644 index 000000000..f007a5f37 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/a.go.golden @@ -0,0 +1,139 @@ +// 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 fillreturns + +import ( + "errors" + "go/ast" + ast2 "go/ast" + "io" + "net/http" + . "net/http" + "net/url" + "strconv" +) + +type T struct{} +type T1 = T +type I interface{} +type I1 = I +type z func(string, http.Handler) error + +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 "return values" +} + +func c() (string, int, error) { + return "", 7, errors.New("foo") // want "return values" +} + +func d() (string, int, error) { + return "", 7, nil // want "return values" +} + +func e() (T, error, *bool) { + return T{}, (z(http.ListenAndServe))("", nil), nil // want "return values" +} + +func preserveLeft() (int, int, error) { + return 1, 0, errors.New("foo") // want "return values" +} + +func matchValues() (int, error, string) { + return 3, errors.New("foo"), "" // want "return values" +} + +func preventDataOverwrite() (int, string) { + return 0, "", errors.New("foo") // want "return values" +} + +func closure() (string, error) { + _ = func() (int, error) { + return 0, nil // want "return values" + } + 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 "return values" +} + +func complex() (*int, []int, [2]int, map[int]int) { + 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 "return values" +} + +func m() (int, error) { + if 1 == 2 { + return 0, nil // want "return values" + } else if 1 == 3 { + return 0, errors.New("foo") // want "return values" + } else { + return 1, nil // want "return values" + } + return 0, nil // want "return values" +} + +func convertibleTypes() (ast2.Expr, int) { + 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 "return values" +} + +func interfaceAndError() (I, int) { + return errors.New("foo"), 0 // want "return values" +} + +func funcOneReturn() (string, error) { + return strconv.Itoa(1), nil // want "return values" +} + +func funcMultipleReturn() (int, error, string) { + return strconv.Atoi("1") +} + +func localFuncMultipleReturn() (string, int, error, string) { + return b() +} + +func multipleUnused() (int, string, string, string) { + return 3, "", "", "", 4, 5 // want "return values" +} + +func gotTooMany() int { + if true { + return 0 // want "return values" + } else { + return 1 // want "return values" + } + return 5 // want "return values" +} + +func fillVars() (int, string, ast.Node, bool, error) { + eint := 0 + s := "a" + var t bool + if true { + err := errors.New("fail") + return eint, s, nil, false, err // want "return values" + } + n := ast.NewIdent("ident") + int := 3 + var b bool + return int, "", n, b, nil // want "return values" +} diff --git a/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go new file mode 100644 index 000000000..8454bd2ce --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden b/gopls/internal/lsp/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden new file mode 100644 index 000000000..8454bd2ce --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/fillstruct/fillstruct.go b/gopls/internal/lsp/analysis/fillstruct/fillstruct.go new file mode 100644 index 000000000..af29a3632 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillstruct/fillstruct.go @@ -0,0 +1,506 @@ +// 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 defines an Analyzer that automatically +// fills in a struct declaration with zero value elements for each field. +// +// The analyzer's diagnostic is merely a prompt. +// The actual fix is created by a separate direct call from gopls to +// the SuggestedFixes function. +// Tests of Analyzer.Run can be found in ./testdata/src. +// Tests of the SuggestedFixes logic live in ../../testdata/fillstruct. +package fillstruct + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/token" + "go/types" + "strings" + "unicode" + + "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/go/ast/inspector" + "golang.org/x/tools/gopls/internal/lsp/safetoken" + "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/internal/fuzzy" + "golang.org/x/tools/internal/typeparams" +) + +const Doc = `note incomplete struct initializations + +This analyzer provides diagnostics for any struct literals that do not have +any fields initialized. Because the suggested fix for this analysis is +expensive to compute, callers should compute it separately, using the +SuggestedFix function below. +` + +var Analyzer = &analysis.Analyzer{ + Name: "fillstruct", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{(*ast.CompositeLit)(nil)} + inspect.Preorder(nodeFilter, func(n ast.Node) { + expr := n.(*ast.CompositeLit) + + // Find enclosing file. + // TODO(adonovan): use inspect.WithStack? + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= expr.Pos() && expr.Pos() <= f.End() { + file = f + break + } + } + if file == nil { + return + } + + typ := pass.TypesInfo.TypeOf(expr) + if typ == nil { + return + } + + // Find reference to the type declaration of the struct being initialized. + typ = deref(typ) + tStruct, ok := typ.Underlying().(*types.Struct) + if !ok { + return + } + // Inv: typ is the possibly-named struct type. + + fieldCount := tStruct.NumFields() + + // Skip any struct that is already populated or that has no fields. + if fieldCount == 0 || fieldCount == len(expr.Elts) { + return + } + + // Are any fields in need of filling? + var fillableFields []string + for i := 0; i < fieldCount; i++ { + field := tStruct.Field(i) + // Ignore fields that are not accessible in the current package. + if field.Pkg() != nil && field.Pkg() != pass.Pkg && !field.Exported() { + continue + } + fillableFields = append(fillableFields, fmt.Sprintf("%s: %s", field.Name(), field.Type().String())) + } + if len(fillableFields) == 0 { + return + } + + // Derive a name for the struct type. + var name string + if typ != tStruct { + // named struct type (e.g. pkg.S[T]) + name = types.TypeString(typ, types.RelativeTo(pass.Pkg)) + } else { + // anonymous struct type + totalFields := len(fillableFields) + const 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), + Pos: expr.Pos(), + End: expr.End(), + }) + }) + return nil, nil +} + +// SuggestedFix computes the suggested fix for the kinds of +// diagnostics produced by the Analyzer above. +func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, file *ast.File, pkg *types.Package, info *types.Info) (*analysis.SuggestedFix, error) { + if info == nil { + return nil, fmt.Errorf("nil types.Info") + } + + pos := start // don't use the end + + // TODO(rstambler): Using ast.Inspect would probably be more efficient than + // calling PathEnclosingInterval. Switch this approach. + path, _ := astutil.PathEnclosingInterval(file, pos, pos) + if len(path) == 0 { + return nil, fmt.Errorf("no enclosing ast.Node") + } + var expr *ast.CompositeLit + for _, n := range path { + if node, ok := n.(*ast.CompositeLit); ok { + expr = node + break + } + } + + typ := info.TypeOf(expr) + if typ == nil { + return nil, fmt.Errorf("no composite literal") + } + + // Find reference to the type declaration of the struct being initialized. + typ = deref(typ) + tStruct, ok := typ.Underlying().(*types.Struct) + if !ok { + return nil, fmt.Errorf("%s is not a (pointer to) struct type", + types.TypeString(typ, types.RelativeTo(pkg))) + } + // Inv: typ is the the possibly-named struct type. + + fieldCount := tStruct.NumFields() + + // Check which types have already been filled in. (we only want to fill in + // the unfilled types, or else we'll blat user-supplied details) + prefilledFields := map[string]ast.Expr{} + for _, e := range expr.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + if key, ok := kv.Key.(*ast.Ident); ok { + prefilledFields[key.Name] = kv.Value + } + } + } + + // Use a new fileset to build up a token.File for the new composite + // literal. We need one line for foo{, one line for }, and one line for + // each field we're going to set. format.Node only cares about line + // numbers, so we don't need to set columns, and each line can be + // 1 byte long. + // TODO(adonovan): why is this necessary? The position information + // is going to be wrong for the existing trees in prefilledFields. + // Can't the formatter just do its best with an empty fileset? + fakeFset := token.NewFileSet() + tok := fakeFset.AddFile("", -1, fieldCount+2) + + line := 2 // account for 1-based lines and the left brace + var fieldTyps []types.Type + for i := 0; i < fieldCount; i++ { + field := tStruct.Field(i) + // Ignore fields that are not accessible in the current package. + if field.Pkg() != nil && field.Pkg() != pkg && !field.Exported() { + fieldTyps = append(fieldTyps, nil) + continue + } + fieldTyps = append(fieldTyps, field.Type()) + } + matches := analysisinternal.MatchingIdents(fieldTyps, file, start, info, pkg) + var elts []ast.Expr + for i, fieldTyp := range fieldTyps { + if fieldTyp == nil { + continue // TODO(adonovan): is this reachable? + } + fieldName := tStruct.Field(i).Name() + + tok.AddLine(line - 1) // add 1 byte per line + if line > tok.LineCount() { + panic(fmt.Sprintf("invalid line number %v (of %v) for fillstruct", line, tok.LineCount())) + } + pos := tok.LineStart(line) + + kv := &ast.KeyValueExpr{ + Key: &ast.Ident{ + NamePos: pos, + Name: fieldName, + }, + Colon: pos, + } + if expr, ok := prefilledFields[fieldName]; ok { + kv.Value = expr + } else { + names, ok := matches[fieldTyp] + if !ok { + return nil, fmt.Errorf("invalid struct field type: %v", fieldTyp) + } + + // Find the name most similar to the field name. + // If no name matches the pattern, generate a zero value. + // NOTE: We currently match on the name of the field key rather than the field type. + if best := fuzzy.BestMatch(fieldName, names); best != "" { + kv.Value = ast.NewIdent(best) + } else if v := populateValue(file, pkg, fieldTyp); v != nil { + kv.Value = v + } else { + return nil, nil + } + } + elts = append(elts, kv) + line++ + } + + // If all of the struct's fields are unexported, we have nothing to do. + if len(elts) == 0 { + return nil, fmt.Errorf("no elements to fill") + } + + // Add the final line for the right brace. Offset is the number of + // bytes already added plus 1. + tok.AddLine(len(elts) + 1) + line = len(elts) + 2 + if line > tok.LineCount() { + panic(fmt.Sprintf("invalid line number %v (of %v) for fillstruct", line, tok.LineCount())) + } + + cl := &ast.CompositeLit{ + Type: expr.Type, + Lbrace: tok.LineStart(1), + Elts: elts, + Rbrace: tok.LineStart(line), + } + + // Find the line on which the composite literal is declared. + split := bytes.Split(content, []byte("\n")) + lineNumber := safetoken.StartPosition(fset, expr.Lbrace).Line + firstLine := split[lineNumber-1] // lines are 1-indexed + + // Trim the whitespace from the left of the line, and use the index + // to get the amount of whitespace on the left. + trimmed := bytes.TrimLeftFunc(firstLine, unicode.IsSpace) + index := bytes.Index(firstLine, trimmed) + whitespace := firstLine[:index] + + // First pass through the formatter: turn the expr into a string. + var formatBuf bytes.Buffer + if err := format.Node(&formatBuf, fakeFset, cl); err != nil { + return nil, fmt.Errorf("failed to run first format on:\n%s\ngot err: %v", cl.Type, err) + } + sug := indent(formatBuf.Bytes(), whitespace) + + if len(prefilledFields) > 0 { + // Attempt a second pass through the formatter to line up columns. + sourced, err := format.Source(sug) + if err == nil { + sug = indent(sourced, whitespace) + } + } + + return &analysis.SuggestedFix{ + TextEdits: []analysis.TextEdit{ + { + Pos: expr.Pos(), + End: expr.End(), + NewText: sug, + }, + }, + }, nil +} + +// indent works line by line through str, indenting (prefixing) each line with +// ind. +func indent(str, ind []byte) []byte { + split := bytes.Split(str, []byte("\n")) + newText := bytes.NewBuffer(nil) + for i, s := range split { + if len(s) == 0 { + continue + } + // Don't add the extra indentation to the first line. + if i != 0 { + newText.Write(ind) + } + newText.Write(s) + if i < len(split)-1 { + newText.WriteByte('\n') + } + } + return newText.Bytes() +} + +// populateValue constructs an expression to fill the value of a struct field. +// +// When the type of a struct field is a basic literal or interface, we return +// default values. For other types, such as maps, slices, and channels, we create +// empty expressions such as []T{} or make(chan T) rather than using default values. +// +// The reasoning here is that users will call fillstruct with the intention of +// initializing the struct, in which case setting these fields to nil has no effect. +func populateValue(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr { + switch u := typ.Underlying().(type) { + case *types.Basic: + switch { + case u.Info()&types.IsNumeric != 0: + return &ast.BasicLit{Kind: token.INT, Value: "0"} + case u.Info()&types.IsBoolean != 0: + return &ast.Ident{Name: "false"} + case u.Info()&types.IsString != 0: + return &ast.BasicLit{Kind: token.STRING, Value: `""`} + case u.Kind() == types.UnsafePointer: + return ast.NewIdent("nil") + default: + panic("unknown basic type") + } + + case *types.Map: + k := analysisinternal.TypeExpr(f, pkg, u.Key()) + v := analysisinternal.TypeExpr(f, pkg, u.Elem()) + if k == nil || v == nil { + return nil + } + return &ast.CompositeLit{ + Type: &ast.MapType{ + Key: k, + Value: v, + }, + } + case *types.Slice: + s := analysisinternal.TypeExpr(f, pkg, u.Elem()) + if s == nil { + return nil + } + return &ast.CompositeLit{ + Type: &ast.ArrayType{ + Elt: s, + }, + } + + case *types.Array: + a := analysisinternal.TypeExpr(f, pkg, u.Elem()) + if a == nil { + return nil + } + return &ast.CompositeLit{ + Type: &ast.ArrayType{ + Elt: a, + Len: &ast.BasicLit{ + Kind: token.INT, Value: fmt.Sprintf("%v", u.Len()), + }, + }, + } + + case *types.Chan: + v := analysisinternal.TypeExpr(f, pkg, u.Elem()) + if v == nil { + return nil + } + dir := ast.ChanDir(u.Dir()) + if u.Dir() == types.SendRecv { + dir = ast.SEND | ast.RECV + } + return &ast.CallExpr{ + Fun: ast.NewIdent("make"), + Args: []ast.Expr{ + &ast.ChanType{ + Dir: dir, + Value: v, + }, + }, + } + + case *types.Struct: + s := analysisinternal.TypeExpr(f, pkg, typ) + if s == nil { + return nil + } + return &ast.CompositeLit{ + Type: s, + } + + case *types.Signature: + var params []*ast.Field + for i := 0; i < u.Params().Len(); i++ { + p := analysisinternal.TypeExpr(f, pkg, u.Params().At(i).Type()) + if p == nil { + return nil + } + params = append(params, &ast.Field{ + Type: p, + Names: []*ast.Ident{ + { + Name: u.Params().At(i).Name(), + }, + }, + }) + } + var returns []*ast.Field + for i := 0; i < u.Results().Len(); i++ { + r := analysisinternal.TypeExpr(f, pkg, u.Results().At(i).Type()) + if r == nil { + return nil + } + returns = append(returns, &ast.Field{ + Type: r, + }) + } + return &ast.FuncLit{ + Type: &ast.FuncType{ + Params: &ast.FieldList{ + List: params, + }, + Results: &ast.FieldList{ + List: returns, + }, + }, + Body: &ast.BlockStmt{}, + } + + case *types.Pointer: + switch u.Elem().(type) { + case *types.Basic: + return &ast.CallExpr{ + Fun: &ast.Ident{ + Name: "new", + }, + Args: []ast.Expr{ + &ast.Ident{ + Name: u.Elem().String(), + }, + }, + } + default: + return &ast.UnaryExpr{ + Op: token.AND, + X: populateValue(f, pkg, u.Elem()), + } + } + + case *types.Interface: + if param, ok := typ.(*typeparams.TypeParam); ok { + // *new(T) is the zero value of a type parameter T. + // TODO(adonovan): one could give a more specific zero + // value if the type has a core type that is, say, + // always a number or a pointer. See go/ssa for details. + return &ast.StarExpr{ + X: &ast.CallExpr{ + Fun: ast.NewIdent("new"), + Args: []ast.Expr{ + ast.NewIdent(param.Obj().Name()), + }, + }, + } + } + + return ast.NewIdent("nil") + } + return nil +} + +func deref(t types.Type) types.Type { + for { + ptr, ok := t.Underlying().(*types.Pointer) + if !ok { + return t + } + t = ptr.Elem() + } +} diff --git a/gopls/internal/lsp/analysis/fillstruct/fillstruct_test.go b/gopls/internal/lsp/analysis/fillstruct/fillstruct_test.go new file mode 100644 index 000000000..66642b7ab --- /dev/null +++ b/gopls/internal/lsp/analysis/fillstruct/fillstruct_test.go @@ -0,0 +1,22 @@ +// 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_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/fillstruct" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.Run(t, testdata, fillstruct.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/fillstruct/testdata/src/a/a.go b/gopls/internal/lsp/analysis/fillstruct/testdata/src/a/a.go new file mode 100644 index 000000000..9ee3860fc --- /dev/null +++ b/gopls/internal/lsp/analysis/fillstruct/testdata/src/a/a.go @@ -0,0 +1,113 @@ +// 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 + +import ( + data "b" + "go/ast" + "go/token" + "unsafe" +) + +type emptyStruct struct{} + +var _ = emptyStruct{} + +type basicStruct struct { + foo int +} + +var _ = basicStruct{} // want `Fill basicStruct` + +type twoArgStruct struct { + foo int + bar string +} + +var _ = twoArgStruct{} // want `Fill twoArgStruct` + +var _ = twoArgStruct{ // want `Fill twoArgStruct` + bar: "bar", +} + +type nestedStruct struct { + bar string + basic basicStruct +} + +var _ = nestedStruct{} // want `Fill nestedStruct` + +var _ = data.B{} // want `Fill b.B` + +type typedStruct struct { + m map[string]int + s []int + c chan int + c1 <-chan int + a [2]string +} + +var _ = typedStruct{} // want `Fill typedStruct` + +type funStruct struct { + fn func(i int) int +} + +var _ = funStruct{} // want `Fill funStruct` + +type funStructComplex struct { + fn func(i int, s string) (string, int) +} + +var _ = funStructComplex{} // want `Fill funStructComplex` + +type funStructEmpty struct { + fn func() +} + +var _ = funStructEmpty{} // want `Fill funStructEmpty` + +type Foo struct { + A int +} + +type Bar struct { + X *Foo + Y *Foo +} + +var _ = Bar{} // want `Fill Bar` + +type importedStruct struct { + m map[*ast.CompositeLit]ast.Field + s []ast.BadExpr + a [3]token.Token + c chan ast.EmptyStmt + fn func(ast_decl ast.DeclStmt) ast.Ellipsis + st ast.CompositeLit +} + +var _ = importedStruct{} // want `Fill importedStruct` + +type pointerBuiltinStruct struct { + b *bool + s *string + i *int +} + +var _ = pointerBuiltinStruct{} // want `Fill pointerBuiltinStruct` + +var _ = []ast.BasicLit{ + {}, // want `Fill go/ast.BasicLit` +} + +var _ = []ast.BasicLit{{}, // want "go/ast.BasicLit" +} + +type unsafeStruct struct { + foo unsafe.Pointer +} + +var _ = unsafeStruct{} // want `Fill unsafeStruct` diff --git a/gopls/internal/lsp/analysis/fillstruct/testdata/src/b/b.go b/gopls/internal/lsp/analysis/fillstruct/testdata/src/b/b.go new file mode 100644 index 000000000..a4b394605 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillstruct/testdata/src/b/b.go @@ -0,0 +1,6 @@ +package fillstruct + +type B struct { + ExportedInt int + unexportedInt int +} diff --git a/gopls/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go b/gopls/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..46bb8ae40 --- /dev/null +++ b/gopls/internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go @@ -0,0 +1,50 @@ +// 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]{} // want `Fill basicStruct\[int\]` + +type twoArgStruct[F, B any] struct { + foo F + bar B +} + +var _ = twoArgStruct[string, int]{} // want `Fill twoArgStruct\[string, int\]` + +var _ = twoArgStruct[int, string]{ // want `Fill twoArgStruct\[int, string\]` + bar: "bar", +} + +type nestedStruct struct { + bar string + basic basicStruct[int] +} + +var _ = nestedStruct{} // want "Fill nestedStruct" + +func _[T any]() { + type S struct{ t T } + x := S{} // want "Fill S" + _ = x +} + +func Test() { + var tests = []struct { + a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p string + }{ + {}, // want "Fill anonymous struct { a: string, b: string, c: string, ... }" + } + for _, test := range tests { + _ = test + } +} diff --git a/gopls/internal/lsp/analysis/infertypeargs/infertypeargs.go b/gopls/internal/lsp/analysis/infertypeargs/infertypeargs.go new file mode 100644 index 000000000..119de50ce --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/infertypeargs_test.go b/gopls/internal/lsp/analysis/infertypeargs/infertypeargs_test.go new file mode 100644 index 000000000..70855e1ab --- /dev/null +++ b/gopls/internal/lsp/analysis/infertypeargs/infertypeargs_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 infertypeargs_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/infertypeargs" + "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, infertypeargs.Analyzer, "a") +} diff --git a/gopls/internal/lsp/analysis/infertypeargs/run_go117.go b/gopls/internal/lsp/analysis/infertypeargs/run_go117.go new file mode 100644 index 000000000..bc5c29b51 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/run_go118.go b/gopls/internal/lsp/analysis/infertypeargs/run_go118.go new file mode 100644 index 000000000..66457429a --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go new file mode 100644 index 000000000..1c3d88ba1 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden new file mode 100644 index 000000000..72348ff77 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go new file mode 100644 index 000000000..fc1f763df --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden new file mode 100644 index 000000000..6099545bb --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go new file mode 100644 index 000000000..f0610a8b4 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go new file mode 100644 index 000000000..c304f1d0d --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden b/gopls/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden new file mode 100644 index 000000000..93c6f707c --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/nonewvars/nonewvars.go b/gopls/internal/lsp/analysis/nonewvars/nonewvars.go new file mode 100644 index 000000000..6937b36d1 --- /dev/null +++ b/gopls/internal/lsp/analysis/nonewvars/nonewvars.go @@ -0,0 +1,95 @@ +// 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 nonewvars defines an Analyzer that applies suggested fixes +// to errors of the type "no new variables on left side of :=". +package nonewvars + +import ( + "bytes" + "go/ast" + "go/format" + "go/token" + + "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/analysisinternal" +) + +const Doc = `suggested fixes for "no new vars on left side of :=" + +This checker provides suggested fixes for type errors of the +type "no new vars on left side of :=". For example: + z := 1 + z := 2 +will turn into + z := 1 + z = 2 +` + +var Analyzer = &analysis.Analyzer{ + Name: "nonewvars", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if len(pass.TypeErrors) == 0 { + return nil, nil + } + + nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)} + inspect.Preorder(nodeFilter, func(n ast.Node) { + assignStmt, _ := n.(*ast.AssignStmt) + // We only care about ":=". + if assignStmt.Tok != token.DEFINE { + return + } + + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= assignStmt.Pos() && assignStmt.Pos() < f.End() { + file = f + break + } + } + if file == nil { + return + } + + for _, err := range pass.TypeErrors { + if !FixesError(err.Msg) { + continue + } + if assignStmt.Pos() > err.Pos || err.Pos >= assignStmt.End() { + continue + } + var buf bytes.Buffer + if err := format.Node(&buf, pass.Fset, file); err != nil { + continue + } + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos), + Message: err.Msg, + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Change ':=' to '='", + TextEdits: []analysis.TextEdit{{ + Pos: err.Pos, + End: err.Pos + 1, + }}, + }}, + }) + } + }) + return nil, nil +} + +func FixesError(msg string) bool { + return msg == "no new variables on left side of :=" +} diff --git a/gopls/internal/lsp/analysis/nonewvars/nonewvars_test.go b/gopls/internal/lsp/analysis/nonewvars/nonewvars_test.go new file mode 100644 index 000000000..8f6f0a51f --- /dev/null +++ b/gopls/internal/lsp/analysis/nonewvars/nonewvars_test.go @@ -0,0 +1,22 @@ +// 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 nonewvars_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/nonewvars" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, nonewvars.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go b/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go new file mode 100644 index 000000000..97d8fcde1 --- /dev/null +++ b/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go @@ -0,0 +1,16 @@ +// 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 nonewvars + +import "log" + +func x() { + z := 1 + z := 2 // want "no new variables on left side of :=" + + _, z := 3, 100 // want "no new variables on left side of :=" + + log.Println(z) +} diff --git a/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go.golden new file mode 100644 index 000000000..17197e564 --- /dev/null +++ b/gopls/internal/lsp/analysis/nonewvars/testdata/src/a/a.go.golden @@ -0,0 +1,16 @@ +// 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 nonewvars + +import "log" + +func x() { + z := 1 + z = 2 // want "no new variables on left side of :=" + + _, z = 3, 100 // want "no new variables on left side of :=" + + log.Println(z) +} diff --git a/gopls/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go b/gopls/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go new file mode 100644 index 000000000..b381c9c09 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go.golden b/gopls/internal/lsp/analysis/nonewvars/testdata/src/typeparams/a.go.golden new file mode 100644 index 000000000..3a5117301 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go new file mode 100644 index 000000000..41952a547 --- /dev/null +++ b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues.go @@ -0,0 +1,92 @@ +// 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 noresultvalues defines an Analyzer that applies suggested fixes +// to errors of the type "no result values expected". +package noresultvalues + +import ( + "bytes" + "go/ast" + "go/format" + "strings" + + "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/analysisinternal" +) + +const Doc = `suggested fixes for unexpected return values + +This checker provides suggested fixes for type errors of the +type "no result values expected" or "too many return values". +For example: + func z() { return nil } +will turn into + func z() { return } +` + +var Analyzer = &analysis.Analyzer{ + Name: "noresultvalues", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if len(pass.TypeErrors) == 0 { + return nil, nil + } + + nodeFilter := []ast.Node{(*ast.ReturnStmt)(nil)} + inspect.Preorder(nodeFilter, func(n ast.Node) { + retStmt, _ := n.(*ast.ReturnStmt) + + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= retStmt.Pos() && retStmt.Pos() < f.End() { + file = f + break + } + } + if file == nil { + return + } + + for _, err := range pass.TypeErrors { + if !FixesError(err.Msg) { + continue + } + if retStmt.Pos() >= err.Pos || err.Pos >= retStmt.End() { + continue + } + var buf bytes.Buffer + if err := format.Node(&buf, pass.Fset, file); err != nil { + continue + } + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos), + Message: err.Msg, + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Delete return values", + TextEdits: []analysis.TextEdit{{ + Pos: retStmt.Pos(), + End: retStmt.End(), + NewText: []byte("return"), + }}, + }}, + }) + } + }) + return nil, nil +} + +func FixesError(msg string) bool { + return msg == "no result values expected" || + strings.HasPrefix(msg, "too many return values") && strings.Contains(msg, "want ()") +} diff --git a/gopls/internal/lsp/analysis/noresultvalues/noresultvalues_test.go b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues_test.go new file mode 100644 index 000000000..24ce39207 --- /dev/null +++ b/gopls/internal/lsp/analysis/noresultvalues/noresultvalues_test.go @@ -0,0 +1,22 @@ +// 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 noresultvalues_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/noresultvalues" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, noresultvalues.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go new file mode 100644 index 000000000..3daa7f7c7 --- /dev/null +++ b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go @@ -0,0 +1,9 @@ +// 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 noresultvalues + +func x() { return nil } // want `no result values expected|too many return values` + +func y() { return nil, "hello" } // want `no result values expected|too many return values` diff --git a/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden new file mode 100644 index 000000000..5e93aa413 --- /dev/null +++ b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden @@ -0,0 +1,9 @@ +// 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 noresultvalues + +func x() { return } // want `no result values expected|too many return values` + +func y() { return } // want `no result values expected|too many return values` diff --git a/gopls/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go new file mode 100644 index 000000000..f8aa43665 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden b/gopls/internal/lsp/analysis/noresultvalues/testdata/src/typeparams/a.go.golden new file mode 100644 index 000000000..963e3f4e1 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit.go b/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit.go new file mode 100644 index 000000000..c91fc7577 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit.go @@ -0,0 +1,196 @@ +// 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 simplifycompositelit defines an Analyzer that simplifies composite literals. +// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go +// https://golang.org/cmd/gofmt/#hdr-The_simplify_command +package simplifycompositelit + +import ( + "bytes" + "fmt" + "go/ast" + "go/printer" + "go/token" + "reflect" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `check for composite literal simplifications + +An array, slice, or map composite literal of the form: + []T{T{}, T{}} +will be simplified to: + []T{{}, {}} + +This is one of the simplifications that "gofmt -s" applies.` + +var Analyzer = &analysis.Analyzer{ + Name: "simplifycompositelit", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{(*ast.CompositeLit)(nil)} + inspect.Preorder(nodeFilter, func(n ast.Node) { + expr := n.(*ast.CompositeLit) + + outer := expr + var keyType, eltType ast.Expr + switch typ := outer.Type.(type) { + case *ast.ArrayType: + eltType = typ.Elt + case *ast.MapType: + keyType = typ.Key + eltType = typ.Value + } + + if eltType == nil { + return + } + var ktyp reflect.Value + if keyType != nil { + ktyp = reflect.ValueOf(keyType) + } + typ := reflect.ValueOf(eltType) + for _, x := range outer.Elts { + // look at value of indexed/named elements + if t, ok := x.(*ast.KeyValueExpr); ok { + if keyType != nil { + simplifyLiteral(pass, ktyp, keyType, t.Key) + } + x = t.Value + } + simplifyLiteral(pass, typ, eltType, x) + } + }) + return nil, nil +} + +func simplifyLiteral(pass *analysis.Pass, typ reflect.Value, astType, x ast.Expr) { + // if the element is a composite literal and its literal type + // matches the outer literal's element type exactly, the inner + // literal type may be omitted + if inner, ok := x.(*ast.CompositeLit); ok && match(typ, reflect.ValueOf(inner.Type)) { + var b bytes.Buffer + printer.Fprint(&b, pass.Fset, inner.Type) + createDiagnostic(pass, inner.Type.Pos(), inner.Type.End(), b.String()) + } + // if the outer literal's element type is a pointer type *T + // and the element is & of a composite literal of type T, + // the inner &T may be omitted. + if ptr, ok := astType.(*ast.StarExpr); ok { + if addr, ok := x.(*ast.UnaryExpr); ok && addr.Op == token.AND { + if inner, ok := addr.X.(*ast.CompositeLit); ok { + if match(reflect.ValueOf(ptr.X), reflect.ValueOf(inner.Type)) { + var b bytes.Buffer + printer.Fprint(&b, pass.Fset, inner.Type) + // Account for the & by subtracting 1 from typ.Pos(). + createDiagnostic(pass, inner.Type.Pos()-1, inner.Type.End(), "&"+b.String()) + } + } + } + } +} + +func createDiagnostic(pass *analysis.Pass, start, end token.Pos, typ string) { + pass.Report(analysis.Diagnostic{ + Pos: start, + End: end, + Message: "redundant type from array, slice, or map composite literal", + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Remove '%s'", typ), + TextEdits: []analysis.TextEdit{{ + Pos: start, + End: end, + NewText: []byte{}, + }}, + }}, + }) +} + +// match reports whether pattern matches val, +// recording wildcard submatches in m. +// If m == nil, match checks whether pattern == val. +// from https://github.com/golang/go/blob/26154f31ad6c801d8bad5ef58df1e9263c6beec7/src/cmd/gofmt/rewrite.go#L160 +func match(pattern, val reflect.Value) bool { + // Otherwise, pattern and val must match recursively. + if !pattern.IsValid() || !val.IsValid() { + return !pattern.IsValid() && !val.IsValid() + } + if pattern.Type() != val.Type() { + return false + } + + // Special cases. + switch pattern.Type() { + case identType: + // For identifiers, only the names need to match + // (and none of the other *ast.Object information). + // This is a common case, handle it all here instead + // of recursing down any further via reflection. + p := pattern.Interface().(*ast.Ident) + v := val.Interface().(*ast.Ident) + return p == nil && v == nil || p != nil && v != nil && p.Name == v.Name + case objectPtrType, positionType: + // object pointers and token positions always match + return true + case callExprType: + // For calls, the Ellipsis fields (token.Position) must + // match since that is how f(x) and f(x...) are different. + // Check them here but fall through for the remaining fields. + p := pattern.Interface().(*ast.CallExpr) + v := val.Interface().(*ast.CallExpr) + if p.Ellipsis.IsValid() != v.Ellipsis.IsValid() { + return false + } + } + + p := reflect.Indirect(pattern) + v := reflect.Indirect(val) + if !p.IsValid() || !v.IsValid() { + return !p.IsValid() && !v.IsValid() + } + + switch p.Kind() { + case reflect.Slice: + if p.Len() != v.Len() { + return false + } + for i := 0; i < p.Len(); i++ { + if !match(p.Index(i), v.Index(i)) { + return false + } + } + return true + + case reflect.Struct: + for i := 0; i < p.NumField(); i++ { + if !match(p.Field(i), v.Field(i)) { + return false + } + } + return true + + case reflect.Interface: + return match(p.Elem(), v.Elem()) + } + + // Handle token integers, etc. + return p.Interface() == v.Interface() +} + +// Values/types for special cases. +var ( + identType = reflect.TypeOf((*ast.Ident)(nil)) + objectPtrType = reflect.TypeOf((*ast.Object)(nil)) + positionType = reflect.TypeOf(token.NoPos) + callExprType = reflect.TypeOf((*ast.CallExpr)(nil)) +) diff --git a/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit_test.go b/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit_test.go new file mode 100644 index 000000000..b0365a6b3 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifycompositelit/simplifycompositelit_test.go @@ -0,0 +1,17 @@ +// 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 simplifycompositelit_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/simplifycompositelit" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, simplifycompositelit.Analyzer, "a") +} diff --git a/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go b/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go new file mode 100644 index 000000000..14e0fa3ae --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go @@ -0,0 +1,234 @@ +// 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 testdata + +type T struct { + x, y int +} + +type T2 struct { + w, z int +} + +var _ = [42]T{ + T{}, // want "redundant type from array, slice, or map composite literal" + T{1, 2}, // want "redundant type from array, slice, or map composite literal" + T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [...]T{ + T{}, // want "redundant type from array, slice, or map composite literal" + T{1, 2}, // want "redundant type from array, slice, or map composite literal" + T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []T{ + T{}, // want "redundant type from array, slice, or map composite literal" + T{1, 2}, // want "redundant type from array, slice, or map composite literal" + T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []T{ + T{}, // want "redundant type from array, slice, or map composite literal" + 10: T{1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []struct { + x, y int +}{ + struct{ x, y int }{}, // want "redundant type from array, slice, or map composite literal" + 10: struct{ x, y int }{1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: struct{ x, y int }{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []interface{}{ + T{}, + 10: T{1, 2}, + 20: T{3, 4}, +} + +var _ = [][]int{ + []int{}, // want "redundant type from array, slice, or map composite literal" + []int{1, 2}, // want "redundant type from array, slice, or map composite literal" + []int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [][]int{ + ([]int{}), + ([]int{1, 2}), + []int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [][][]int{ + [][]int{}, // want "redundant type from array, slice, or map composite literal" + [][]int{ // want "redundant type from array, slice, or map composite literal" + []int{}, // want "redundant type from array, slice, or map composite literal" + []int{0, 1, 2, 3}, // want "redundant type from array, slice, or map composite literal" + []int{4, 5}, // want "redundant type from array, slice, or map composite literal" + }, +} + +var _ = map[string]T{ + "foo": T{}, // want "redundant type from array, slice, or map composite literal" + "bar": T{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]struct { + x, y int +}{ + "foo": struct{ x, y int }{}, // want "redundant type from array, slice, or map composite literal" + "bar": struct{ x, y int }{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": struct{ x, y int }{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]interface{}{ + "foo": T{}, + "bar": T{1, 2}, + "bal": T{3, 4}, +} + +var _ = map[string][]int{ + "foo": []int{}, // want "redundant type from array, slice, or map composite literal" + "bar": []int{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": []int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string][]int{ + "foo": ([]int{}), + "bar": ([]int{1, 2}), + "bal": []int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +type Point struct { + a int + b int +} + +type Piece struct { + a int + b int + c Point + d []Point + e *Point + f *Point +} + +// from exp/4s/data.go +var pieces3 = []Piece{ + Piece{0, 0, Point{4, 1}, []Point{Point{0, 0}, Point{1, 0}, Point{1, 0}, Point{1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + Piece{1, 0, Point{1, 4}, []Point{Point{0, 0}, Point{0, 1}, Point{0, 1}, Point{0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + Piece{2, 0, Point{4, 1}, []Point{Point{0, 0}, Point{1, 0}, Point{1, 0}, Point{1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + Piece{3, 0, Point{1, 4}, []Point{Point{0, 0}, Point{0, 1}, Point{0, 1}, Point{0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = [42]*T{ + &T{}, // want "redundant type from array, slice, or map composite literal" + &T{1, 2}, // want "redundant type from array, slice, or map composite literal" + &T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [...]*T{ + &T{}, // want "redundant type from array, slice, or map composite literal" + &T{1, 2}, // want "redundant type from array, slice, or map composite literal" + &T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*T{ + &T{}, // want "redundant type from array, slice, or map composite literal" + &T{1, 2}, // want "redundant type from array, slice, or map composite literal" + &T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*T{ + &T{}, // want "redundant type from array, slice, or map composite literal" + 10: &T{1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: &T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*struct { + x, y int +}{ + &struct{ x, y int }{}, // want "redundant type from array, slice, or map composite literal" + 10: &struct{ x, y int }{1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: &struct{ x, y int }{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []interface{}{ + &T{}, + 10: &T{1, 2}, + 20: &T{3, 4}, +} + +var _ = []*[]int{ + &[]int{}, // want "redundant type from array, slice, or map composite literal" + &[]int{1, 2}, // want "redundant type from array, slice, or map composite literal" + &[]int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*[]int{ + (&[]int{}), + (&[]int{1, 2}), + &[]int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*[]*[]int{ + &[]*[]int{}, // want "redundant type from array, slice, or map composite literal" + &[]*[]int{ // want "redundant type from array, slice, or map composite literal" + &[]int{}, // want "redundant type from array, slice, or map composite literal" + &[]int{0, 1, 2, 3}, // want "redundant type from array, slice, or map composite literal" + &[]int{4, 5}, // want "redundant type from array, slice, or map composite literal" + }, +} + +var _ = map[string]*T{ + "foo": &T{}, // want "redundant type from array, slice, or map composite literal" + "bar": &T{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": &T{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]*struct { + x, y int +}{ + "foo": &struct{ x, y int }{}, // want "redundant type from array, slice, or map composite literal" + "bar": &struct{ x, y int }{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": &struct{ x, y int }{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]interface{}{ + "foo": &T{}, + "bar": &T{1, 2}, + "bal": &T{3, 4}, +} + +var _ = map[string]*[]int{ + "foo": &[]int{}, // want "redundant type from array, slice, or map composite literal" + "bar": &[]int{1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": &[]int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]*[]int{ + "foo": (&[]int{}), + "bar": (&[]int{1, 2}), + "bal": &[]int{3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var pieces4 = []*Piece{ + &Piece{0, 0, Point{4, 1}, []Point{Point{0, 0}, Point{1, 0}, Point{1, 0}, Point{1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + &Piece{1, 0, Point{1, 4}, []Point{Point{0, 0}, Point{0, 1}, Point{0, 1}, Point{0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + &Piece{2, 0, Point{4, 1}, []Point{Point{0, 0}, Point{1, 0}, Point{1, 0}, Point{1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + &Piece{3, 0, Point{1, 4}, []Point{Point{0, 0}, Point{0, 1}, Point{0, 1}, Point{0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = map[T]T2{ + T{1, 2}: T2{3, 4}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + T{5, 6}: T2{7, 8}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = map[*T]*T2{ + &T{1, 2}: &T2{3, 4}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + &T{5, 6}: &T2{7, 8}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} diff --git a/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go.golden new file mode 100644 index 000000000..6bfed45a5 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifycompositelit/testdata/src/a/a.go.golden @@ -0,0 +1,234 @@ +// 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 testdata + +type T struct { + x, y int +} + +type T2 struct { + w, z int +} + +var _ = [42]T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [...]T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []T{ + {}, // want "redundant type from array, slice, or map composite literal" + 10: {1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []struct { + x, y int +}{ + {}, // want "redundant type from array, slice, or map composite literal" + 10: {1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []interface{}{ + T{}, + 10: T{1, 2}, + 20: T{3, 4}, +} + +var _ = [][]int{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [][]int{ + ([]int{}), + ([]int{1, 2}), + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [][][]int{ + {}, // want "redundant type from array, slice, or map composite literal" + { // want "redundant type from array, slice, or map composite literal" + {}, // want "redundant type from array, slice, or map composite literal" + {0, 1, 2, 3}, // want "redundant type from array, slice, or map composite literal" + {4, 5}, // want "redundant type from array, slice, or map composite literal" + }, +} + +var _ = map[string]T{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]struct { + x, y int +}{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]interface{}{ + "foo": T{}, + "bar": T{1, 2}, + "bal": T{3, 4}, +} + +var _ = map[string][]int{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string][]int{ + "foo": ([]int{}), + "bar": ([]int{1, 2}), + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +type Point struct { + a int + b int +} + +type Piece struct { + a int + b int + c Point + d []Point + e *Point + f *Point +} + +// from exp/4s/data.go +var pieces3 = []Piece{ + {0, 0, Point{4, 1}, []Point{{0, 0}, {1, 0}, {1, 0}, {1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {1, 0, Point{1, 4}, []Point{{0, 0}, {0, 1}, {0, 1}, {0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {2, 0, Point{4, 1}, []Point{{0, 0}, {1, 0}, {1, 0}, {1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {3, 0, Point{1, 4}, []Point{{0, 0}, {0, 1}, {0, 1}, {0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = [42]*T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = [...]*T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*T{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*T{ + {}, // want "redundant type from array, slice, or map composite literal" + 10: {1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*struct { + x, y int +}{ + {}, // want "redundant type from array, slice, or map composite literal" + 10: {1, 2}, // want "redundant type from array, slice, or map composite literal" + 20: {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []interface{}{ + &T{}, + 10: &T{1, 2}, + 20: &T{3, 4}, +} + +var _ = []*[]int{ + {}, // want "redundant type from array, slice, or map composite literal" + {1, 2}, // want "redundant type from array, slice, or map composite literal" + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*[]int{ + (&[]int{}), + (&[]int{1, 2}), + {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = []*[]*[]int{ + {}, // want "redundant type from array, slice, or map composite literal" + { // want "redundant type from array, slice, or map composite literal" + {}, // want "redundant type from array, slice, or map composite literal" + {0, 1, 2, 3}, // want "redundant type from array, slice, or map composite literal" + {4, 5}, // want "redundant type from array, slice, or map composite literal" + }, +} + +var _ = map[string]*T{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]*struct { + x, y int +}{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]interface{}{ + "foo": &T{}, + "bar": &T{1, 2}, + "bal": &T{3, 4}, +} + +var _ = map[string]*[]int{ + "foo": {}, // want "redundant type from array, slice, or map composite literal" + "bar": {1, 2}, // want "redundant type from array, slice, or map composite literal" + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var _ = map[string]*[]int{ + "foo": (&[]int{}), + "bar": (&[]int{1, 2}), + "bal": {3, 4}, // want "redundant type from array, slice, or map composite literal" +} + +var pieces4 = []*Piece{ + {0, 0, Point{4, 1}, []Point{{0, 0}, {1, 0}, {1, 0}, {1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {1, 0, Point{1, 4}, []Point{{0, 0}, {0, 1}, {0, 1}, {0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {2, 0, Point{4, 1}, []Point{{0, 0}, {1, 0}, {1, 0}, {1, 0}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {3, 0, Point{1, 4}, []Point{{0, 0}, {0, 1}, {0, 1}, {0, 1}}, nil, nil}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = map[T]T2{ + {1, 2}: {3, 4}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {5, 6}: {7, 8}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} + +var _ = map[*T]*T2{ + {1, 2}: {3, 4}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" + {5, 6}: {7, 8}, // want "redundant type from array, slice, or map composite literal" "redundant type from array, slice, or map composite literal" +} diff --git a/gopls/internal/lsp/analysis/simplifyrange/simplifyrange.go b/gopls/internal/lsp/analysis/simplifyrange/simplifyrange.go new file mode 100644 index 000000000..c9cb38798 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyrange/simplifyrange.go @@ -0,0 +1,116 @@ +// 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 simplifyrange defines an Analyzer that simplifies range statements. +// https://golang.org/cmd/gofmt/#hdr-The_simplify_command +// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go +package simplifyrange + +import ( + "bytes" + "go/ast" + "go/printer" + "go/token" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `check for range statement simplifications + +A range of the form: + for x, _ = range v {...} +will be simplified to: + for x = range v {...} + +A range of the form: + for _ = range v {...} +will be simplified to: + for range v {...} + +This is one of the simplifications that "gofmt -s" applies.` + +var Analyzer = &analysis.Analyzer{ + Name: "simplifyrange", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.RangeStmt)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + var copy *ast.RangeStmt + if stmt, ok := n.(*ast.RangeStmt); ok { + x := *stmt + copy = &x + } + if copy == nil { + return + } + end := newlineIndex(pass.Fset, copy) + + // Range statements of the form: for i, _ := range x {} + var old ast.Expr + if isBlank(copy.Value) { + old = copy.Value + copy.Value = nil + } + // Range statements of the form: for _ := range x {} + if isBlank(copy.Key) && copy.Value == nil { + old = copy.Key + copy.Key = nil + } + // Return early if neither if condition is met. + if old == nil { + return + } + pass.Report(analysis.Diagnostic{ + Pos: old.Pos(), + End: old.End(), + Message: "simplify range expression", + SuggestedFixes: suggestedFixes(pass.Fset, copy, end), + }) + }) + return nil, nil +} + +func suggestedFixes(fset *token.FileSet, rng *ast.RangeStmt, end token.Pos) []analysis.SuggestedFix { + var b bytes.Buffer + printer.Fprint(&b, fset, rng) + stmt := b.Bytes() + index := bytes.Index(stmt, []byte("\n")) + // If there is a new line character, then don't replace the body. + if index != -1 { + stmt = stmt[:index] + } + return []analysis.SuggestedFix{{ + Message: "Remove empty value", + TextEdits: []analysis.TextEdit{{ + Pos: rng.Pos(), + End: end, + NewText: stmt[:index], + }}, + }} +} + +func newlineIndex(fset *token.FileSet, rng *ast.RangeStmt) token.Pos { + var b bytes.Buffer + printer.Fprint(&b, fset, rng) + contents := b.Bytes() + index := bytes.Index(contents, []byte("\n")) + if index == -1 { + return rng.End() + } + return rng.Pos() + token.Pos(index) +} + +func isBlank(x ast.Expr) bool { + ident, ok := x.(*ast.Ident) + return ok && ident.Name == "_" +} diff --git a/gopls/internal/lsp/analysis/simplifyrange/simplifyrange_test.go b/gopls/internal/lsp/analysis/simplifyrange/simplifyrange_test.go new file mode 100644 index 000000000..fbd57ec2d --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyrange/simplifyrange_test.go @@ -0,0 +1,17 @@ +// 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 simplifyrange_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/simplifyrange" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, simplifyrange.Analyzer, "a") +} diff --git a/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go b/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go new file mode 100644 index 000000000..49face1e9 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go @@ -0,0 +1,16 @@ +// 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 testdata + +import "log" + +func m() { + maps := make(map[string]string) + for k, _ := range maps { // want "simplify range expression" + log.Println(k) + } + for _ = range maps { // want "simplify range expression" + } +} diff --git a/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden new file mode 100644 index 000000000..ec8490ab3 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyrange/testdata/src/a/a.go.golden @@ -0,0 +1,16 @@ +// 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 testdata + +import "log" + +func m() { + maps := make(map[string]string) + for k := range maps { // want "simplify range expression" + log.Println(k) + } + for range maps { // want "simplify range expression" + } +} diff --git a/gopls/internal/lsp/analysis/simplifyslice/simplifyslice.go b/gopls/internal/lsp/analysis/simplifyslice/simplifyslice.go new file mode 100644 index 000000000..da1728e6f --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyslice/simplifyslice.go @@ -0,0 +1,94 @@ +// 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 simplifyslice defines an Analyzer that simplifies slice statements. +// https://github.com/golang/go/blob/master/src/cmd/gofmt/simplify.go +// https://golang.org/cmd/gofmt/#hdr-The_simplify_command +package simplifyslice + +import ( + "bytes" + "fmt" + "go/ast" + "go/printer" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `check for slice simplifications + +A slice expression of the form: + s[a:len(s)] +will be simplified to: + s[a:] + +This is one of the simplifications that "gofmt -s" applies.` + +var Analyzer = &analysis.Analyzer{ + Name: "simplifyslice", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +// Note: We could also simplify slice expressions of the form s[0:b] to s[:b] +// but we leave them as is since sometimes we want to be very explicit +// about the lower bound. +// An example where the 0 helps: +// x, y, z := b[0:2], b[2:4], b[4:6] +// An example where it does not: +// x, y := b[:n], b[n:] + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.SliceExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + expr := n.(*ast.SliceExpr) + // - 3-index slices always require the 2nd and 3rd index + if expr.Max != nil { + return + } + s, ok := expr.X.(*ast.Ident) + // the array/slice object is a single, resolved identifier + if !ok || s.Obj == nil { + return + } + call, ok := expr.High.(*ast.CallExpr) + // the high expression is a function call with a single argument + if !ok || len(call.Args) != 1 || call.Ellipsis.IsValid() { + return + } + fun, ok := call.Fun.(*ast.Ident) + // the function called is "len" and it is not locally defined; and + // because we don't have dot imports, it must be the predefined len() + if !ok || fun.Name != "len" || fun.Obj != nil { + return + } + arg, ok := call.Args[0].(*ast.Ident) + // the len argument is the array/slice object + if !ok || arg.Obj != s.Obj { + return + } + var b bytes.Buffer + printer.Fprint(&b, pass.Fset, expr.High) + pass.Report(analysis.Diagnostic{ + Pos: expr.High.Pos(), + End: expr.High.End(), + Message: fmt.Sprintf("unneeded: %s", b.String()), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Remove '%s'", b.String()), + TextEdits: []analysis.TextEdit{{ + Pos: expr.High.Pos(), + End: expr.High.End(), + NewText: []byte{}, + }}, + }}, + }) + }) + return nil, nil +} diff --git a/gopls/internal/lsp/analysis/simplifyslice/simplifyslice_test.go b/gopls/internal/lsp/analysis/simplifyslice/simplifyslice_test.go new file mode 100644 index 000000000..41914ba31 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyslice/simplifyslice_test.go @@ -0,0 +1,22 @@ +// 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 simplifyslice_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/simplifyslice" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, simplifyslice.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go new file mode 100644 index 000000000..20792105d --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go @@ -0,0 +1,70 @@ +// 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 testdata + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:len(a)] // want "unneeded: len\\(a\\)" + _ = a[3:(len(a))] + _ = a[len(a)-1 : len(a)] // want "unneeded: len\\(a\\)" + _ = a[2:len(a):len(a)] + + _ = a[:] + _ = a[:10] + _ = a[:len(a)] // want "unneeded: len\\(a\\)" + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(a):len(a)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:len(s)] // want "unneeded: len\\(s\\)" + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + _ = s[2:len(s):len(s)] + + _ = s[:] + _ = s[:10] + _ = s[:len(s)] // want "unneeded: len\\(s\\)" + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + _ = s[:len(s):len(s)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + _ = t.s[2:len(t.s):len(t.s)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] + _ = t.s[:len(t.s):len(t.s)] +) + +func _() { + s := s[0:len(s)] // want "unneeded: len\\(s\\)" + _ = s +} + +func m() { + maps := []int{} + _ = maps[1:len(maps)] // want "unneeded: len\\(maps\\)" +} diff --git a/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden new file mode 100644 index 000000000..45c791421 --- /dev/null +++ b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/a/a.go.golden @@ -0,0 +1,70 @@ +// 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 testdata + +var ( + a [10]byte + b [20]float32 + s []int + t struct { + s []byte + } + + _ = a[0:] + _ = a[1:10] + _ = a[2:] // want "unneeded: len\\(a\\)" + _ = a[3:(len(a))] + _ = a[len(a)-1:] // want "unneeded: len\\(a\\)" + _ = a[2:len(a):len(a)] + + _ = a[:] + _ = a[:10] + _ = a[:] // want "unneeded: len\\(a\\)" + _ = a[:(len(a))] + _ = a[:len(a)-1] + _ = a[:len(a):len(a)] + + _ = s[0:] + _ = s[1:10] + _ = s[2:] // want "unneeded: len\\(s\\)" + _ = s[3:(len(s))] + _ = s[len(a) : len(s)-1] + _ = s[0:len(b)] + _ = s[2:len(s):len(s)] + + _ = s[:] + _ = s[:10] + _ = s[:] // want "unneeded: len\\(s\\)" + _ = s[:(len(s))] + _ = s[:len(s)-1] + _ = s[:len(b)] + _ = s[:len(s):len(s)] + + _ = t.s[0:] + _ = t.s[1:10] + _ = t.s[2:len(t.s)] + _ = t.s[3:(len(t.s))] + _ = t.s[len(a) : len(t.s)-1] + _ = t.s[0:len(b)] + _ = t.s[2:len(t.s):len(t.s)] + + _ = t.s[:] + _ = t.s[:10] + _ = t.s[:len(t.s)] + _ = t.s[:(len(t.s))] + _ = t.s[:len(t.s)-1] + _ = t.s[:len(b)] + _ = t.s[:len(t.s):len(t.s)] +) + +func _() { + s := s[0:] // want "unneeded: len\\(s\\)" + _ = s +} + +func m() { + maps := []int{} + _ = maps[1:] // want "unneeded: len\\(maps\\)" +} diff --git a/gopls/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..69db3100a --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go.golden b/gopls/internal/lsp/analysis/simplifyslice/testdata/src/typeparams/typeparams.go.golden new file mode 100644 index 000000000..99ca9e447 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/stubmethods/stubmethods.go b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go new file mode 100644 index 000000000..e0d2c692c --- /dev/null +++ b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go @@ -0,0 +1,418 @@ +// 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 pass.TypeErrors { + 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.Fset, 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. + Fset *token.FileSet // the FileSet used to type-check the types below + Interface *types.TypeName + Concrete *types.Named + Pointer bool +} + +// GetStubInfo determines whether the "missing method error" +// can be used to deduced what the concrete and interface types are. +// +// TODO(adonovan): this function (and its following 5 helpers) tries +// to deduce a pair of (concrete, interface) types that are related by +// an assignment, either explictly or through a return statement or +// function call. This is essentially what the refactor/satisfy does, +// more generally. Refactor to share logic, after auditing 'satisfy' +// for safety on ill-typed code. +func GetStubInfo(fset *token.FileSet, ti *types.Info, path []ast.Node, pos token.Pos) *StubInfo { + for _, n := range path { + switch n := n.(type) { + case *ast.ValueSpec: + return fromValueSpec(fset, 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(fset, ti, pos, path, n) + return si + case *ast.AssignStmt: + return fromAssignStmt(fset, ti, n, pos) + case *ast.CallExpr: + // Note that some call expressions don't carry the interface type + // because they don't point to a function or method declaration elsewhere. + // For eaxmple, "var Interface = (*Concrete)(nil)". In that case, continue + // this loop to encounter other possibilities such as *ast.ValueSpec or others. + si := fromCallExpr(fset, ti, pos, n) + if si != nil { + return si + } + } + } + return nil +} + +// fromCallExpr tries to find an *ast.CallExpr's function declaration and +// analyzes a function call's signature against the passed in parameter to deduce +// the concrete and interface types. +func fromCallExpr(fset *token.FileSet, ti *types.Info, pos token.Pos, ce *ast.CallExpr) *StubInfo { + paramIdx := -1 + for i, p := range ce.Args { + if pos >= p.Pos() && pos <= p.End() { + paramIdx = i + break + } + } + if paramIdx == -1 { + return nil + } + p := ce.Args[paramIdx] + concObj, pointer := concreteType(p, ti) + if concObj == nil || concObj.Obj().Pkg() == nil { + return nil + } + tv, ok := ti.Types[ce.Fun] + if !ok { + return nil + } + sig, ok := tv.Type.(*types.Signature) + if !ok { + return nil + } + sigVar := sig.Params().At(paramIdx) + iface := ifaceObjFromType(sigVar.Type()) + if iface == nil { + return nil + } + return &StubInfo{ + Fset: fset, + Concrete: concObj, + Pointer: pointer, + Interface: iface, + } +} + +// 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(fset *token.FileSet, 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{ + Fset: fset, + Concrete: concObj, + Pointer: pointer, + Interface: iface, + }, nil +} + +// fromValueSpec returns *StubInfo from a variable declaration such as +// var x io.Writer = &T{} +func fromValueSpec(fset *token.FileSet, 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{ + Fset: fset, + Concrete: concObj, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// fromAssignStmt returns *StubInfo from a variable re-assignment such as +// var x io.Writer +// x = &T{} +func fromAssignStmt(fset *token.FileSet, 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{ + Fset: fset, + Concrete: concType, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// RelativeToFiles returns a types.Qualifier that formats package +// names according to the import environments of the files that define +// the concrete type and the interface type. (Only the imports of the +// latter file are provided.) +// +// 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 importEnv in case +// an import is missing from concFile but is present among importEnv. +// +// 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. +// +// TODO(rfindley): investigate if this can be merged with source.Qualifier. +func RelativeToFiles(concPkg *types.Package, concFile *ast.File, ifaceImports []*ast.ImportSpec, 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 == "_") + // TODO(adonovan): this comparison disregards a vendor prefix in 'other'. + 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 + for _, imp := range ifaceImports { + impPath, _ := strconv.Unquote(imp.Path.Value) + isIgnored := imp.Name != nil && (imp.Name.Name == "." || imp.Name.Name == "_") + // TODO(adonovan): this comparison disregards a vendor prefix in 'other'. + 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.TypeName { + tv, ok := ti.Types[n] + if !ok { + return nil + } + return ifaceObjFromType(tv.Type) +} + +func ifaceObjFromType(t types.Type) *types.TypeName { + named, ok := t.(*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/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go new file mode 100644 index 000000000..c5d8a2d78 --- /dev/null +++ b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go @@ -0,0 +1,28 @@ +// 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 undeclared + +func x() int { + var z int + z = y // want "(undeclared name|undefined): y" + + if z == m { // want "(undeclared name|undefined): m" + z = 1 + } + + if z == 1 { + z = 1 + } else if z == n+1 { // want "(undeclared name|undefined): n" + z = 1 + } + + switch z { + case 10: + z = 1 + case a: // want "(undeclared name|undefined): a" + z = 1 + } + return z +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go new file mode 100644 index 000000000..76c7ba685 --- /dev/null +++ b/gopls/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|undefined): undefinedChannels" +} + +func c() (<-chan string, chan string) { + return make(<-chan string), make(chan string) +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go new file mode 100644 index 000000000..73beace10 --- /dev/null +++ b/gopls/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|undefined): undefinedConsecutiveParams" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go new file mode 100644 index 000000000..5de925411 --- /dev/null +++ b/gopls/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|undefined): undefinedErrorParam" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go new file mode 100644 index 000000000..c62174ec9 --- /dev/null +++ b/gopls/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|undefined): undefinedLiterals" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go new file mode 100644 index 000000000..9396da4bd --- /dev/null +++ b/gopls/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|undefined): undefinedOperation" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go new file mode 100644 index 000000000..a4ed290d4 --- /dev/null +++ b/gopls/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|undefined): undefinedSelector" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go new file mode 100644 index 000000000..5cde299ad --- /dev/null +++ b/gopls/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|undefined): undefinedSlice" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go new file mode 100644 index 000000000..9e91c59c2 --- /dev/null +++ b/gopls/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|undefined): undefinedTuple" +} + +func b() (string, error) { + return "", nil +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go new file mode 100644 index 000000000..5b4241425 --- /dev/null +++ b/gopls/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|undefined): undefinedUniqueArguments" +} diff --git a/gopls/internal/lsp/analysis/undeclaredname/undeclared.go b/gopls/internal/lsp/analysis/undeclaredname/undeclared.go new file mode 100644 index 000000000..043979408 --- /dev/null +++ b/gopls/internal/lsp/analysis/undeclaredname/undeclared.go @@ -0,0 +1,347 @@ +// 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 undeclaredname defines an Analyzer that applies suggested fixes +// to errors of the type "undeclared name: %s". +package undeclaredname + +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" + "golang.org/x/tools/gopls/internal/lsp/safetoken" + "golang.org/x/tools/internal/analysisinternal" +) + +const Doc = `suggested fixes for "undeclared name: <>" + +This checker provides suggested fixes for type errors of the +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: "undeclaredname", + Doc: Doc, + Requires: []*analysis.Analyzer{}, + Run: run, + RunDespiteErrors: true, +} + +// The prefix for this error message changed in Go 1.20. +var undeclaredNamePrefixes = []string{"undeclared name: ", "undefined: "} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, err := range pass.TypeErrors { + runForError(pass, err) + } + return nil, nil +} + +func runForError(pass *analysis.Pass, err types.Error) { + var name string + for _, prefix := range undeclaredNamePrefixes { + if !strings.HasPrefix(err.Msg, prefix) { + continue + } + name = strings.TrimPrefix(err.Msg, prefix) + } + if name == "" { + return + } + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= err.Pos && err.Pos < f.End() { + file = f + break + } + } + 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 + } + } + 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 := safetoken.StartPosition(pass.Fset, err.Pos).Offset + end := tok.Pos(offset + len(name)) // TODO(adonovan): dubious! err.Pos + len(name)?? + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: end, + Message: err.Msg, + }) +} + +func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, file *ast.File, pkg *types.Package, info *types.Info) (*analysis.SuggestedFix, error) { + pos := start // don't use the end + path, _ := astutil.PathEnclosingInterval(file, pos, pos) + if len(path) < 2 { + return nil, fmt.Errorf("no expression found") + } + ident, ok := path[0].(*ast.Ident) + if !ok { + 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("could not locate insertion point") + } + + insertBefore := safetoken.StartPosition(fset, insertBeforeStmt.Pos()).Offset + + // Get the indent to add on the line after the new statement. + // Since this will have a parse error, we can not use format.Source(). + contentBeforeStmt, indent := content[:insertBefore], "\n" + 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{ + Message: fmt.Sprintf("Create variable \"%s\"", ident.Name), + TextEdits: []analysis.TextEdit{{ + Pos: insertBeforeStmt.Pos(), + End: insertBeforeStmt.Pos(), + NewText: []byte(newStmt), + }}, + }, nil +} + +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(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/gopls/internal/lsp/analysis/undeclaredname/undeclared_test.go b/gopls/internal/lsp/analysis/undeclaredname/undeclared_test.go new file mode 100644 index 000000000..306c3f039 --- /dev/null +++ b/gopls/internal/lsp/analysis/undeclaredname/undeclared_test.go @@ -0,0 +1,17 @@ +// 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 undeclaredname_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/undeclaredname" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, undeclaredname.Analyzer, "a") +} diff --git a/gopls/internal/lsp/analysis/unusedparams/testdata/src/a/a.go b/gopls/internal/lsp/analysis/unusedparams/testdata/src/a/a.go new file mode 100644 index 000000000..23e4122c4 --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedparams/testdata/src/a/a.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 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, i2 int, 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(c bytes.Buffer) { // want "potentially unused parameter: 'c'" + _ = 1 +} + +func z(h 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, b int) int { // want "potentially unused parameter: 'b'" + a += 1 + return a +} + +func y(a int) { + panic("yo") +} diff --git a/gopls/internal/lsp/analysis/unusedparams/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/unusedparams/testdata/src/a/a.go.golden new file mode 100644 index 000000000..e28a6bdea --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go b/gopls/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go new file mode 100644 index 000000000..93af2681b --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go.golden b/gopls/internal/lsp/analysis/unusedparams/testdata/src/typeparams/typeparams.go.golden new file mode 100644 index 000000000..c86bf289a --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/unusedparams/unusedparams.go b/gopls/internal/lsp/analysis/unusedparams/unusedparams.go new file mode 100644 index 000000000..4c933c8fb --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedparams/unusedparams.go @@ -0,0 +1,152 @@ +// 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 unusedparams defines an analyzer that checks for unused +// parameters of functions. +package unusedparams + +import ( + "fmt" + "go/ast" + "go/types" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `check for unused parameters of functions + +The unusedparams analyzer checks functions to see if there are +any parameters that are not being used. + +To reduce false positives it ignores: +- methods +- parameters that do not have a name or are underscored +- functions in test files +- functions with empty bodies or those with just a return stmt` + +var Analyzer = &analysis.Analyzer{ + Name: "unusedparams", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +type paramData struct { + field *ast.Field + ident *ast.Ident + typObj types.Object +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + (*ast.FuncLit)(nil), + } + + inspect.Preorder(nodeFilter, func(n ast.Node) { + var fieldList *ast.FieldList + var body *ast.BlockStmt + + // Get the fieldList and body from the function node. + switch f := n.(type) { + case *ast.FuncDecl: + fieldList, body = f.Type.Params, f.Body + // TODO(golang/go#36602): add better handling for methods, if we enable methods + // we will get false positives if a struct is potentially implementing + // an interface. + if f.Recv != nil { + return + } + // Ignore functions in _test.go files to reduce false positives. + if file := pass.Fset.File(n.Pos()); file != nil && strings.HasSuffix(file.Name(), "_test.go") { + return + } + case *ast.FuncLit: + fieldList, body = f.Type.Params, f.Body + } + // If there are no arguments or the function is empty, then return. + if fieldList.NumFields() == 0 || body == nil || len(body.List) == 0 { + return + } + + switch expr := body.List[0].(type) { + case *ast.ReturnStmt: + // Ignore functions that only contain a return statement to reduce false positives. + return + case *ast.ExprStmt: + callExpr, ok := expr.X.(*ast.CallExpr) + if !ok || len(body.List) > 1 { + break + } + // Ignore functions that only contain a panic statement to reduce false positives. + if fun, ok := callExpr.Fun.(*ast.Ident); ok && fun.Name == "panic" { + return + } + } + + // Get the useful data from each field. + params := make(map[string]*paramData) + unused := make(map[*paramData]bool) + for _, f := range fieldList.List { + for _, i := range f.Names { + if i.Name == "_" { + continue + } + params[i.Name] = ¶mData{ + field: f, + ident: i, + typObj: pass.TypesInfo.ObjectOf(i), + } + unused[params[i.Name]] = true + } + } + + // Traverse through the body of the function and + // check to see which parameters are unused. + ast.Inspect(body, func(node ast.Node) bool { + n, ok := node.(*ast.Ident) + if !ok { + return true + } + param, ok := params[n.Name] + if !ok { + return false + } + if nObj := pass.TypesInfo.ObjectOf(n); nObj != param.typObj { + return false + } + delete(unused, param) + return false + }) + + // Create the reports for the unused parameters. + for u := range unused { + start, end := u.field.Pos(), u.field.End() + if len(u.field.Names) > 1 { + start, end = u.ident.Pos(), u.ident.End() + } + // TODO(golang/go#36602): Add suggested fixes to automatically + // 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("_"), + }}, + }}, + }) + } + }) + return nil, nil +} diff --git a/gopls/internal/lsp/analysis/unusedparams/unusedparams_test.go b/gopls/internal/lsp/analysis/unusedparams/unusedparams_test.go new file mode 100644 index 000000000..fdd43b821 --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedparams/unusedparams_test.go @@ -0,0 +1,22 @@ +// 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 unusedparams_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/unusedparams" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + tests := []string{"a"} + if typeparams.Enabled { + tests = append(tests, "typeparams") + } + analysistest.RunWithSuggestedFixes(t, testdata, unusedparams.Analyzer, tests...) +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go new file mode 100644 index 000000000..aa9f46e5b --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go @@ -0,0 +1,74 @@ +// 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 ( + "fmt" + "os" +) + +type A struct { + b int +} + +func singleAssignment() { + v := "s" // want `v declared (and|but) not used` + + s := []int{ // want `s declared (and|but) not used` + 1, + 2, + } + + a := func(s string) bool { // want `a declared (and|but) not used` + return false + } + + if 1 == 1 { + s := "v" // want `s declared (and|but) not used` + } + + panic("I should survive") +} + +func noOtherStmtsInBlock() { + v := "s" // want `v declared (and|but) not used` +} + +func partOfMultiAssignment() { + f, err := os.Open("file") // want `f declared (and|but) not used` + panic(err) +} + +func sideEffects(cBool chan bool, cInt chan int) { + b := <-c // want `b declared (and|but) not used` + s := fmt.Sprint("") // want `s declared (and|but) not used` + a := A{ // want `a declared (and|but) not used` + b: func() int { + return 1 + }(), + } + c := A{<-cInt} // want `c declared (and|but) not used` + d := fInt() + <-cInt // want `d declared (and|but) not used` + e := fBool() && <-cBool // want `e declared (and|but) not used` + f := map[int]int{ // want `f declared (and|but) not used` + fInt(): <-cInt, + } + g := []int{<-cInt} // want `g declared (and|but) not used` + h := func(s string) {} // want `h declared (and|but) not used` + i := func(s string) {}() // want `i declared (and|but) not used` +} + +func commentAbove() { + // v is a variable + v := "s" // want `v declared (and|but) not used` +} + +func fBool() bool { + return true +} + +func fInt() int { + return 1 +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go.golden b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go.golden new file mode 100644 index 000000000..18173ce0b --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go.golden @@ -0,0 +1,59 @@ +// 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 ( + "fmt" + "os" +) + +type A struct { + b int +} + +func singleAssignment() { + if 1 == 1 { + } + + panic("I should survive") +} + +func noOtherStmtsInBlock() { +} + +func partOfMultiAssignment() { + _, err := os.Open("file") // want `f declared (and|but) not used` + panic(err) +} + +func sideEffects(cBool chan bool, cInt chan int) { + <-c // want `b declared (and|but) not used` + fmt.Sprint("") // want `s declared (and|but) not used` + A{ // want `a declared (and|but) not used` + b: func() int { + return 1 + }(), + } + A{<-cInt} // want `c declared (and|but) not used` + fInt() + <-cInt // want `d declared (and|but) not used` + fBool() && <-cBool // want `e declared (and|but) not used` + map[int]int{ // want `f declared (and|but) not used` + fInt(): <-cInt, + } + []int{<-cInt} // want `g declared (and|but) not used` + func(s string) {}() // want `i declared (and|but) not used` +} + +func commentAbove() { + // v is a variable +} + +func fBool() bool { + return true +} + +func fInt() int { + return 1 +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go new file mode 100644 index 000000000..8e843024a --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go @@ -0,0 +1,30 @@ +// 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 decl + +func a() { + var b, c bool // want `b declared (and|but) not used` + panic(c) + + if 1 == 1 { + var s string // want `s declared (and|but) not used` + } +} + +func b() { + // b is a variable + var b bool // want `b declared (and|but) not used` +} + +func c() { + var ( + d string + + // some comment for c + c bool // want `c declared (and|but) not used` + ) + + panic(d) +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go.golden b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go.golden new file mode 100644 index 000000000..6ed97332e --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/testdata/src/decl/a.go.golden @@ -0,0 +1,24 @@ +// 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 decl + +func a() { + var c bool // want `b declared (and|but) not used` + panic(c) + + if 1 == 1 { + } +} + +func b() { + // b is a variable +} + +func c() { + var ( + d string + ) + panic(d) +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go new file mode 100644 index 000000000..904016be7 --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go @@ -0,0 +1,300 @@ +// 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 unusedvariable defines an analyzer that checks for unused variables. +package unusedvariable + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/token" + "go/types" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/ast/astutil" +) + +const Doc = `check for unused variables + +The unusedvariable analyzer suggests fixes for unused variables errors. +` + +var Analyzer = &analysis.Analyzer{ + Name: "unusedvariable", + Doc: Doc, + Requires: []*analysis.Analyzer{}, + Run: run, + RunDespiteErrors: true, // an unusedvariable diagnostic is a compile error +} + +// The suffix for this error message changed in Go 1.20. +var unusedVariableSuffixes = []string{" declared and not used", " declared but not used"} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, typeErr := range pass.TypeErrors { + for _, suffix := range unusedVariableSuffixes { + if strings.HasSuffix(typeErr.Msg, suffix) { + varName := strings.TrimSuffix(typeErr.Msg, suffix) + err := runForError(pass, typeErr, varName) + if err != nil { + return nil, err + } + } + } + } + + return nil, nil +} + +func runForError(pass *analysis.Pass, err types.Error, name string) error { + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= err.Pos && err.Pos < f.End() { + file = f + break + } + } + if file == nil { + return nil + } + + path, _ := astutil.PathEnclosingInterval(file, err.Pos, err.Pos) + if len(path) < 2 { + return nil + } + + ident, ok := path[0].(*ast.Ident) + if !ok || ident.Name != name { + return nil + } + + diag := analysis.Diagnostic{ + Pos: ident.Pos(), + End: ident.End(), + Message: err.Msg, + } + + for i := range path { + switch stmt := path[i].(type) { + case *ast.ValueSpec: + // Find GenDecl to which offending ValueSpec belongs. + if decl, ok := path[i+1].(*ast.GenDecl); ok { + fixes := removeVariableFromSpec(pass, path, stmt, decl, ident) + // fixes may be nil + if len(fixes) > 0 { + diag.SuggestedFixes = fixes + pass.Report(diag) + } + } + + case *ast.AssignStmt: + if stmt.Tok != token.DEFINE { + continue + } + + containsIdent := false + for _, expr := range stmt.Lhs { + if expr == ident { + containsIdent = true + } + } + if !containsIdent { + continue + } + + fixes := removeVariableFromAssignment(pass, path, stmt, ident) + // fixes may be nil + if len(fixes) > 0 { + diag.SuggestedFixes = fixes + pass.Report(diag) + } + } + } + + return nil +} + +func removeVariableFromSpec(pass *analysis.Pass, path []ast.Node, stmt *ast.ValueSpec, decl *ast.GenDecl, ident *ast.Ident) []analysis.SuggestedFix { + newDecl := new(ast.GenDecl) + *newDecl = *decl + newDecl.Specs = nil + + for _, spec := range decl.Specs { + if spec != stmt { + newDecl.Specs = append(newDecl.Specs, spec) + continue + } + + newSpec := new(ast.ValueSpec) + *newSpec = *stmt + newSpec.Names = nil + + for _, n := range stmt.Names { + if n != ident { + newSpec.Names = append(newSpec.Names, n) + } + } + + if len(newSpec.Names) > 0 { + newDecl.Specs = append(newDecl.Specs, newSpec) + } + } + + // decl.End() does not include any comments, so if a comment is present we + // need to account for it when we delete the statement + end := decl.End() + if stmt.Comment != nil && stmt.Comment.End() > end { + end = stmt.Comment.End() + } + + // There are no other specs left in the declaration, the whole statement can + // be deleted + if len(newDecl.Specs) == 0 { + // Find parent DeclStmt and delete it + for _, node := range path { + if declStmt, ok := node.(*ast.DeclStmt); ok { + return []analysis.SuggestedFix{ + { + Message: suggestedFixMessage(ident.Name), + TextEdits: deleteStmtFromBlock(path, declStmt), + }, + } + } + } + } + + var b bytes.Buffer + if err := format.Node(&b, pass.Fset, newDecl); err != nil { + return nil + } + + return []analysis.SuggestedFix{ + { + Message: suggestedFixMessage(ident.Name), + TextEdits: []analysis.TextEdit{ + { + Pos: decl.Pos(), + // Avoid adding a new empty line + End: end + 1, + NewText: b.Bytes(), + }, + }, + }, + } +} + +func removeVariableFromAssignment(pass *analysis.Pass, path []ast.Node, stmt *ast.AssignStmt, ident *ast.Ident) []analysis.SuggestedFix { + // The only variable in the assignment is unused + if len(stmt.Lhs) == 1 { + // If LHS has only one expression to be valid it has to have 1 expression + // on RHS + // + // RHS may have side effects, preserve RHS + if exprMayHaveSideEffects(stmt.Rhs[0]) { + // Delete until RHS + return []analysis.SuggestedFix{ + { + Message: suggestedFixMessage(ident.Name), + TextEdits: []analysis.TextEdit{ + { + Pos: ident.Pos(), + End: stmt.Rhs[0].Pos(), + }, + }, + }, + } + } + + // RHS does not have any side effects, delete the whole statement + return []analysis.SuggestedFix{ + { + Message: suggestedFixMessage(ident.Name), + TextEdits: deleteStmtFromBlock(path, stmt), + }, + } + } + + // Otherwise replace ident with `_` + return []analysis.SuggestedFix{ + { + Message: suggestedFixMessage(ident.Name), + TextEdits: []analysis.TextEdit{ + { + Pos: ident.Pos(), + End: ident.End(), + NewText: []byte("_"), + }, + }, + }, + } +} + +func suggestedFixMessage(name string) string { + return fmt.Sprintf("Remove variable %s", name) +} + +func deleteStmtFromBlock(path []ast.Node, stmt ast.Stmt) []analysis.TextEdit { + // Find innermost enclosing BlockStmt. + var block *ast.BlockStmt + for i := range path { + if blockStmt, ok := path[i].(*ast.BlockStmt); ok { + block = blockStmt + break + } + } + + nodeIndex := -1 + for i, blockStmt := range block.List { + if blockStmt == stmt { + nodeIndex = i + break + } + } + + // The statement we need to delete was not found in BlockStmt + if nodeIndex == -1 { + return nil + } + + // Delete until the end of the block unless there is another statement after + // the one we are trying to delete + end := block.Rbrace + if nodeIndex < len(block.List)-1 { + end = block.List[nodeIndex+1].Pos() + } + + return []analysis.TextEdit{ + { + Pos: stmt.Pos(), + End: end, + }, + } +} + +// exprMayHaveSideEffects reports whether the expression may have side effects +// (because it contains a function call or channel receive). We disregard +// runtime panics as well written programs should not encounter them. +func exprMayHaveSideEffects(expr ast.Expr) bool { + var mayHaveSideEffects bool + ast.Inspect(expr, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.CallExpr: // possible function call + mayHaveSideEffects = true + return false + case *ast.UnaryExpr: + if n.Op == token.ARROW { // channel receive + mayHaveSideEffects = true + return false + } + case *ast.FuncLit: + return false // evaluating what's inside a FuncLit has no effect + } + return true + }) + + return mayHaveSideEffects +} diff --git a/gopls/internal/lsp/analysis/unusedvariable/unusedvariable_test.go b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable_test.go new file mode 100644 index 000000000..08223155f --- /dev/null +++ b/gopls/internal/lsp/analysis/unusedvariable/unusedvariable_test.go @@ -0,0 +1,24 @@ +// 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 unusedvariable_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/gopls/internal/lsp/analysis/unusedvariable" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + + t.Run("decl", func(t *testing.T) { + analysistest.RunWithSuggestedFixes(t, testdata, unusedvariable.Analyzer, "decl") + }) + + t.Run("assign", func(t *testing.T) { + analysistest.RunWithSuggestedFixes(t, testdata, unusedvariable.Analyzer, "assign") + }) +} diff --git a/gopls/internal/lsp/analysis/useany/testdata/src/a/a.go b/gopls/internal/lsp/analysis/useany/testdata/src/a/a.go new file mode 100644 index 000000000..22d693150 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/useany/testdata/src/a/a.go.golden b/gopls/internal/lsp/analysis/useany/testdata/src/a/a.go.golden new file mode 100644 index 000000000..efd8fd640 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/useany/useany.go b/gopls/internal/lsp/analysis/useany/useany.go new file mode 100644 index 000000000..73e2f7633 --- /dev/null +++ b/gopls/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/gopls/internal/lsp/analysis/useany/useany_test.go b/gopls/internal/lsp/analysis/useany/useany_test.go new file mode 100644 index 000000000..083c3d54f --- /dev/null +++ b/gopls/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/gopls/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") +} |