internal/lsp: improve completions in *ast.FieldList

Now we always expect type names inside of *ast.FieldList. This expands
the previous func signature logic to also work for *ast.StructType
and *ast.InterfaceType. For example, we will now prefer type names in
cases like:

type myStruct struct { i i<> }

Also, fix a check for anonymous fields to make sure the field is
actually embedded. This fixes cases like this to properly have no
completions:

type myStruct struct { i<> i }

where this will still give type name completions:

type myStruct struct { i<> }

I introduced a new error type source.ErrIsDefinition so source_test.go
could avoid erroring out on tests that make sure definition
identifiers have no completions.

Fixes golang/go#34412.

Change-Id: Ib56cb52af639f2e2b132274d1f04f8074c0d9353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196560
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/source/completion.go

@@ -343,6 +343,21 @@ type candidate struct {
 	imp *imports.ImportInfo
 }
 
+// ErrIsDefinition is an error that informs the user they got no
+// completions because they tried to complete the name of a new object
+// being defined.
+type ErrIsDefinition struct {
+	objStr string
+}
+
+func (e ErrIsDefinition) Error() string {
+	msg := "this is a definition"
+	if e.objStr != "" {
+		msg += " of " + e.objStr
+	}
+	return msg
+}
+
 // Completion returns a list of possible candidates for completion, given a
 // a file and a position.
 //
@@ -449,15 +464,15 @@ func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position,
 		}
 		// reject defining identifiers
 		if obj, ok := pkg.GetTypesInfo().Defs[n]; ok {
-			if v, ok := obj.(*types.Var); ok && v.IsField() {
+			if v, ok := obj.(*types.Var); ok && v.IsField() && v.Embedded() {
 				// An anonymous field is also a reference to a type.
 			} else {
-				of := ""
+				objStr := ""
 				if obj != nil {
 					qual := types.RelativeTo(pkg.GetTypes())
-					of += ", of " + types.ObjectString(obj, qual)
+					objStr = types.ObjectString(obj, qual)
 				}
-				return nil, nil, errors.Errorf("this is a definition%s", of)
+				return nil, nil, ErrIsDefinition{objStr: objStr}
 			}
 		}
 		if err := c.lexical(); err != nil {
@@ -1134,24 +1149,14 @@ func expectTypeName(c *completer) typeInference {
 Nodes:
 	for i, p := range c.path {
 		switch n := p.(type) {
-		case *ast.FuncDecl:
-			// Expect type names in a function declaration receiver, params and results.
-
-			if r := n.Recv; r != nil && r.Pos() <= c.pos && c.pos <= r.End() {
-				wantTypeName = true
-				break Nodes
-			}
-			if t := n.Type; t != nil {
-				if p := t.Params; p != nil && p.Pos() <= c.pos && c.pos <= p.End() {
-					wantTypeName = true
-					break Nodes
-				}
-				if r := t.Results; r != nil && r.Pos() <= c.pos && c.pos <= r.End() {
-					wantTypeName = true
-					break Nodes
-				}
-			}
-			return typeInference{}
+		case *ast.FieldList:
+			// Expect a type name if pos is in a FieldList. This applies to
+			// FuncType params/results, FuncDecl receiver, StructType, and
+			// InterfaceType. We don't need to worry about the field name
+			// because completion bails out early if pos is in an *ast.Ident
+			// that defines an object.
+			wantTypeName = true
+			break Nodes
 		case *ast.CaseClause:
 			// Expect type names in type switch case clauses.
 			if swtch, ok := findSwitchStmt(c.path[i+1:], c.pos, n).(*ast.TypeSwitchStmt); ok {

internal/lsp/source/source_test.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/testenv"
+	errors "golang.org/x/xerrors"
 )
 
 func TestMain(m *testing.M) {
@@ -246,7 +247,7 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options source.Comp
 		Line:      float64(src.Start().Line() - 1),
 		Character: float64(src.Start().Column() - 1),
 	}, options)
-	if err != nil {
+	if err != nil && !errors.As(err, &source.ErrIsDefinition{}) {
 		t.Fatalf("failed for %v: %v", src, err)
 	}
 	var prefix string

internal/lsp/testdata/fieldlist/field_list.go

@@ -0,0 +1,27 @@
+package fieldlist
+
+var myInt int   //@item(flVar, "myInt", "int", "var")
+type myType int //@item(flType, "myType", "int", "type")
+
+func (my) _()    {} //@complete(") _", flType, flVar)
+func (my my) _() {} //@complete(" my)"),complete(") _", flType, flVar)
+
+func (myType) _() {} //@complete(") {", flType, flVar)
+
+func (myType) _(my my) {} //@complete(" my)"),complete(") {", flType, flVar)
+
+func (myType) _() my {} //@complete(" {", flType, flVar)
+
+func (myType) _() (my my) {} //@complete(" my"),complete(") {", flType, flVar)
+
+func _() {
+	var _ struct {
+		//@complete("", flType, flVar)
+		m my //@complete(" my"),complete(" //", flType, flVar)
+	}
+
+	var _ interface {
+		//@complete("", flType, flVar)
+		m() my //@complete("("),complete(" //", flType, flVar)
+	}
+}

internal/lsp/tests/tests.go

@@ -29,7 +29,7 @@ import (
 // We hardcode the expected number of test cases to ensure that all tests
 // are being executed. If a test is added, this number must be changed.
 const (
-	ExpectedCompletionsCount           = 154
+	ExpectedCompletionsCount           = 169
 	ExpectedCompletionSnippetCount     = 36
 	ExpectedUnimportedCompletionsCount = 1
 	ExpectedDeepCompletionsCount       = 5