From b9fed7929fc1d5f8c3daa266389c35f3ee124473 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 29 Apr 2019 19:47:54 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/view.go | 1 + internal/lsp/completion.go | 15 +++-- internal/lsp/source/completion.go | 60 ++++++++++++------- internal/lsp/source/completion_format.go | 31 +++++----- internal/lsp/source/completion_snippet.go | 70 +++++++++++------------ internal/lsp/source/diagnostics.go | 10 +--- 6 files changed, 97 insertions(+), 90 deletions(-) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index e6109a20..913cf681 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -126,6 +126,7 @@ func (v *View) FileSet() *token.FileSet { 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 7e7a3783..7028101f 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -51,25 +51,24 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string 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 58e6f42d..ca05bbca 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -19,27 +19,43 @@ type CompletionItem struct { // 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 (c *completer) found(obj types.Object, weight float64) { 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 @@ func Completion(ctx context.Context, f File, pos token.Pos) ([]CompletionItem, s 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 @@ func Completion(ctx context.Context, f File, pos token.Pos) ([]CompletionItem, s 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 @@ func enclosingFunction(path []ast.Node, pos token.Pos, info *types.Info) *types. 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 @@ func (c *completer) expectedCompositeLiteralType(cl *ast.CompositeLit, kv *ast.K // 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 15015843..1ab5a3bf 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -35,8 +35,8 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { 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 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { } 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 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { 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 5e430e59..7cd6db79 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 @@ func (c *completer) funcCallSnippet(funcName string, params []string) (*snippet. } } - // 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 42a4f9b9..25946e6d 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -98,12 +98,8 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag } } 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 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag }) 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 @@ func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analys 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 {