diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index 9ce8dca0f4..9b55f2ff12 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -28,7 +28,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara 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 71403748b9..7e932eb4f7 100644 --- a/internal/lsp/source/completion/completion.go +++ b/internal/lsp/source/completion/completion.go @@ -155,8 +155,9 @@ type completer struct { 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 @@ type methodSetKey struct { 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 @@ type candidate struct { // 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 @@ func (e ErrIsDefinition) Error() string { // 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 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan 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 @@ func (c *completer) collectCompletions(ctx context.Context) error { // 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 @@ func (c *completer) emptySwitchStmt() bool { // (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 @@ func (c *completer) populateImportCompletions(ctx context.Context, searchImport 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) populateImportCompletions(ctx context.Context, searchImport 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast } 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast } 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 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast 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 @@ func (c *completer) addFieldItems(ctx context.Context, fields *ast.FieldList) { 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 @@ func (c *completer) addFieldItems(ctx context.Context, fields *ast.FieldList) { 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) wantStructFieldCompletions() bool { } 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 @@ func (c *candidate) anyCandType(f func(t types.Type, addressable bool) bool) boo // 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 9549f87c17..878bebaeec 100644 --- a/internal/lsp/source/completion/completion_format.go +++ b/internal/lsp/source/completion/completion_format.go @@ -17,12 +17,25 @@ import ( "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 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e // 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 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e } 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 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e } if cand.expandFuncCall { - if err := expandFuncCall(sig); err != nil { - return CompletionItem{}, err - } + expandFuncCall(sig) } case *types.PkgName: kind = protocol.ModuleCompletion @@ -153,6 +158,10 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e } 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 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e 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 f8d3431da2..a6d9febe41 100644 --- a/internal/lsp/source/completion/completion_package.go +++ b/internal/lsp/source/completion/completion_package.go @@ -189,6 +189,8 @@ func (c *completer) packageNameCompletions(ctx context.Context, fileURI span.URI 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 @@ func (c *completer) packageNameCompletions(ctx context.Context, fileURI span.URI } 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 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s 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 da8c117e1d..61b29c3447 100644 --- a/internal/lsp/source/completion/deep_completion.go +++ b/internal/lsp/source/completion/deep_completion.go @@ -130,8 +130,10 @@ outer: } // 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 @@ func (c *completer) addCandidate(ctx context.Context, cand *candidate) { } 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 890d8e0feb..ed0252cfaf 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -116,10 +116,7 @@ FindCall: } 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 27c4a1d97c..38fdb3133e 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -301,7 +301,7 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options func(*sourc 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 07784c1673..9da19c8189 100644 --- a/internal/lsp/source/types_format.go +++ b/internal/lsp/source/types_format.go @@ -160,7 +160,7 @@ func formatFieldList(ctx context.Context, snapshot Snapshot, list *ast.FieldList } // 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 @@ func NewSignature(ctx context.Context, s Snapshot, pkg Package, file *ast.File, 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 e48b581310..dbca0ff175 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 @@ var C string //@item(variableC, "C", "string", "var") //@complete(" ", variableC // //@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 }