internal/lsp/analysis: quick-fix to remove unnecessary type arguments
This CL adds a new infertypeargs analyzer, which finds call exprs where
type arguments could be inferred, and suggests a quick fix to simplify
them.
Along the way, may two changes to the supporting frameworks:
- Initialized types.Info.Instances in go/packages
- Fail analysis tests run with suggested fixes if formatting the
resulting source fails.
Change-Id: Ib15e5bd7c26aa293c5fc18a4cff6bc047e9e31d2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351552
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index 8447244..aa27c1b 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -196,12 +196,13 @@
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 @@
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 8a1a2d6..d545b13 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -26,6 +26,7 @@
"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 @@
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 cc344b8..3a5ae5f 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -184,6 +184,22 @@
**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**
check references to loop variables from within nested functions
diff --git a/internal/lsp/analysis/infertypeargs/infertypeargs.go b/internal/lsp/analysis/infertypeargs/infertypeargs.go
new file mode 100644
index 0000000..34e6e72
--- /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 0000000..2957f46
--- /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 0000000..bc5c29b
--- /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 0000000..96654c0
--- /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 0000000..1c3d88b
--- /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 0000000..72348ff
--- /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 0000000..fc1f763
--- /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 0000000..6099545
--- /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 0000000..f0610a8
--- /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 0000000..c304f1d
--- /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 0000000..93c6f70
--- /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 59eb437..0c32807 100755
--- a/internal/lsp/source/api_json.go
+++ b/internal/lsp/source/api_json.go
@@ -442,6 +442,11 @@
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",
Default: "true",
@@ -1015,6 +1020,11 @@
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",
Default: true,
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 374fb70..cb4b11d 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -49,6 +49,7 @@
"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 @@
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 479b556..ae153ab 100644
--- a/internal/typeparams/typeparams_go117.go
+++ b/internal/typeparams/typeparams_go117.go
@@ -9,6 +9,7 @@
import (
"go/ast"
+ "go/token"
"go/types"
)
@@ -30,6 +31,24 @@
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 16c6c0d..aca937a 100644
--- a/internal/typeparams/typeparams_go118.go
+++ b/internal/typeparams/typeparams_go118.go
@@ -9,6 +9,7 @@
import (
"go/ast"
+ "go/token"
"go/types"
)
@@ -36,6 +37,30 @@
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 {
