internal/lsp: enable fillstruct for generics

This enables some fill struct code actions for instances of structs with
type parameters.

This additionally adds a filtering mechanism to the suggested fixes in
order to account for multiple suggested fixes in the same location.

Change-Id: I98866b462b026f4c5a4897bc278f704381623f25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/418415
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>

internal/lsp/analysis/fillstruct/fillstruct.go

@@ -68,14 +68,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 
-		// Ignore types that have type parameters for now.
-		// TODO: support type params.
-		if typ, ok := typ.(*types.Named); ok {
-			if tparams := typeparams.ForNamed(typ); tparams != nil && tparams.Len() > 0 {
-				return
-			}
-		}
-
 		// Find reference to the type declaration of the struct being initialized.
 		for {
 			p, ok := typ.Underlying().(*types.Pointer)

internal/lsp/analysis/fillstruct/testdata/src/typeparams/typeparams.go

@@ -12,18 +12,16 @@ type basicStruct[T any] struct {
 	foo T
 }
 
-var _ = basicStruct[int]{}
-
-type fooType[T any] T
+var _ = basicStruct[int]{} // want ""
 
 type twoArgStruct[F, B any] struct {
-	foo fooType[F]
-	bar fooType[B]
+	foo F
+	bar B
 }
 
-var _ = twoArgStruct[string, int]{}
+var _ = twoArgStruct[string, int]{} // want ""
 
-var _ = twoArgStruct[int, string]{
+var _ = twoArgStruct[int, string]{ // want ""
 	bar: "bar",
 }
 

internal/lsp/cmd/test/suggested_fix.go

@@ -12,14 +12,16 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
-func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string, expectedActions int) {
+func (r *runner) SuggestedFix(t *testing.T, spn span.Span, suggestedFixes []tests.SuggestedFix, expectedActions int) {
 	uri := spn.URI()
 	filename := uri.Filename()
 	args := []string{"fix", "-a", fmt.Sprintf("%s", spn)}
-	for _, kind := range actionKinds {
-		if kind == "refactor.rewrite" {
+	var actionKinds []string
+	for _, sf := range suggestedFixes {
+		if sf.ActionKind == "refactor.rewrite" {
 			t.Skip("refactor.rewrite is not yet supported on the command line")
 		}
+		actionKinds = append(actionKinds, sf.ActionKind)
 	}
 	args = append(args, actionKinds...)
 	got, stderr := r.NormalizeGoplsCmd(t, args...)

internal/lsp/lsp_test.go

@@ -487,7 +487,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
 	}
 }
 
-func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string, expectedActions int) {
+func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.SuggestedFix, expectedActions int) {
 	uri := spn.URI()
 	view, err := r.server.session.ViewOf(uri)
 	if err != nil {
@@ -516,9 +516,9 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string,
 	}
 	codeActionKinds := []protocol.CodeActionKind{}
 	for _, k := range actionKinds {
-		codeActionKinds = append(codeActionKinds, protocol.CodeActionKind(k))
+		codeActionKinds = append(codeActionKinds, protocol.CodeActionKind(k.ActionKind))
 	}
-	actions, err := r.server.CodeAction(r.ctx, &protocol.CodeActionParams{
+	allActions, err := r.server.CodeAction(r.ctx, &protocol.CodeActionParams{
 		TextDocument: protocol.TextDocumentIdentifier{
 			URI: protocol.URIFromSpanURI(uri),
 		},
@@ -531,6 +531,16 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string,
 	if err != nil {
 		t.Fatalf("CodeAction %s failed: %v", spn, err)
 	}
+	var actions []protocol.CodeAction
+	for _, action := range allActions {
+		for _, fix := range actionKinds {
+			if strings.Contains(action.Title, fix.Title) {
+				actions = append(actions, action)
+				break
+			}
+		}
+
+	}
 	if len(actions) != expectedActions {
 		// Hack: We assume that we only get one code action per range.
 		var cmds []string

internal/lsp/source/source_test.go

@@ -968,14 +968,12 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.Signa
 }
 
 // These are pure LSP features, no source level functionality to be tested.
-func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) {}
-
-func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string, expectedActions int) {
-}
-func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {}
-func (r *runner) MethodExtraction(t *testing.T, start span.Span, end span.Span)   {}
-func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens)   {}
-func (r *runner) AddImport(t *testing.T, uri span.URI, expectedImport string)     {}
+func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link)                          {}
+func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actions []tests.SuggestedFix, want int) {}
+func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span)                  {}
+func (r *runner) MethodExtraction(t *testing.T, start span.Span, end span.Span)                    {}
+func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens)                    {}
+func (r *runner) AddImport(t *testing.T, uri span.URI, expectedImport string)                      {}
 
 func spanToRange(data *tests.Data, spn span.Span) (*protocol.ColumnMapper, protocol.Range, error) {
 	m, err := data.Mapper(spn.URI())

internal/lsp/testdata/extract/extract_variable/extract_basic_lit.go

@@ -1,6 +1,6 @@
 package extract
 
 func _() {
-	var _ = 1 + 2 //@suggestedfix("1", "refactor.extract")
-	var _ = 3 + 4 //@suggestedfix("3 + 4", "refactor.extract")
+	var _ = 1 + 2 //@suggestedfix("1", "refactor.extract", "")
+	var _ = 3 + 4 //@suggestedfix("3 + 4", "refactor.extract", "")
 }

internal/lsp/testdata/extract/extract_variable/extract_basic_lit.go.golden

@@ -3,16 +3,16 @@ package extract
 
 func _() {
 	x := 1
-	var _ = x + 2 //@suggestedfix("1", "refactor.extract")
-	var _ = 3 + 4 //@suggestedfix("3 + 4", "refactor.extract")
+	var _ = x + 2 //@suggestedfix("1", "refactor.extract", "")
+	var _ = 3 + 4 //@suggestedfix("3 + 4", "refactor.extract", "")
 }
 
 -- suggestedfix_extract_basic_lit_5_10 --
 package extract
 
 func _() {
-	var _ = 1 + 2 //@suggestedfix("1", "refactor.extract")
+	var _ = 1 + 2 //@suggestedfix("1", "refactor.extract", "")
 	x := 3 + 4
-	var _ = x //@suggestedfix("3 + 4", "refactor.extract")
+	var _ = x //@suggestedfix("3 + 4", "refactor.extract", "")
 }
 

internal/lsp/testdata/extract/extract_variable/extract_func_call.go

@@ -3,7 +3,7 @@ package extract
 import "strconv"
 
 func _() {
-	x0 := append([]int{}, 1) //@suggestedfix("append([]int{}, 1)", "refactor.extract")
+	x0 := append([]int{}, 1) //@suggestedfix("append([]int{}, 1)", "refactor.extract", "")
 	str := "1"
-	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract")
+	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract", "")
 }

internal/lsp/testdata/extract/extract_variable/extract_func_call.go.golden

@@ -5,9 +5,9 @@ import "strconv"
 
 func _() {
 	x0 := append([]int{}, 1)
-	a := x0 //@suggestedfix("append([]int{}, 1)", "refactor.extract")
+	a := x0 //@suggestedfix("append([]int{}, 1)", "refactor.extract", "")
 	str := "1"
-	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract")
+	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract", "")
 }
 
 -- suggestedfix_extract_func_call_6_8 --
@@ -17,9 +17,9 @@ import "strconv"
 
 func _() {
 	x := append([]int{}, 1)
-	x0 := x //@suggestedfix("append([]int{}, 1)", "refactor.extract")
+	x0 := x //@suggestedfix("append([]int{}, 1)", "refactor.extract", "")
 	str := "1"
-	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract")
+	b, err := strconv.Atoi(str) //@suggestedfix("strconv.Atoi(str)", "refactor.extract", "")
 }
 
 -- suggestedfix_extract_func_call_8_12 --
@@ -28,9 +28,9 @@ package extract
 import "strconv"
 
 func _() {
-	x0 := append([]int{}, 1) //@suggestedfix("append([]int{}, 1)", "refactor.extract")
+	x0 := append([]int{}, 1) //@suggestedfix("append([]int{}, 1)", "refactor.extract", "")
 	str := "1"
 	x, x1 := strconv.Atoi(str)
-	b, err := x, x1 //@suggestedfix("strconv.Atoi(str)", "refactor.extract")
+	b, err := x, x1 //@suggestedfix("strconv.Atoi(str)", "refactor.extract", "")
 }
 

internal/lsp/testdata/extract/extract_variable/extract_scope.go

@@ -5,9 +5,9 @@ import "go/ast"
 func _() {
 	x0 := 0
 	if true {
-		y := ast.CompositeLit{} //@suggestedfix("ast.CompositeLit{}", "refactor.extract")
+		y := ast.CompositeLit{} //@suggestedfix("ast.CompositeLit{}", "refactor.extract", "")
 	}
 	if true {
-		x1 := !false //@suggestedfix("!false", "refactor.extract")
+		x1 := !false //@suggestedfix("!false", "refactor.extract", "")
 	}
 }

internal/lsp/testdata/extract/extract_variable/extract_scope.go.golden

@@ -6,11 +6,11 @@ import "go/ast"
 func _() {
 	x0 := 0
 	if true {
-		y := ast.CompositeLit{} //@suggestedfix("ast.CompositeLit{}", "refactor.extract")
+		y := ast.CompositeLit{} //@suggestedfix("ast.CompositeLit{}", "refactor.extract", "")
 	}
 	if true {
 		x := !false
-		x1 := x //@suggestedfix("!false", "refactor.extract")
+		x1 := x //@suggestedfix("!false", "refactor.extract", "")
 	}
 }
 
@@ -23,10 +23,10 @@ func _() {
 	x0 := 0
 	if true {
 		x := ast.CompositeLit{}
-		y := x //@suggestedfix("ast.CompositeLit{}", "refactor.extract")
+		y := x //@suggestedfix("ast.CompositeLit{}", "refactor.extract", "")
 	}
 	if true {
-		x1 := !false //@suggestedfix("!false", "refactor.extract")
+		x1 := !false //@suggestedfix("!false", "refactor.extract", "")
 	}
 }
 

internal/lsp/testdata/fillstruct/a.go

@@ -8,20 +8,20 @@ type basicStruct struct {
 	foo int
 }
 
-var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type twoArgStruct struct {
 	foo int
 	bar string
 }
 
-var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type nestedStruct struct {
 	bar   string
 	basic basicStruct
 }
 
-var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
-var _ = data.B{} //@suggestedfix("}", "refactor.rewrite")
+var _ = data.B{} //@suggestedfix("}", "refactor.rewrite", "Fill")

internal/lsp/testdata/fillstruct/a.go.golden

@@ -11,23 +11,23 @@ type basicStruct struct {
 
 var _ = basicStruct{
 	foo: 0,
-} //@suggestedfix("}", "refactor.rewrite")
+} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type twoArgStruct struct {
 	foo int
 	bar string
 }
 
-var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type nestedStruct struct {
 	bar   string
 	basic basicStruct
 }
 
-var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
-var _ = data.B{} //@suggestedfix("}", "refactor.rewrite")
+var _ = data.B{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 -- suggestedfix_a_18_22 --
 package fillstruct
@@ -40,7 +40,7 @@ type basicStruct struct {
 	foo int
 }
 
-var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type twoArgStruct struct {
 	foo int
@@ -50,16 +50,16 @@ type twoArgStruct struct {
 var _ = twoArgStruct{
 	foo: 0,
 	bar: "",
-} //@suggestedfix("}", "refactor.rewrite")
+} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type nestedStruct struct {
 	bar   string
 	basic basicStruct
 }
 
-var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
-var _ = data.B{} //@suggestedfix("}", "refactor.rewrite")
+var _ = data.B{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 -- suggestedfix_a_25_22 --
 package fillstruct
@@ -72,14 +72,14 @@ type basicStruct struct {
 	foo int
 }
 
-var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type twoArgStruct struct {
 	foo int
 	bar string
 }
 
-var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type nestedStruct struct {
 	bar   string
@@ -89,9 +89,9 @@ type nestedStruct struct {
 var _ = nestedStruct{
 	bar:   "",
 	basic: basicStruct{},
-} //@suggestedfix("}", "refactor.rewrite")
+} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
-var _ = data.B{} //@suggestedfix("}", "refactor.rewrite")
+var _ = data.B{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 -- suggestedfix_a_27_16 --
 package fillstruct
@@ -104,23 +104,23 @@ type basicStruct struct {
 	foo int
 }
 
-var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = basicStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type twoArgStruct struct {
 	foo int
 	bar string
 }
 
-var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = twoArgStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type nestedStruct struct {
 	bar   string
 	basic basicStruct
 }
 
-var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = nestedStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 var _ = data.B{
 	ExportedInt: 0,
-} //@suggestedfix("}", "refactor.rewrite")
+} //@suggestedfix("}", "refactor.rewrite", "Fill")
 

internal/lsp/testdata/fillstruct/a2.go

@@ -8,22 +8,22 @@ type typedStruct struct {
 	a  [2]string
 }
 
-var _ = typedStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = typedStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type funStruct struct {
 	fn func(i int) int
 }
 
-var _ = funStruct{} //@suggestedfix("}", "refactor.rewrite")
+var _ = funStruct{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type funStructCompex struct {
 	fn func(i int, s string) (string, int)
 }
 
-var _ = funStructCompex{} //@suggestedfix("}", "refactor.rewrite")
+var _ = funStructCompex{} //@suggestedfix("}", "refactor.rewrite", "Fill")
 
 type funStructEmpty struct {
 	fn func()
 }
 
-var _ = funStructEmpty{} //@suggestedfix("}", "refactor.rewrite")
+var _ = funStructEmpty{} //@suggestedfix("}", "refactor.rewrite", "Fill")