diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 6dff423a54b..3ea1d9763e6 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -231,21 +231,161 @@ func fix(ctx context.Context, n ast.Node, tok *token.File, src []byte) error { // Don't propagate this error since *ast.BadExpr is very common // and it is only sometimes due to array types. Errors from here // are expected and not actionable in general. - fixArrayErr := fixArrayType(n, parent, tok, src) - if fixArrayErr == nil { + if fixArrayType(n, parent, tok, src) == nil { // Recursively fix in our fixed node. err = fix(ctx, parent, tok, src) + return false } + + // Fix cases where the parser expects an expression but finds a keyword, e.g.: + // + // someFunc(var<>) // want to complete to "variance" + // + fixAccidentalKeyword(n, parent, tok, src) + return false + case *ast.DeclStmt: + // Fix cases where the completion prefix looks like a decl, e.g.: + // + // func typeName(obj interface{}) string {} + // type<> // want to call "typeName()" but looks like a "type" decl + // + fixAccidentalDecl(n, parent, tok, src) + return false + case *ast.SelectorExpr: + // Fix cases where a keyword prefix results in a phantom "_" selector, e.g.: + // + // foo.var<> // want to complete to "foo.variance" + // + fixPhantomSelector(n, tok, src) + return true default: ancestors = append(ancestors, n) parent = n return true } }) + return err } +// 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: +// +// foo.var +// +// yields a "_" selector instead of "var" since "var" is a keyword. +func fixPhantomSelector(sel *ast.SelectorExpr, tok *token.File, src []byte) { + if !isPhantomUnderscore(sel.Sel, tok, src) { + return + } + + maybeKeyword := readKeyword(sel.Sel.Pos(), tok, src) + if maybeKeyword == "" { + return + } + + replaceNode(sel, sel.Sel, &ast.Ident{ + Name: maybeKeyword, + NamePos: sel.Sel.Pos(), + }) +} + +// isPhantomUnderscore reports whether the given ident is a phantom +// underscore. The parser sometimes inserts phantom underscores when +// it encounters otherwise unparseable situations. +func isPhantomUnderscore(id *ast.Ident, tok *token.File, src []byte) bool { + if id == nil || id.Name != "_" { + return false + } + + // Phantom underscore means the underscore is not actually in the + // program text. + offset := tok.Offset(id.Pos()) + return len(src) <= offset || src[offset] != '_' +} + +// fixAccidentalKeyword tries to fix "accidental" keyword expressions. For example: +// +// variance := 123 +// doMath(var<>) +// +// If we find an *ast.BadExpr that begins with a keyword, we replace +// the BadExpr with an *ast.Ident containing the text of the keyword. +// This allows completion to work to some degree. +func fixAccidentalKeyword(bad *ast.BadExpr, parent ast.Node, tok *token.File, src []byte) { + if !bad.Pos().IsValid() { + return + } + + maybeKeyword := readKeyword(bad.Pos(), tok, src) + if maybeKeyword == "" { + return + } + + replaceNode(parent, bad, &ast.Ident{Name: maybeKeyword, NamePos: bad.Pos()}) +} + +// readKeyword reads the keyword starting at pos, if any. +func readKeyword(pos token.Pos, tok *token.File, src []byte) string { + var kwBytes []byte + for i := tok.Offset(pos); i < len(src); i++ { + // Use a simplified identifier check since keywords are always lowercase ASCII. + if src[i] < 'a' || src[i] > 'z' { + break + } + kwBytes = append(kwBytes, src[i]) + + // Stop search at arbitrarily chosen too-long-for-a-keyword length. + if len(kwBytes) > 15 { + return "" + } + } + + if kw := string(kwBytes); token.Lookup(kw).IsKeyword() { + return kw + } + + return "" +} + // fixArrayType tries to parse an *ast.BadExpr into an *ast.ArrayType. // go/parser often turns lone array types like "[]int" into BadExprs // if it isn't expecting a type. diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index d5bcf0fffe3..b06c6346a9d 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -786,7 +786,7 @@ func importPath(s *ast.ImportSpec) string { } func nodeContains(n ast.Node, pos token.Pos) bool { - return n != nil && n.Pos() <= pos && pos < n.End() + return n != nil && n.Pos() <= pos && pos <= n.End() } func (c *completer) inConstDecl() bool { diff --git a/internal/lsp/testdata/keywords/accidental_keywords.go.in b/internal/lsp/testdata/keywords/accidental_keywords.go.in new file mode 100644 index 00000000000..1ae8ff39d4c --- /dev/null +++ b/internal/lsp/testdata/keywords/accidental_keywords.go.in @@ -0,0 +1,19 @@ +package keywords + +// non-matching candidate - shouldn't show up as completion +var apple = "apple" + +func _() { + variance := 123 //@item(kwVariance, "variance", "int", "var") + println(var) //@complete(")", kwVariance) +} + +func _() { + var s struct { variance int } //@item(kwVarianceField, "variance", "int", "field") + s.var //@complete(" //", kwVarianceField) +} + +func _() { + var typeName string //@item(kwTypeName, "typeName", "string", "var") + type //@complete(" //", kwTypeName) +} diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index fdfde62f623..60d49aabaf0 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,5 +1,5 @@ -- summary -- -CompletionsCount = 216 +CompletionsCount = 219 CompletionSnippetCount = 51 UnimportedCompletionsCount = 3 DeepCompletionsCount = 5