internal/lsp/source: refactor c.item to support deepSearch in all cases
This change eliminates any special scenarios where we need to call
c.item instead of going through deepSearch by adding support for all the
cases in deepSearch and c.addItem (previously c.item).
Change-Id: Ifb250be54da2f8c7b656475fcafaa38a4e306244
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258858
Run-TryBot: Danish Dua <danishdua@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Danish Dua <danishdua@google.com>
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 9ce8dca..9b55f2f 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -28,7 +28,7 @@
var surrounding *completion.Selection
switch fh.Kind() {
case source.Go:
- candidates, surrounding, err = completion.Completion(ctx, snapshot, fh, params.Position, params.Context.TriggerCharacter)
+ candidates, surrounding, err = completion.Completion(ctx, snapshot, fh, params.Position, params.Context)
case source.Mod:
candidates, surrounding = nil, nil
}
diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go
index 7140374..7e932eb 100644
--- a/internal/lsp/source/completion/completion.go
+++ b/internal/lsp/source/completion/completion.go
@@ -155,8 +155,9 @@
qf types.Qualifier
opts *completionOptions
- // triggerCharacter is the character that triggered this request, if any.
- triggerCharacter string
+ // completionContext contains information about the trigger for this
+ // completion request.
+ completionContext completionContext
// fh is a handle to the file associated with this completion request.
fh source.FileHandle
@@ -259,6 +260,21 @@
addressable bool
}
+type completionContext struct {
+ // triggerCharacter is the character used to trigger completion at current
+ // position, if any.
+ triggerCharacter string
+
+ // triggerKind is information about how a completion was triggered.
+ triggerKind protocol.CompletionTriggerKind
+
+ // commentCompletion is true if we are completing a comment.
+ commentCompletion bool
+
+ // packageCompletion is true if we are completing a package name.
+ packageCompletion bool
+}
+
// A Selection represents the cursor position and surrounding identifier.
type Selection struct {
content string
@@ -337,6 +353,10 @@
// name is the deep object name path, e.g. "foo.bar"
name string
+ // detail is additional information about this item. If not specified,
+ // defaults to type string for the object.
+ detail string
+
// path holds the path from the search root (excluding the candidate
// itself) for a deep candidate.
path []types.Object
@@ -394,7 +414,7 @@
// The selection is computed based on the preceding identifier and can be used by
// the client to score the quality of the completion. For instance, some clients
// may tolerate imperfect matches as valid completion results, since users may make typos.
-func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, protoPos protocol.Position, triggerCharacter string) ([]CompletionItem, *Selection, error) {
+func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, protoPos protocol.Position, protoContext protocol.CompletionContext) ([]CompletionItem, *Selection, error) {
ctx, done := event.Start(ctx, "completion.Completion")
defer done()
@@ -469,10 +489,13 @@
opts := snapshot.View().Options()
c := &completer{
- pkg: pkg,
- snapshot: snapshot,
- qf: source.Qualifier(pgf.File, pkg.GetTypes(), pkg.GetTypesInfo()),
- triggerCharacter: triggerCharacter,
+ pkg: pkg,
+ snapshot: snapshot,
+ qf: source.Qualifier(pgf.File, pkg.GetTypes(), pkg.GetTypesInfo()),
+ completionContext: completionContext{
+ triggerCharacter: protoContext.TriggerCharacter,
+ triggerKind: protoContext.TriggerKind,
+ },
fh: fh,
filename: fh.URI().Filename(),
file: pgf.File,
@@ -556,9 +579,6 @@
// Inside comments, offer completions for the name of the relevant symbol.
for _, comment := range c.file.Comments {
if comment.Pos() < c.pos && c.pos <= comment.End() {
- // Deep completion doesn't work properly in comments since we don't
- // have a type object to complete further.
- c.deepState.enabled = false
c.populateCommentCompletions(ctx, comment)
return nil
}
@@ -703,6 +723,9 @@
// (i.e. "golang.org/x/"). The user is meant to accept completion suggestions
// until they reach a complete import path.
func (c *completer) populateImportCompletions(ctx context.Context, searchImport *ast.ImportSpec) error {
+ // deepSearch is not valuable for import completions.
+ c.deepState.enabled = false
+
importPath := searchImport.Path.Value
// Extract the text between the quotes (if any) in an import spec.
@@ -804,15 +827,13 @@
mu.Lock()
defer mu.Unlock()
- obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkgToConsider, pkg.IdentName))
- cand := candidate{obj: obj, name: namePrefix + name + nameSuffix, score: score}
- // We use c.item here to be able to manually update the detail for a
- // candidate. deepSearch doesn't give us access to the completion item,
- // so we don't enqueue the item here.
- if item, err := c.item(ctx, cand); err == nil {
- item.Detail = fmt.Sprintf("%q", pkgToConsider)
- c.items = append(c.items, item)
- }
+ name = namePrefix + name + nameSuffix
+ obj := types.NewPkgName(0, nil, name, types.NewPackage(pkgToConsider, name))
+ c.deepState.enqueue(candidate{
+ obj: obj,
+ detail: fmt.Sprintf("%q", pkgToConsider),
+ score: score,
+ })
}
c.completionCallbacks = append(c.completionCallbacks, func(opts *imports.Options) error {
@@ -825,7 +846,7 @@
func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast.CommentGroup) {
// If the completion was triggered by a period, ignore it. These types of
// completions will not be useful in comments.
- if c.triggerCharacter == "." {
+ if c.completionContext.triggerCharacter == "." {
return
}
@@ -835,6 +856,15 @@
return
}
+ // Deep completion doesn't work properly in comments since we don't
+ // have a type object to complete further.
+ c.deepState.enabled = false
+ c.completionContext.commentCompletion = true
+
+ // Documentation isn't useful in comments, since it might end up being the
+ // comment itself.
+ c.opts.documentation = false
+
commentLine := file.Line(comment.End())
// comment is valid, set surrounding as word boundaries around cursor
@@ -859,9 +889,6 @@
continue
}
obj := c.pkg.GetTypesInfo().ObjectOf(name)
- if matchScore := c.matcher.Score(name.String()); matchScore <= 0 {
- continue
- }
c.deepState.enqueue(candidate{obj: obj, score: stdScore})
}
case *ast.TypeSpec:
@@ -881,9 +908,6 @@
}
obj := c.pkg.GetTypesInfo().ObjectOf(spec.Name)
- if matchScore := c.matcher.Score(obj.Name()); matchScore <= 0 {
- continue
- }
// Type name should get a higher score than fields but not highScore by default
// since field near a comment cursor gets a highScore
score := stdScore * 1.1
@@ -892,13 +916,7 @@
score = highScore
}
- // we use c.item in addFieldItems so we have to use c.item
- // here to ensure scoring order is maintained. deepSearch
- // manipulates the score so we can't enqueue the item
- // directly.
- if item, err := c.item(ctx, candidate{obj: obj, name: obj.Name(), score: score}); err == nil {
- c.items = append(c.items, item)
- }
+ c.deepState.enqueue(candidate{obj: obj, score: score})
}
}
// handle functions
@@ -926,18 +944,7 @@
}
for i := 0; i < recvStruct.NumFields(); i++ {
field := recvStruct.Field(i)
- // we use c.item in addFieldItems so we have to
- // use c.item here to ensure scoring order is
- // maintained. deepSearch manipulates the score so
- // we can't enqueue the items directly.
- if matchScore := c.matcher.Score(field.Name()); matchScore <= 0 {
- continue
- }
- item, err := c.item(ctx, candidate{obj: field, name: field.Name(), score: lowScore})
- if err != nil {
- continue
- }
- c.items = append(c.items, item)
+ c.deepState.enqueue(candidate{obj: field, score: lowScore})
}
}
}
@@ -952,21 +959,7 @@
continue
}
- if matchScore := c.matcher.Score(obj.Name()); matchScore <= 0 {
- continue
- }
- // We don't want to expandFuncCall inside comments. deepSearch
- // doesn't respect this setting so we don't enqueue the item here.
- item, err := c.item(ctx, candidate{
- obj: obj,
- name: obj.Name(),
- expandFuncCall: false,
- score: highScore,
- })
- if err != nil {
- continue
- }
- c.items = append(c.items, item)
+ c.deepState.enqueue(candidate{obj: obj, score: highScore})
}
}
}
@@ -1028,10 +1021,6 @@
continue
}
- if matchScore := c.matcher.Score(obj.Name()); matchScore <= 0 {
- continue
- }
-
// if we're in a field comment/doc, score that field as more relevant
score := stdScore
if field.Comment != nil && field.Comment.Pos() <= cursor && cursor <= field.Comment.End() {
@@ -1040,17 +1029,7 @@
score = highScore
}
- cand := candidate{
- obj: obj,
- name: obj.Name(),
- expandFuncCall: false,
- score: score,
- }
- // We don't want to expandFuncCall inside comments. deepSearch
- // doesn't respect this setting so we don't enqueue the item here.
- if item, err := c.item(ctx, cand); err == nil {
- c.items = append(c.items, item)
- }
+ c.deepState.enqueue(candidate{obj: obj, score: score})
}
}
}
@@ -1065,7 +1044,7 @@
}
func (c *completer) wantTypeName() bool {
- return c.inference.typeName.wantTypeName
+ return !c.completionContext.commentCompletion && c.inference.typeName.wantTypeName
}
// See https://golang.org/issue/36001. Unimported completions are expensive.
@@ -2435,6 +2414,10 @@
// matchingCandidate reports whether cand matches our type inferences.
// It mutates cand's score in certain cases.
func (c *completer) matchingCandidate(cand *candidate) bool {
+ if c.completionContext.commentCompletion {
+ return false
+ }
+
if isTypeName(cand.obj) {
return c.matchingTypeName(cand)
} else if c.wantTypeName() {
diff --git a/internal/lsp/source/completion/completion_format.go b/internal/lsp/source/completion/completion_format.go
index 9549f87..878beba 100644
--- a/internal/lsp/source/completion/completion_format.go
+++ b/internal/lsp/source/completion/completion_format.go
@@ -17,12 +17,25 @@
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
+ errors "golang.org/x/xerrors"
)
-// formatCompletion creates a completion item for a given candidate.
+// item formats a candidate to a CompletionItem.
func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, error) {
obj := cand.obj
+ // if the object isn't a valid match against the surrounding, return early.
+ matchScore := c.matcher.Score(cand.name)
+ if matchScore <= 0 {
+ return CompletionItem{}, errors.New("not a surrounding match")
+ }
+ cand.score *= float64(matchScore)
+
+ // Ignore deep candidates that wont be in the MaxDeepCompletions anyway.
+ if len(cand.path) != 0 && !c.deepState.isHighScore(cand.score) {
+ return CompletionItem{}, errors.New("not a high scoring candidate")
+ }
+
// Handle builtin types separately.
if obj.Parent() == types.Universe {
return c.formatBuiltin(ctx, cand)
@@ -42,14 +55,10 @@
// expandFuncCall mutates the completion label, detail, and snippet
// to that of an invocation of sig.
- expandFuncCall := func(sig *types.Signature) error {
- s, err := source.NewSignature(ctx, c.snapshot, c.pkg, c.file, "", sig, nil, c.qf)
- if err != nil {
- return err
- }
+ expandFuncCall := func(sig *types.Signature) {
+ s := source.NewSignature(ctx, c.snapshot, c.pkg, c.file, "", sig, nil, c.qf)
snip = c.functionCallSnippet(label, s.Params())
detail = "func" + s.Format()
- return nil
}
switch obj := obj.(type) {
@@ -74,9 +83,7 @@
}
if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall {
- if err := expandFuncCall(sig); err != nil {
- return CompletionItem{}, err
- }
+ expandFuncCall(sig)
}
case *types.Func:
sig, ok := obj.Type().Underlying().(*types.Signature)
@@ -89,9 +96,7 @@
}
if cand.expandFuncCall {
- if err := expandFuncCall(sig); err != nil {
- return CompletionItem{}, err
- }
+ expandFuncCall(sig)
}
case *types.PkgName:
kind = protocol.ModuleCompletion
@@ -153,6 +158,10 @@
}
detail = strings.TrimPrefix(detail, "untyped ")
+ // override computed detail with provided detail, if something is provided.
+ if cand.detail != "" {
+ detail = cand.detail
+ }
item := CompletionItem{
Label: label,
InsertText: insert,
@@ -207,6 +216,7 @@
if c.opts.fullDocumentation {
item.Documentation = hover.FullDocumentation
}
+
return item, nil
}
diff --git a/internal/lsp/source/completion/completion_package.go b/internal/lsp/source/completion/completion_package.go
index f8d3431..a6d9feb 100644
--- a/internal/lsp/source/completion/completion_package.go
+++ b/internal/lsp/source/completion/completion_package.go
@@ -189,6 +189,8 @@
return errors.New("cursor is not in package name identifier")
}
+ c.completionContext.packageCompletion = true
+
prefix := name.Name[:cursor]
packageSuggestions, err := packageSuggestions(ctx, c.snapshot, fileURI, prefix)
if err != nil {
@@ -196,9 +198,7 @@
}
for _, pkg := range packageSuggestions {
- if item, err := c.item(ctx, pkg); err == nil {
- c.items = append(c.items, item)
- }
+ c.deepState.enqueue(pkg)
}
return nil
}
@@ -220,7 +220,7 @@
toCandidate := func(name string, score float64) candidate {
obj := types.NewPkgName(0, nil, name, types.NewPackage("", name))
- return candidate{obj: obj, name: name, score: score}
+ return candidate{obj: obj, name: name, detail: name, score: score}
}
matcher := fuzzy.NewMatcher(prefix)
diff --git a/internal/lsp/source/completion/deep_completion.go b/internal/lsp/source/completion/deep_completion.go
index da8c117..61b29c3 100644
--- a/internal/lsp/source/completion/deep_completion.go
+++ b/internal/lsp/source/completion/deep_completion.go
@@ -130,8 +130,10 @@
}
// If obj is not accessible because it lives in another package and is
- // not exported, don't treat it as a completion candidate.
- if obj.Pkg() != nil && obj.Pkg() != c.pkg.GetTypes() && !obj.Exported() {
+ // not exported, don't treat it as a completion candidate unless it's
+ // a package completion candidate.
+ if !c.completionContext.packageCompletion &&
+ obj.Pkg() != nil && obj.Pkg() != c.pkg.GetTypes() && !obj.Exported() {
continue
}
@@ -262,19 +264,9 @@
}
cand.name = strings.Join(append(cand.names, cand.obj.Name()), ".")
- matchScore := c.matcher.Score(cand.name)
- if matchScore > 0 {
- cand.score *= float64(matchScore)
-
- // Avoid calling c.item() for deep candidates that wouldn't be in the top
- // MaxDeepCompletions anyway.
- if len(cand.path) == 0 || c.deepState.isHighScore(cand.score) {
- if item, err := c.item(ctx, *cand); err == nil {
- c.items = append(c.items, item)
- }
- }
+ if item, err := c.item(ctx, *cand); err == nil {
+ c.items = append(c.items, item)
}
-
}
// penalty reports a score penalty for cand in the range (0, 1).
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index 890d8e0..ed0252c 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -116,10 +116,7 @@
} else {
name = "func"
}
- s, err := NewSignature(ctx, snapshot, pkg, pgf.File, name, sig, comment, qf)
- if err != nil {
- return nil, 0, err
- }
+ s := NewSignature(ctx, snapshot, pkg, pgf.File, name, sig, comment, qf)
paramInfo := make([]protocol.ParameterInformation, 0, len(s.params))
for _, p := range s.params {
paramInfo = append(paramInfo, protocol.ParameterInformation{Label: p})
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 27c4a1d..38fdb31 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -301,7 +301,7 @@
list, surrounding, err := completion.Completion(r.ctx, r.snapshot, fh, protocol.Position{
Line: float64(src.Start().Line() - 1),
Character: float64(src.Start().Column() - 1),
- }, "")
+ }, protocol.CompletionContext{})
if err != nil && !errors.As(err, &completion.ErrIsDefinition{}) {
t.Fatalf("failed for %v: %v", src, err)
}
diff --git a/internal/lsp/source/types_format.go b/internal/lsp/source/types_format.go
index 07784c1..9da19c8 100644
--- a/internal/lsp/source/types_format.go
+++ b/internal/lsp/source/types_format.go
@@ -160,7 +160,7 @@
}
// NewSignature returns formatted signature for a types.Signature struct.
-func NewSignature(ctx context.Context, s Snapshot, pkg Package, file *ast.File, name string, sig *types.Signature, comment *ast.CommentGroup, qf types.Qualifier) (*signature, error) {
+func NewSignature(ctx context.Context, s Snapshot, pkg Package, file *ast.File, name string, sig *types.Signature, comment *ast.CommentGroup, qf types.Qualifier) *signature {
params := make([]string, 0, sig.Params().Len())
for i := 0; i < sig.Params().Len(); i++ {
el := sig.Params().At(i)
@@ -198,7 +198,7 @@
results: results,
variadic: sig.Variadic(),
needResultParens: needResultParens,
- }, nil
+ }
}
// FormatVarType formats a *types.Var, accounting for type aliases.
diff --git a/internal/lsp/testdata/comment_completion/comment_completion.go.in b/internal/lsp/testdata/comment_completion/comment_completion.go.in
index e48b581..dbca0ff 100644
--- a/internal/lsp/testdata/comment_completion/comment_completion.go.in
+++ b/internal/lsp/testdata/comment_completion/comment_completion.go.in
@@ -32,16 +32,16 @@
// //@complete(" ", constant)
const Constant = "example" //@item(constant, "Constant", "string", "const") //@complete(" ", constant)
-// //@complete(" ", structType, fieldA, fieldB)
+// //@complete(" ", structType, fieldB, fieldA)
type StructType struct { //@item(structType, "StructType", "struct{...}", "struct") //@complete(" ", structType, fieldA, fieldB)
// //@complete(" ", fieldA, structType, fieldB)
A string //@item(fieldA, "A", "string", "field") //@complete(" ", fieldA, structType, fieldB)
b int //@item(fieldB, "b", "int", "field") //@complete(" ", fieldB, structType, fieldA)
}
-// //@complete(" ", method, paramX, resultY, structRecv, fieldA, fieldB)
+// //@complete(" ", method, structRecv, paramX, resultY, fieldB, fieldA)
func (structType *StructType) Method(X int) (Y int) { //@item(structRecv, "structType", "*StructType", "var"),item(method, "Method", "func(X int) (Y int)", "method"),item(paramX, "X", "int", "var"),item(resultY, "Y", "int", "var")
- // //@complete(" ", method, paramX, resultY, structRecv, fieldA, fieldB)
+ // //@complete(" ", method, structRecv, paramX, resultY, fieldB, fieldA)
return
}
