diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 8447244672..aa27c1b9df 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -196,12 +196,13 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns want := string(bytes.TrimRight(vf.Data, "\n")) + "\n" formatted, err := format.Source([]byte(out)) if err != nil { + t.Errorf("%s: error formatting edited source: %v\n%s", file.Name(), err, out) continue } if want != string(formatted) { d, err := myers.ComputeEdits("", want, string(formatted)) if err != nil { - t.Errorf("failed to compute suggested fixes: %v", err) + t.Errorf("failed to compute suggested fix diff: %v", err) } t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), diff.ToUnified(fmt.Sprintf("%s.golden [%s]", file.Name(), sf), "actual", want, d)) } @@ -225,12 +226,13 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns formatted, err := format.Source([]byte(out)) if err != nil { + t.Errorf("%s: error formatting resulting source: %v\n%s", file.Name(), err, out) continue } if want != string(formatted) { d, err := myers.ComputeEdits("", want, string(formatted)) if err != nil { - t.Errorf("failed to compute edits: %s", err) + t.Errorf("%s: failed to compute suggested fix diff: %s", file.Name(), err) } t.Errorf("suggested fixes failed for %s:\n%s", file.Name(), diff.ToUnified(file.Name()+".golden", "actual", want, d)) } diff --git a/go/packages/packages.go b/go/packages/packages.go index 8a1a2d6810..d545b13758 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -26,6 +26,7 @@ import ( "golang.org/x/tools/go/gcexportdata" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/packagesinternal" + "golang.org/x/tools/internal/typeparams" "golang.org/x/tools/internal/typesinternal" ) @@ -910,6 +911,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { Scopes: make(map[ast.Node]*types.Scope), Selections: make(map[*ast.SelectorExpr]*types.Selection), } + typeparams.InitInstanceInfo(lpkg.TypesInfo) lpkg.TypesSizes = ld.sizes importer := importerFunc(func(path string) (*types.Package, error) { diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index cc344b8234..3a5ae5fd21 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -182,6 +182,22 @@ The Read method in v has a different signature than the Read method in io.Reader, so this assertion cannot succeed. +**Enabled by default.** + +## **infertypeargs** + +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 +} + + **Enabled by default.** ## **loopclosure** diff --git a/internal/lsp/analysis/infertypeargs/infertypeargs.go b/internal/lsp/analysis/infertypeargs/infertypeargs.go new file mode 100644 index 0000000000..34e6e72235 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/infertypeargs.go @@ -0,0 +1,31 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package infertypeargs defines an analyzer that checks for explicit function +// arguments that could be inferred. +package infertypeargs + +import ( + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" +) + +const Doc = `check for unnecessary type arguments in call expressions + +Explicit type arguments may be omitted from call expressions if they can be +inferred from function arguments, or from other type arguments: + +func f[T any](T) {} + +func _() { + f[string]("foo") // string could be inferred +} +` + +var Analyzer = &analysis.Analyzer{ + Name: "infertypeargs", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} diff --git a/internal/lsp/analysis/infertypeargs/infertypeargs_test.go b/internal/lsp/analysis/infertypeargs/infertypeargs_test.go new file mode 100644 index 0000000000..2957f46e36 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/infertypeargs_test.go @@ -0,0 +1,23 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package infertypeargs_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/infertypeargs" + "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + testenv.NeedsGo1Point(t, 13) + if !typeparams.Enabled { + t.Skip("type params are not enabled") + } + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, infertypeargs.Analyzer, "a") +} diff --git a/internal/lsp/analysis/infertypeargs/run_go117.go b/internal/lsp/analysis/infertypeargs/run_go117.go new file mode 100644 index 0000000000..bc5c29b51d --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/run_go117.go @@ -0,0 +1,16 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !go1.18 +// +build !go1.18 + +package infertypeargs + +import "golang.org/x/tools/go/analysis" + +// This analyzer only relates to go1.18+, and uses the types.CheckExpr API that +// was added in Go 1.13. +func run(pass *analysis.Pass) (interface{}, error) { + return nil, nil +} diff --git a/internal/lsp/analysis/infertypeargs/run_go118.go b/internal/lsp/analysis/infertypeargs/run_go118.go new file mode 100644 index 0000000000..96654c0011 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/run_go118.go @@ -0,0 +1,115 @@ +// 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) + ident, ix := instanceData(call) + if ix == nil || len(ix.Indices) == 0 { + return // no explicit args, nothing to do + } + + // Confirm that instantiation actually occurred at this ident. + _, instance := typeparams.GetInstance(pass.TypesInfo, ident) + if instance == nil { + return // something went wrong, but fail open + } + + // Start removing argument expressions from the right, and check if we can + // still infer the call expression. + required := len(ix.Indices) // number of type expressions that are required + for i := len(ix.Indices) - 1; i >= 0; i-- { + var fun ast.Expr + if i == 0 { + // No longer an index expression: just use the parameterized operand. + fun = ix.X + } else { + fun = typeparams.PackIndexExpr(ix.X, ix.Lbrack, ix.Indices[:i], ix.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 + } + _, newInstance := typeparams.GetInstance(info, ident) + 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(ix.Indices) { + var start, end token.Pos + if required == 0 { + start, end = ix.Lbrack, ix.Rbrack+1 // erase the entire index + } else { + start = ix.Indices[required-1].End() + end = ix.Rbrack + } + pass.Report(analysis.Diagnostic{ + Pos: start, + End: end, + Message: "unnecessary type arguments", + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "simplify type arguments", + TextEdits: []analysis.TextEdit{{ + Pos: start, + End: end, + }}, + }}, + }) + } + }) + + return nil, nil +} + +// instanceData returns the instantiated identifier and index data. +func instanceData(call *ast.CallExpr) (*ast.Ident, *typeparams.IndexExprData) { + ix := typeparams.GetIndexExprData(call.Fun) + if ix == nil { + return nil, nil + } + var id *ast.Ident + switch x := ix.X.(type) { + case *ast.SelectorExpr: + id = x.Sel + case *ast.Ident: + id = x + default: + return nil, nil + } + return id, ix +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go new file mode 100644 index 0000000000..1c3d88ba1a --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go @@ -0,0 +1,20 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the infertyepargs checker. + +package a + +func f[T any](T) {} + +func g[T any]() T { var x T; return x } + +func h[P interface{ ~*T }, T any]() {} + +func _() { + f[string]("hello") // want "unnecessary type arguments" + f[int](2) // want "unnecessary type arguments" + _ = g[int]() + h[*int, int]() // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden new file mode 100644 index 0000000000..72348ff775 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/basic.go.golden @@ -0,0 +1,20 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the infertyepargs checker. + +package a + +func f[T any](T) {} + +func g[T any]() T { var x T; return x } + +func h[P interface{ ~*T }, T any]() {} + +func _() { + f("hello") // want "unnecessary type arguments" + f(2) // want "unnecessary type arguments" + _ = g[int]() + h[*int]() // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go new file mode 100644 index 0000000000..fc1f763df6 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go @@ -0,0 +1,12 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import "a/imported" + +func _() { + var x int + imported.F[int](x) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden new file mode 100644 index 0000000000..6099545bba --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported.go.golden @@ -0,0 +1,12 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import "a/imported" + +func _() { + var x int + imported.F(x) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go new file mode 100644 index 0000000000..f0610a8b4c --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/imported/imported.go @@ -0,0 +1,7 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package imported + +func F[T any](T) {} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go new file mode 100644 index 0000000000..c304f1d0d2 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go @@ -0,0 +1,26 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// We should not suggest removing type arguments if doing so would change the +// resulting type. + +package a + +func id[T any](t T) T { return t } + +var _ = id[int](1) // want "unnecessary type arguments" +var _ = id[string]("foo") // want "unnecessary type arguments" +var _ = id[int64](2) + +func pair[T any](t T) (T, T) { return t, t } + +var _, _ = pair[int](3) // want "unnecessary type arguments" +var _, _ = pair[int64](3) + +func noreturn[T any](t T) {} + +func _() { + noreturn[int64](4) + noreturn[int](4) // want "unnecessary type arguments" +} diff --git a/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden new file mode 100644 index 0000000000..93c6f707c3 --- /dev/null +++ b/internal/lsp/analysis/infertypeargs/testdata/src/a/notypechange.go.golden @@ -0,0 +1,26 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// We should not suggest removing type arguments if doing so would change the +// resulting type. + +package a + +func id[T any](t T) T { return t } + +var _ = id(1) // want "unnecessary type arguments" +var _ = id("foo") // want "unnecessary type arguments" +var _ = id[int64](2) + +func pair[T any](t T) (T, T) { return t, t } + +var _, _ = pair(3) // want "unnecessary type arguments" +var _, _ = pair[int64](3) + +func noreturn[T any](t T) {} + +func _() { + noreturn[int64](4) + noreturn(4) // want "unnecessary type arguments" +} diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 59eb437fa0..0c328071ad 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -441,6 +441,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "detect impossible interface-to-interface type assertions\n\nThis checker flags type assertions v.(T) and corresponding type-switch cases\nin which the static type V of v is an interface that cannot possibly implement\nthe target interface T. This occurs when V and T contain methods with the same\nname but different signatures. Example:\n\n\tvar v interface {\n\t\tRead()\n\t}\n\t_ = v.(io.Reader)\n\nThe Read method in v has a different signature than the Read method in\nio.Reader, so this assertion cannot succeed.\n", Default: "true", }, + { + Name: "\"infertypeargs\"", + Doc: "check for unnecessary type arguments in call expressions\n\nExplicit type arguments may be omitted from call expressions if they can be\ninferred from function arguments, or from other type arguments:\n\nfunc f[T any](T) {}\n\nfunc _() {\n\tf[string](\"foo\") // string could be inferred\n}\n", + Default: "true", + }, { Name: "\"loopclosure\"", Doc: "check references to loop variables from within nested functions\n\nThis analyzer checks for references to loop variables from within a\nfunction literal inside the loop body. It checks only instances where\nthe function literal is called in a defer or go statement that is the\nlast statement in the loop body, as otherwise we would need whole\nprogram analysis.\n\nFor example:\n\n\tfor i, v := range s {\n\t\tgo func() {\n\t\t\tprintln(i, v) // not what you might expect\n\t\t}()\n\t}\n\nSee: https://golang.org/doc/go_faq.html#closures_and_goroutines", @@ -1014,6 +1019,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "detect impossible interface-to-interface type assertions\n\nThis checker flags type assertions v.(T) and corresponding type-switch cases\nin which the static type V of v is an interface that cannot possibly implement\nthe target interface T. This occurs when V and T contain methods with the same\nname but different signatures. Example:\n\n\tvar v interface {\n\t\tRead()\n\t}\n\t_ = v.(io.Reader)\n\nThe Read method in v has a different signature than the Read method in\nio.Reader, so this assertion cannot succeed.\n", Default: true, }, + { + Name: "infertypeargs", + Doc: "check for unnecessary type arguments in call expressions\n\nExplicit type arguments may be omitted from call expressions if they can be\ninferred from function arguments, or from other type arguments:\n\nfunc f[T any](T) {}\n\nfunc _() {\n\tf[string](\"foo\") // string could be inferred\n}\n", + Default: true, + }, { Name: "loopclosure", Doc: "check references to loop variables from within nested functions\n\nThis analyzer checks for references to loop variables from within a\nfunction literal inside the loop body. It checks only instances where\nthe function literal is called in a defer or go statement that is the\nlast statement in the loop body, as otherwise we would need whole\nprogram analysis.\n\nFor example:\n\n\tfor i, v := range s {\n\t\tgo func() {\n\t\t\tprintln(i, v) // not what you might expect\n\t\t}()\n\t}\n\nSee: https://golang.org/doc/go_faq.html#closures_and_goroutines", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 374fb7063d..cb4b11d965 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -49,6 +49,7 @@ import ( "golang.org/x/tools/go/analysis/passes/unusedwrite" "golang.org/x/tools/internal/lsp/analysis/fillreturns" "golang.org/x/tools/internal/lsp/analysis/fillstruct" + "golang.org/x/tools/internal/lsp/analysis/infertypeargs" "golang.org/x/tools/internal/lsp/analysis/nonewvars" "golang.org/x/tools/internal/lsp/analysis/noresultvalues" "golang.org/x/tools/internal/lsp/analysis/simplifycompositelit" @@ -1238,6 +1239,7 @@ func defaultAnalyzers() map[string]*Analyzer { unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false}, unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: false}, useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: true}, + infertypeargs.Analyzer.Name: {Analyzer: infertypeargs.Analyzer, Enabled: true}, // gofmt -s suite: simplifycompositelit.Analyzer.Name: { diff --git a/internal/typeparams/typeparams_go117.go b/internal/typeparams/typeparams_go117.go index 479b5561d7..ae153ab201 100644 --- a/internal/typeparams/typeparams_go117.go +++ b/internal/typeparams/typeparams_go117.go @@ -9,6 +9,7 @@ package typeparams import ( "go/ast" + "go/token" "go/types" ) @@ -30,6 +31,24 @@ func GetIndexExprData(n ast.Node) *IndexExprData { return nil } +// PackIndexExpr returns an *ast.IndexExpr with the given index. +// Calling PackIndexExpr with len(indices) != 1 will panic. +func PackIndexExpr(x ast.Expr, lbrack token.Pos, indices []ast.Expr, rbrack token.Pos) ast.Expr { + switch len(indices) { + case 0: + panic("empty indices") + case 1: + return &ast.IndexExpr{ + X: x, + Lbrack: lbrack, + Index: indices[0], + Rbrack: rbrack, + } + default: + panic("cannot pack multiple indices at this go version") + } +} + // ForTypeSpec returns an empty field list, as type parameters on not supported // at this Go version. func ForTypeSpec(*ast.TypeSpec) *ast.FieldList { diff --git a/internal/typeparams/typeparams_go118.go b/internal/typeparams/typeparams_go118.go index 16c6c0d157..aca937a2a2 100644 --- a/internal/typeparams/typeparams_go118.go +++ b/internal/typeparams/typeparams_go118.go @@ -9,6 +9,7 @@ package typeparams import ( "go/ast" + "go/token" "go/types" ) @@ -36,6 +37,30 @@ func GetIndexExprData(n ast.Node) *IndexExprData { return nil } +// PackIndexExpr returns an *ast.IndexExpr or *ast.IndexListExpr, depending on +// the cardinality of indices. Calling PackIndexExpr with len(indices) == 0 +// will panic. +func PackIndexExpr(x ast.Expr, lbrack token.Pos, indices []ast.Expr, rbrack token.Pos) ast.Expr { + switch len(indices) { + case 0: + panic("empty indices") + case 1: + return &ast.IndexExpr{ + X: x, + Lbrack: lbrack, + Index: indices[0], + Rbrack: rbrack, + } + default: + return &ast.IndexListExpr{ + X: x, + Lbrack: lbrack, + Indices: indices, + Rbrack: rbrack, + } + } +} + // ForTypeSpec returns n.TypeParams. func ForTypeSpec(n *ast.TypeSpec) *ast.FieldList { if n == nil {