internal/lsp/cache: improve completion after dangling selector

Dangling selectors such as:

    func _() {
      x.
    }
    var x struct { i int }

tend to wreak havoc on the AST. In the above example you didn't used
to get completions because the declaration of "x" was missing from the
AST.

We now work around this issue by inserting a "_" into the source code
before parsing to make the selector valid:

    func _() {
      x._ // <-- insert "_" here
    }
    var x struct { i int }

This makes completion work as expected because the declaration of "x"
is present in the AST.

I had to change fixAST() to be called before fixSrc() because
otherwise this new workaround in fixSrc() breaks the "accidental
keyword" countermeasures in fixAST().

Fixes golang/go#31973.
Updates golang/go#31687.

Change-Id: Ia7ef6c045a9c71502d1b8b36f187ac9b8a85fe21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216484
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/parse.go

@@ -18,7 +18,6 @@ import (
 	"golang.org/x/tools/internal/lsp/telemetry"
 	"golang.org/x/tools/internal/memoize"
 	"golang.org/x/tools/internal/span"
-	"golang.org/x/tools/internal/telemetry/log"
 	"golang.org/x/tools/internal/telemetry/trace"
 	errors "golang.org/x/xerrors"
 )
@@ -134,6 +133,9 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod
 			return &parseGoData{err: errors.Errorf("successfully parsed but no token.File for %s (%v)", fh.Identity().URI, parseError)}
 		}
 
+		// Fix any badly parsed parts of the AST.
+		_ = fixAST(ctx, file, tok, buf)
+
 		// Fix certain syntax errors that render the file unparseable.
 		newSrc := fixSrc(file, tok, buf)
 		if newSrc != nil {
@@ -143,17 +145,14 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod
 				file = newFile
 				buf = newSrc
 				tok = fset.File(file.Pos())
+
+				_ = fixAST(ctx, file, tok, buf)
 			}
 		}
 
 		if mode == source.ParseExported {
 			trimAST(file)
 		}
-
-		// Fix any badly parsed parts of the AST.
-		if err := fixAST(ctx, file, tok, buf); err != nil {
-			log.Error(ctx, "failed to fix AST", err)
-		}
 	}
 
 	if file == nil {
@@ -301,6 +300,8 @@ func fixSrc(f *ast.File, tok *token.File, src []byte) (newSrc []byte) {
 		switch n := n.(type) {
 		case *ast.BlockStmt:
 			newSrc = fixMissingCurlies(f, n, parent, tok, src)
+		case *ast.SelectorExpr:
+			newSrc = fixDanglingSelector(f, n, parent, tok, src)
 		}
 
 		return newSrc == nil
@@ -390,6 +391,76 @@ func fixMissingCurlies(f *ast.File, b *ast.BlockStmt, parent ast.Node, tok *toke
 	return buf.Bytes()
 }
 
+// fixDanglingSelector inserts real "_" selector expressions in place
+// of phantom "_" selectors. For example:
+//
+// func _() {
+//   x.<>
+// }
+// var x struct { i int }
+//
+// To fix completion at "<>", we insert a real "_" after the "." so the
+// following declaration of "x" can be parsed and type checked
+// normally.
+func fixDanglingSelector(f *ast.File, s *ast.SelectorExpr, parent ast.Node, tok *token.File, src []byte) []byte {
+	if !isPhantomUnderscore(s.Sel, tok, src) {
+		return nil
+	}
+
+	if !s.X.End().IsValid() {
+		return nil
+	}
+
+	// Insert directly after the selector's ".".
+	insertOffset := tok.Offset(s.X.End()) + 1
+	if src[insertOffset-1] != '.' {
+		return nil
+	}
+
+	var buf bytes.Buffer
+	buf.Grow(len(src) + 1)
+	buf.Write(src[:insertOffset])
+	buf.WriteByte('_')
+	buf.Write(src[insertOffset:])
+	return buf.Bytes()
+}
+
+// fixAccidentalDecl tries to fix "accidental" declarations. For example:
+//
+// func typeOf() {}
+// type<> // want to call typeOf(), not declare a type
+//
+// If we find an *ast.DeclStmt with only a single phantom "_" spec, we
+// replace the decl statement with an expression statement containing
+// only the keyword. This allows completion to work to some degree.
+func fixAccidentalDecl(decl *ast.DeclStmt, parent ast.Node, tok *token.File, src []byte) {
+	genDecl, _ := decl.Decl.(*ast.GenDecl)
+	if genDecl == nil || len(genDecl.Specs) != 1 {
+		return
+	}
+
+	switch spec := genDecl.Specs[0].(type) {
+	case *ast.TypeSpec:
+		// If the name isn't a phantom "_" identifier inserted by the
+		// parser then the decl is likely legitimate and we shouldn't mess
+		// with it.
+		if !isPhantomUnderscore(spec.Name, tok, src) {
+			return
+		}
+	case *ast.ValueSpec:
+		if len(spec.Names) != 1 || !isPhantomUnderscore(spec.Names[0], tok, src) {
+			return
+		}
+	}
+
+	replaceNode(parent, decl, &ast.ExprStmt{
+		X: &ast.Ident{
+			Name:    genDecl.Tok.String(),
+			NamePos: decl.Pos(),
+		},
+	})
+}
+
 // fixPhantomSelector tries to fix selector expressions with phantom
 // "_" selectors. In particular, we check if the selector is a
 // keyword, and if so we swap in an *ast.Ident with the keyword text. For example:
@@ -402,6 +473,16 @@ func fixPhantomSelector(sel *ast.SelectorExpr, tok *token.File, src []byte) {
 		return
 	}
 
+	// Only consider selectors directly abutting the selector ".". This
+	// avoids false positives in cases like:
+	//
+	//   foo. // don't think "var" is our selector
+	//   var bar = 123
+	//
+	if sel.Sel.Pos() != sel.X.End()+1 {
+		return
+	}
+
 	maybeKeyword := readKeyword(sel.Sel.Pos(), tok, src)
 	if maybeKeyword == "" {
 		return

internal/lsp/testdata/lsp/primarymod/baz/baz.go.in

@@ -20,13 +20,13 @@ func Baz() {
 
 func _() {
 	bob := f.StructFoo{Value: 5}
-	if x := bob.           //@complete(" //", Value)
+	if x := bob. //@complete(" //", Value)
 	switch true == false {
 		case true:
-			if x := bob.   //@complete(" //", Value)
+			if x := bob. //@complete(" //", Value)
 		case false:
 	}
-	if x := bob.Va         //@complete("a", Value)
+	if x := bob.Va //@complete("a", Value)
 	switch true == true {
 		default:
 	}

internal/lsp/testdata/lsp/primarymod/danglingstmt/dangling_selector_1.go

@@ -0,0 +1,7 @@
+package danglingstmt
+
+func _() {
+	x. //@rank(" //", danglingI)
+}
+
+var x struct { i int } //@item(danglingI, "i", "int", "field")

internal/lsp/testdata/lsp/primarymod/danglingstmt/dangling_selector_2.go

@@ -0,0 +1,8 @@
+package danglingstmt
+
+import "golang.org/x/tools/internal/lsp/foo"
+
+func _() {
+	foo. //@rank(" //", Foo)
+	var _ = []string{foo.} //@rank("}", Foo)
+}

internal/lsp/testdata/lsp/summary.txt.golden

@@ -4,7 +4,7 @@ CompletionSnippetCount = 67
 UnimportedCompletionsCount = 11
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 8
-RankedCompletionsCount = 95
+RankedCompletionsCount = 98
 CaseSensitiveCompletionsCount = 4
 DiagnosticsCount = 38
 FoldingRangesCount = 2