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>

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))
 				}

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) {

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**

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,
+}

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")
+}

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
+}

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
+}

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"
+}

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"
+}

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"
+}

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"
+}

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) {}

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"
+}

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"
+}