internal/lsp: change some comments and variable names in completion code
Also, return diagnostics instead of errors from source.Diagnostics (to
avoid stuck diagnostics).
Change-Id: I56b067273982fd086ed74185e50eda5b72b5fba1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/174400
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e6109a2..913cf68 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -126,6 +126,7 @@
func (v *View) SetContent(ctx context.Context, uri span.URI, content []byte) error {
v.mu.Lock()
defer v.mu.Unlock()
+
// Cancel all still-running previous requests, since they would be
// operating on stale data.
v.cancel()
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 7e7a378..7028101 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -51,25 +51,24 @@
if !strings.HasPrefix(candidate.Label, prefix) {
continue
}
-
- insertText := candidate.Insert
+ insertText := candidate.InsertText
if insertTextFormat == protocol.SnippetTextFormat {
if usePlaceholders && candidate.PlaceholderSnippet != nil {
insertText = candidate.PlaceholderSnippet.String()
- } else if candidate.PlainSnippet != nil {
- insertText = candidate.PlainSnippet.String()
+ } else if candidate.Snippet != nil {
+ insertText = candidate.Snippet.String()
}
}
-
+ // If the user has already typed some part of the completion candidate,
+ // don't insert that portion of the text.
if strings.HasPrefix(insertText, prefix) {
insertText = insertText[len(prefix):]
}
-
- filterText := candidate.Insert
+ // Don't filter on text that might have snippets in it.
+ filterText := candidate.InsertText
if strings.HasPrefix(filterText, prefix) {
filterText = filterText[len(prefix):]
}
-
item := protocol.CompletionItem{
Label: candidate.Label,
Detail: candidate.Detail,
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 58e6f42..ca05bbc 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -19,27 +19,43 @@
// Label is the primary text the user sees for this completion item.
Label string
- // Detail is supplemental information to present to the user. This
- // often contains the Go type of the completion item.
+ // Detail is supplemental information to present to the user.
+ // This often contains the type or return type of the completion item.
Detail string
- // Insert is the text to insert if this item is selected. Any already-typed
- // prefix has not been trimmed. Insert does not contain snippets.
- Insert string
+ // InsertText is the text to insert if this item is selected.
+ // Any of the prefix that has already been typed is not trimmed.
+ // The insert text does not contain snippets.
+ InsertText string
Kind CompletionItemKind
- // Score is the internal relevance score. Higher is more relevant.
+ // Score is the internal relevance score.
+ // A higher score indicates that this completion item is more relevant.
Score float64
- // PlainSnippet is the LSP snippet to be inserted if not nil and snippets are
- // enabled and placeholders are not desired. This can contain tabs stops, but
- // should not contain placeholder text.
- PlainSnippet *snippet.Builder
+ // Snippet is the LSP snippet for the completion item, without placeholders.
+ // The LSP specification contains details about LSP snippets.
+ // For example, a snippet for a function with the following signature:
+ //
+ // func foo(a, b, c int)
+ //
+ // would be:
+ //
+ // foo(${1:})
+ //
+ Snippet *snippet.Builder
- // PlaceholderSnippet is the LSP snippet to be inserted if not nil and
- // snippets are enabled and placeholders are desired. This can contain
- // placeholder text.
+ // PlaceholderSnippet is the LSP snippet for the completion ite, containing
+ // placeholders. The LSP specification contains details about LSP snippets.
+ // For example, a placeholder snippet for a function with the following signature:
+ //
+ // func foo(a, b, c int)
+ //
+ // would be:
+ //
+ // foo(${1:a int}, ${2: b int}, ${3: c int})
+ //
PlaceholderSnippet *snippet.Builder
}
@@ -144,7 +160,7 @@
func Completion(ctx context.Context, f File, pos token.Pos) ([]CompletionItem, string, error) {
file := f.GetAST(ctx)
pkg := f.GetPackage(ctx)
- if pkg.IsIllTyped() {
+ if pkg == nil || pkg.IsIllTyped() {
return nil, "", fmt.Errorf("package for %s is ill typed", f.URI())
}
@@ -165,7 +181,7 @@
return nil, "", nil
}
- cl, kv, clField := enclosingCompositeLiteral(path, pos)
+ lit, kv, inCompositeLiteralField := enclosingCompositeLiteral(path, pos)
c := &completer{
types: pkg.GetTypes(),
info: pkg.GetTypesInfo(),
@@ -177,9 +193,9 @@
expectedType: expectedType(path, pos, pkg.GetTypesInfo()),
enclosingFunction: enclosingFunction(path, pos, pkg.GetTypesInfo()),
preferTypeNames: preferTypeNames(path, pos),
- enclosingCompositeLiteral: cl,
+ enclosingCompositeLiteral: lit,
enclosingKeyValue: kv,
- inCompositeLiteralField: clField,
+ inCompositeLiteralField: inCompositeLiteralField,
}
// Composite literals are handled entirely separately.
@@ -466,12 +482,12 @@
return nil
}
-func (c *completer) expectedCompositeLiteralType(cl *ast.CompositeLit, kv *ast.KeyValueExpr) types.Type {
- clType, ok := c.info.Types[cl]
+func (c *completer) expectedCompositeLiteralType(lit *ast.CompositeLit, kv *ast.KeyValueExpr) types.Type {
+ litType, ok := c.info.Types[lit]
if !ok {
return nil
}
- switch t := clType.Type.Underlying().(type) {
+ switch t := litType.Type.Underlying().(type) {
case *types.Slice:
return t.Elem()
case *types.Array:
@@ -501,14 +517,14 @@
// We are in a struct literal, but not a specific key-value pair.
// If the struct literal doesn't have explicit field names,
// we may still be able to suggest an expected type.
- for _, el := range cl.Elts {
+ for _, el := range lit.Elts {
if _, ok := el.(*ast.KeyValueExpr); ok {
return nil
}
}
// The order of the literal fields must match the order in the struct definition.
// Find the element that the position belongs to and suggest that field's type.
- if i := indexExprAtPos(c.pos, cl.Elts); i < t.NumFields() {
+ if i := indexExprAtPos(c.pos, lit.Elts); i < t.NumFields() {
return t.Field(i).Type()
}
}
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 1501584..1ab5a3b 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -35,8 +35,8 @@
detail = ""
} else {
val := o.Val().ExactString()
- if !strings.Contains(val, "\\n") { // skip any multiline constants
- label += " = " + o.Val().ExactString()
+ if !strings.ContainsRune(val, '\n') { // skip any multiline constants
+ label += " = " + val
}
}
kind = ConstantCompletionItem
@@ -46,24 +46,25 @@
}
if o.IsField() {
kind = FieldCompletionItem
- plainSnippet, placeholderSnippet = c.structFieldSnippet(label, detail)
+ plainSnippet, placeholderSnippet = c.structFieldSnippets(label, detail)
} else if c.isParameter(o) {
kind = ParameterCompletionItem
} else {
kind = VariableCompletionItem
}
case *types.Func:
- if sig, ok := o.Type().(*types.Signature); ok {
- params := formatEachParam(sig, c.qf)
- label += formatParamParts(params)
- detail = strings.Trim(types.TypeString(sig.Results(), c.qf), "()")
- kind = FunctionCompletionItem
- if sig.Recv() != nil {
- kind = MethodCompletionItem
- }
-
- plainSnippet, placeholderSnippet = c.funcCallSnippet(obj.Name(), params)
+ sig, ok := o.Type().(*types.Signature)
+ if !ok {
+ break
}
+ params := formatEachParam(sig, c.qf)
+ label += formatParamParts(params)
+ detail = strings.Trim(types.TypeString(sig.Results(), c.qf), "()")
+ kind = FunctionCompletionItem
+ if sig.Recv() != nil {
+ kind = MethodCompletionItem
+ }
+ plainSnippet, placeholderSnippet = c.functionCallSnippets(obj.Name(), params)
case *types.Builtin:
item, ok := builtinDetails[obj.Name()]
if !ok {
@@ -82,11 +83,11 @@
return CompletionItem{
Label: label,
- Insert: insert,
+ InsertText: insert,
Detail: detail,
Kind: kind,
Score: score,
- PlainSnippet: plainSnippet,
+ Snippet: plainSnippet,
PlaceholderSnippet: placeholderSnippet,
}
}
diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go
index 5e430e5..7cd6db7 100644
--- a/internal/lsp/source/completion_snippet.go
+++ b/internal/lsp/source/completion_snippet.go
@@ -5,69 +5,66 @@
package source
import (
+ "fmt"
"go/ast"
"golang.org/x/tools/internal/lsp/snippet"
)
-// structField calculates the plain and placeholder snippets for struct literal
-// field names as in "Foo{Ba<>".
-func (c *completer) structFieldSnippet(label, detail string) (*snippet.Builder, *snippet.Builder) {
+// structFieldSnippets calculates the plain and placeholder snippets for struct literal field names.
+func (c *completer) structFieldSnippets(label, detail string) (*snippet.Builder, *snippet.Builder) {
if !c.inCompositeLiteralField {
return nil, nil
}
- cl := c.enclosingCompositeLiteral
+ lit := c.enclosingCompositeLiteral
kv := c.enclosingKeyValue
- // If we aren't in a composite literal or are already in a key/value
- // expression, we don't want a snippet.
- if cl == nil || kv != nil {
+ // If we aren't in a composite literal or are already in a key-value expression,
+ // we don't want a snippet.
+ if lit == nil || kv != nil {
return nil, nil
}
-
- if len(cl.Elts) > 0 {
- i := indexExprAtPos(c.pos, cl.Elts)
- if i >= len(cl.Elts) {
+ // First, confirm that we are actually completing a struct field.
+ if len(lit.Elts) > 0 {
+ i := indexExprAtPos(c.pos, lit.Elts)
+ if i >= len(lit.Elts) {
return nil, nil
}
-
- // If our expression is not an identifer, we know it isn't a
- // struct field name.
- if _, ok := cl.Elts[i].(*ast.Ident); !ok {
+ // If the expression is not an identifer, it is not a struct field name.
+ if _, ok := lit.Elts[i].(*ast.Ident); !ok {
return nil, nil
}
}
- // It is a multi-line literal if pos is not on the same line as the literal's
- // opening brace.
- multiLine := c.fset.Position(c.pos).Line != c.fset.Position(cl.Lbrace).Line
+ plain, placeholder := &snippet.Builder{}, &snippet.Builder{}
+ label = fmt.Sprintf("%s: ", label)
- // Plain snippet will turn "Foo{Ba<>" into "Foo{Bar: <>"
- plain := &snippet.Builder{}
- plain.WriteText(label + ": ")
+ // A plain snippet turns "Foo{Ba<>" into "Foo{Bar: <>".
+ plain.WriteText(label)
plain.WritePlaceholder(nil)
- if multiLine {
- plain.WriteText(",")
- }
- // Placeholder snippet will turn "Foo{Ba<>" into "Foo{Bar: *int*"
- placeholder := &snippet.Builder{}
- placeholder.WriteText(label + ": ")
+ // A placeholder snippet turns "Foo{Ba<>" into "Foo{Bar: <*int*>".
+ placeholder.WriteText(label)
placeholder.WritePlaceholder(func(b *snippet.Builder) {
b.WriteText(detail)
})
- if multiLine {
+
+ // If the cursor position is on a different line from the literal's opening brace,
+ // we are in a multiline literal.
+ if c.fset.Position(c.pos).Line != c.fset.Position(lit.Lbrace).Line {
+ plain.WriteText(",")
placeholder.WriteText(",")
}
return plain, placeholder
}
-// funcCall calculates the plain and placeholder snippets for function calls.
-func (c *completer) funcCallSnippet(funcName string, params []string) (*snippet.Builder, *snippet.Builder) {
+// functionCallSnippets calculates the plain and placeholder snippets for function calls.
+func (c *completer) functionCallSnippets(name string, params []string) (*snippet.Builder, *snippet.Builder) {
for i := 1; i <= 2 && i < len(c.path); i++ {
call, ok := c.path[i].(*ast.CallExpr)
+
// If we are the left side (i.e. "Fun") part of a call expression,
// we don't want a snippet since there are already parens present.
if ok && call.Fun == c.path[i-1] {
@@ -75,17 +72,18 @@
}
}
- // Plain snippet turns "someFun<>" into "someFunc(<>)"
- plain := &snippet.Builder{}
- plain.WriteText(funcName + "(")
+ plain, placeholder := &snippet.Builder{}, &snippet.Builder{}
+ label := fmt.Sprintf("%s(", name)
+
+ // A plain snippet turns "someFun<>" into "someFunc(<>)".
+ plain.WriteText(label)
if len(params) > 0 {
plain.WritePlaceholder(nil)
}
plain.WriteText(")")
- // Placeholder snippet turns "someFun<>" into "someFunc(*i int*, s string)"
- placeholder := &snippet.Builder{}
- placeholder.WriteText(funcName + "(")
+ // A placeholder snippet turns "someFun<>" into "someFunc(<*i int*>, *s string*)".
+ placeholder.WriteText(label)
for i, p := range params {
if i > 0 {
placeholder.WriteText(", ")
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 42a4f9b..25946e6 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -98,12 +98,8 @@
}
}
if len(diags) > 0 {
- v.Logger().Debugf(ctx, "found parse or type-checking errors for %s, returning", uri)
return reports, nil
}
-
- v.Logger().Debugf(ctx, "running `go vet` analyses for %s", uri)
-
// Type checking and parsing succeeded. Run analyses.
if err := runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) error {
r := span.NewRange(v.FileSet(), diag.Pos, 0)
@@ -124,11 +120,9 @@
})
return nil
}); err != nil {
- return nil, err
+ return singleDiagnostic(uri, "unable to run analyses for %s: %v", uri, err), nil
}
- v.Logger().Debugf(ctx, "completed reporting `go vet` analyses for %s", uri)
-
return reports, nil
}
@@ -208,8 +202,6 @@
return err
}
- v.Logger().Debugf(ctx, "analyses have completed for %s", pkg.GetTypes().Path())
-
// Report diagnostics and errors from root analyzers.
for _, r := range roots {
for _, diag := range r.diagnostics {
