gopls/internal/lsp/source: show promoted methods in hover

This change updates the logic to display the method set of the
selected type so that:
- it includes promoted methods;
- it includes interface methods, if that would not be
  redundant with the syntax of the the type;
- receiver variables are properly named, and have a
  pointer type if appropriate.

Fixes golang/go#61634
Updates golang/go#56331

Change-Id: Ied9c06c36178424575012adb8fde0b1ddbd688c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/555456
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/internal/lsp/source/hover.go

@@ -244,23 +244,42 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
 			return protocol.Range{}, nil, err
 		}
 
-		// Display the declared methods accessible from the identifier.
-		//
-		// (The format.Node call above displays any struct fields, public
-		// or private, in syntactic form. We choose not to recursively
-		// enumerate any fields and methods promoted from them.)
-		if !types.IsInterface(obj.Type()) {
-			sep := "\n\n"
-			for _, m := range typeutil.IntuitiveMethodSet(obj.Type(), nil) {
-				// Show direct methods that are either exported, or defined in the
-				// current package.
-				if (m.Obj().Exported() || m.Obj().Pkg() == pkg.GetTypes()) && len(m.Index()) == 1 {
-					b.WriteString(sep)
-					sep = "\n"
-					b.WriteString(types.ObjectString(m.Obj(), qf))
+		// For an interface type, explicit methods will have
+		// already been displayed when the node was formatted
+		// above. Don't list these again.
+		var skip map[string]bool
+		if iface, ok := spec.Type.(*ast.InterfaceType); ok {
+			if iface.Methods.List != nil {
+				for _, m := range iface.Methods.List {
+					if len(m.Names) == 1 {
+						if skip == nil {
+							skip = make(map[string]bool)
+						}
+						skip[m.Names[0].Name] = true
+					}
 				}
 			}
 		}
+
+		// Display all the type's accessible methods,
+		// including those that require a pointer receiver,
+		// and those promoted from embedded struct fields or
+		// embedded interfaces.
+		sep := "\n\n"
+		for _, m := range typeutil.IntuitiveMethodSet(obj.Type(), nil) {
+			if m.Obj().Pkg() != pkg.GetTypes() && !m.Obj().Exported() {
+				continue // inaccessible
+			}
+			if skip[m.Obj().Name()] {
+				continue // redundant with format.Node above
+			}
+			b.WriteString(sep)
+			sep = "\n"
+
+			// Use objectString for its prettier rendering of method receivers.
+			b.WriteString(objectString(m.Obj(), qf, token.NoPos, nil, nil))
+		}
+
 		signature = b.String()
 	}
 

gopls/internal/test/marker/testdata/definition/embed.txt

@@ -208,6 +208,8 @@ type S2 struct {
 	F2   int    //@loc(S2F2, "F2")
 	*a.A        //@def("A", AString),def("a",AImport)
 }
+
+func (a.A) Hi()
 ```
 
 [`b.S2` on pkg.go.dev](https://pkg.go.dev/mod.com/b#S2)
@@ -242,7 +244,7 @@ field Field int
 ```go
 type A string
 
-func (a.A).Hi()
+func (a.A) Hi()
 ```
 
 @loc(AString, "A")
@@ -253,7 +255,7 @@ func (a.A).Hi()
 ```go
 type aAlias = a.A
 
-func (a.A).Hi()
+func (a.A) Hi()
 ```
 
 @loc(aAlias, "aAlias")

gopls/internal/test/marker/testdata/definition/misc.txt

@@ -175,6 +175,8 @@ type I interface {
 	B()
 	J
 }
+
+func (J) Hello()
 ```
 
 [`a.I` on pkg.go.dev](https://pkg.go.dev/mod.com#I)

gopls/internal/test/marker/testdata/hover/godef.txt

@@ -115,10 +115,10 @@ type Thing struct {
 	Member string //@loc(Member, "Member")
 }
 
-func (Thing).Method(i int) string
-func (*Thing).Method2(i int, j int) (error, string)
-func (Thing).Method3()
-func (*Thing).private()
+func (t Thing) Method(i int) string
+func (t *Thing) Method2(i int, j int) (error, string)
+func (t Thing) Method3()
+func (t *Thing) private()
 ```
 
 [`a.Thing` on pkg.go.dev](https://pkg.go.dev/godef.test/a#Thing)
@@ -129,8 +129,11 @@ type NextThing struct {
 	Value int
 }
 
-func (*NextThing).Method3() int
-func (NextThing).another() string
+func (t Thing) Method(i int) string
+func (t *Thing) Method2(i int, j int) (error, string)
+func (n *NextThing) Method3() int
+func (n NextThing) another() string
+func (t *Thing) private()
 ```
 
 [`a.NextThing` on pkg.go.dev](https://pkg.go.dev/godef.test/a#NextThing)

gopls/internal/test/marker/testdata/hover/linkable.txt

@@ -104,8 +104,8 @@ type T struct {
 	}
 }
 
-func (T).M()
-func (T).m()
+func (T) M()
+func (T) m()
 ```
 
 T is in the package scope, and so should be linkable.

gopls/internal/test/marker/testdata/hover/linkable_generics.txt

@@ -67,7 +67,7 @@ type GT[P any] struct {
 	F P //@hover("F", "F", F),hover("P", "P", FP)
 }
 
-func (GT[P]).M(p P)
+func (GT[P]) M(p P)
 ```
 
 Hovering over type parameters should link to documentation.
@@ -86,7 +86,7 @@ type GT[P any] struct {
 	F P //@hover("F", "F", F),hover("P", "P", FP)
 }
 
-func (GT[P]).M(p P)
+func (GT[P]) M(p P)
 ```
 
 Hovering over type parameters should link to documentation.
@@ -136,7 +136,7 @@ type GT[P any] struct {
 	F P //@hover("F", "F", F),hover("P", "P", FP)
 }
 
-func (generic.GT[P]).M(p P)
+func (generic.GT[P]) M(p P)
 ```
 
 Hovering over type parameters should link to documentation.

gopls/internal/test/marker/testdata/hover/methods.txt

@@ -0,0 +1,67 @@
+This test checks the formatting of the list of accessible methods.
+
+Observe that:
+- interface methods that appear in the syntax are not repeated
+  in the method set of the type;
+- promoted methods of structs are shown;
+- receiver variables are correctly named;
+- receiver variables have a pointer type if appropriate;
+- only accessible methods are shown.
+
+-- go.mod --
+module testdata
+
+-- lib/lib.go --
+package lib
+
+type I interface {
+	A()
+	b()
+	J
+}
+
+type J interface { C() }
+
+type S struct { I }
+func (s S) A() {}
+func (s S) b() {}
+func (s *S) PA() {}
+func (s *S) pb() {}
+
+-- a/a.go --
+package a
+
+import "testdata/lib"
+
+var _ lib.I //@hover("I", "I", I)
+var _ lib.J //@hover("J", "J", J)
+var _ lib.S //@hover("S", "S", S)
+
+-- @I --
+```go
+type I interface {
+	A()
+	b()
+	J
+}
+
+func (lib.J) C()
+```
+
+[`lib.I` on pkg.go.dev](https://pkg.go.dev/testdata/lib#I)
+-- @J --
+```go
+type J interface{ C() }
+```
+
+[`lib.J` on pkg.go.dev](https://pkg.go.dev/testdata/lib#J)
+-- @S --
+```go
+type S struct{ I }
+
+func (s lib.S) A()
+func (lib.J) C()
+func (s *lib.S) PA()
+```
+
+[`lib.S` on pkg.go.dev](https://pkg.go.dev/testdata/lib#S)