gopls/stub: support for method generation with same name but differen…

…t receiver

Fixes golang/go#64114

Change-Id: I5581bdbe1cbaa08e1e5589a304da5e637c6a5228
Reviewed-on: https://go-review.googlesource.com/c/tools/+/544915
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
Reviewed-by: Benny Siegert <[email protected]>

gopls/internal/analysis/stubmethods/stubmethods.go

@@ -66,7 +66,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 // MatchesMessage reports whether msg matches the error message sought after by
 // the stubmethods fix.
 func MatchesMessage(msg string) bool {
-	return strings.Contains(msg, "missing method") || strings.HasPrefix(msg, "cannot convert")
+	return strings.Contains(msg, "missing method") || strings.HasPrefix(msg, "cannot convert") || strings.Contains(msg, "not implement")
 }
 
 // DiagnosticForError computes a diagnostic suggesting to implement an

gopls/internal/analysis/stubmethods/stubmethods_test.go

@@ -0,0 +1,17 @@
+// Copyright 2023 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_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/gopls/internal/analysis/stubmethods"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, stubmethods.Analyzer, "a")
+}

gopls/internal/analysis/stubmethods/testdata/src/typeparams/implement.go

@@ -0,0 +1,15 @@
+// Copyright 2023 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
+
+var _ I = Y{} // want "Implement I"
+
+type I interface{ F() }
+
+type X struct{}
+
+func (X) F(string) {}
+
+type Y struct{ X }

gopls/internal/lsp/source/stub.go

@@ -75,14 +75,30 @@ func stubMethodsFixer(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.
 		importEnv[importPath] = name // latest alias wins
 	}
 
+	// Record all direct methods of the current object
+	concreteFuncs := make(map[string]struct{})
+	for i := 0; i < si.Concrete.NumMethods(); i++ {
+		concreteFuncs[si.Concrete.Method(i).Name()] = struct{}{}
+	}
+
 	// Find subset of interface methods that the concrete type lacks.
-	var missing []*types.Func
 	ifaceType := si.Interface.Type().Underlying().(*types.Interface)
+
+	type missingFn struct {
+		fn         *types.Func
+		needSubtle string
+	}
+
+	var (
+		missing                  []missingFn
+		concreteStruct, isStruct = si.Concrete.Origin().Underlying().(*types.Struct)
+	)
+
 	for i := 0; i < ifaceType.NumMethods(); i++ {
 		imethod := ifaceType.Method(i)
-		cmethod, _, _ := types.LookupFieldOrMethod(si.Concrete, si.Pointer, imethod.Pkg(), imethod.Name())
+		cmethod, index, _ := types.LookupFieldOrMethod(si.Concrete, si.Pointer, imethod.Pkg(), imethod.Name())
 		if cmethod == nil {
-			missing = append(missing, imethod)
+			missing = append(missing, missingFn{fn: imethod})
 			continue
 		}
 
@@ -92,10 +108,27 @@ func stubMethodsFixer(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.
 				conc.Name(), imethod.Name())
 		}
 
-		if !types.Identical(cmethod.Type(), imethod.Type()) {
-			return nil, nil, fmt.Errorf("method %s.%s already exists but has the wrong type: got %s, want %s",
-				conc.Name(), imethod.Name(), cmethod.Type(), imethod.Type())
+		if _, exist := concreteFuncs[imethod.Name()]; exist {
+			if !types.Identical(cmethod.Type(), imethod.Type()) {
+				return nil, nil, fmt.Errorf("method %s.%s already exists but has the wrong type: got %s, want %s",
+					conc.Name(), imethod.Name(), cmethod.Type(), imethod.Type())
+			}
+			continue
+		}
+
+		mf := missingFn{fn: imethod}
+		if isStruct && len(index) > 0 {
+			field := concreteStruct.Field(index[0])
+
+			fn := field.Name()
+			if _, ok := field.Type().(*types.Pointer); ok {
+				fn = "*" + fn
+			}
+
+			mf.needSubtle = fmt.Sprintf("// Subtle: this method shadows the method (%s).%s of %s.%s.\n", fn, imethod.Name(), si.Concrete.Obj().Name(), field.Name())
 		}
+
+		missing = append(missing, mf)
 	}
 	if len(missing) == 0 {
 		return nil, nil, fmt.Errorf("no missing methods found")
@@ -159,19 +192,20 @@ func stubMethodsFixer(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.
 
 	// Format the new methods.
 	var newMethods bytes.Buffer
-	for _, method := range missing {
+	for index := range missing {
 		fmt.Fprintf(&newMethods, `// %s implements %s.
-func (%s%s%s) %s%s {
+%sfunc (%s%s%s) %s%s {
 	panic("unimplemented")
 }
 `,
-			method.Name(),
+			missing[index].fn.Name(),
 			iface,
+			missing[index].needSubtle,
 			star,
 			si.Concrete.Obj().Name(),
 			FormatTypeParams(si.Concrete.TypeParams()),
-			method.Name(),
-			strings.TrimPrefix(types.TypeString(method.Type(), qual), "func"))
+			missing[index].fn.Name(),
+			strings.TrimPrefix(types.TypeString(missing[index].fn.Type(), qual), "func"))
 	}
 
 	// Compute insertion point for new methods:

gopls/internal/test/marker/testdata/stubmethods/issue64114.txt

@@ -0,0 +1,37 @@
+This test verifies that the embedded field has a method with the same name.
+
+-- issue64114.go --
+package stub
+
+// Regression test for issue #64114: code action "implement" is not listed.
+
+var _ WriteTest = (*WriteStruct)(nil) //@suggestedfix("(", re"does not implement", issue64114)
+
+type WriterTwoStruct struct{}
+
+// Write implements io.ReadWriter.
+func (t *WriterTwoStruct) RRRR(str string) error {
+	panic("unimplemented")
+}
+
+type WriteTest interface {
+	RRRR()
+	WWWW()
+}
+
+type WriteStruct struct {
+	WriterTwoStruct
+}
+-- @issue64114/issue64114.go --
+@@ -22 +22,11 @@
++
++// RRRR implements WriteTest.
++// Subtle: this method shadows the method (WriterTwoStruct).RRRR of WriteStruct.WriterTwoStruct.
++func (*WriteStruct) RRRR() {
++	panic("unimplemented")
++}
++
++// WWWW implements WriteTest.
++func (*WriteStruct) WWWW() {
++	panic("unimplemented")
++}