From 547ecf7b1ef191ac7fb91078460aa55f0fb4416d Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 16 Aug 2019 17:05:40 -0400 Subject: [PATCH] internal/lsp: use protocol.Range in completion items This change switches Completion to use protocol positions instead of token.Pos. Change-Id: I012ce03c9316d8363938dd0156f485982b7e04fe Reviewed-on: https://go-review.googlesource.com/c/tools/+/190600 Reviewed-by: Ian Cottrell Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/cmd/check_test.go | 1 - internal/lsp/code_action.go | 6 +- internal/lsp/completion.go | 71 ++++++-------------- internal/lsp/format.go | 20 +----- internal/lsp/rename.go | 2 +- internal/lsp/source/completion.go | 85 ++++++++++++++++++------ internal/lsp/source/completion_format.go | 12 ++-- internal/lsp/source/format.go | 19 ++++++ internal/lsp/source/source_test.go | 21 +++--- internal/lsp/source/util.go | 29 ++++++++ 10 files changed, 157 insertions(+), 109 deletions(-) diff --git a/internal/lsp/cmd/check_test.go b/internal/lsp/cmd/check_test.go index 7f39ee83f9..d918f0da35 100644 --- a/internal/lsp/cmd/check_test.go +++ b/internal/lsp/cmd/check_test.go @@ -52,7 +52,6 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { got[l] = struct{}{} } for _, diag := range want { - // TODO: This is a hack, fix this. expect := fmt.Sprintf("%v:%v:%v: %v", diag.URI.Filename(), diag.Range.Start.Line+1, diag.Range.Start.Character+1, diag.Message) if diag.Range.Start.Character == 0 { expect = fmt.Sprintf("%v:%v: %v", diag.URI.Filename(), diag.Range.Start.Line+1, diag.Message) diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 3491e36beb..2c6f065624 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -133,14 +133,14 @@ func organizeImports(ctx context.Context, view source.View, s span.Span) ([]prot return nil, nil, err } // Convert all source edits to protocol edits. - pEdits, err := ToProtocolEdits(m, edits) + pEdits, err := source.ToProtocolEdits(m, edits) if err != nil { return nil, nil, err } pEditsPerFix := make([]*protocolImportFix, len(editsPerFix)) for i, fix := range editsPerFix { - pEdits, err := ToProtocolEdits(m, fix.Edits) + pEdits, err := source.ToProtocolEdits(m, fix.Edits) if err != nil { return nil, nil, err } @@ -241,7 +241,7 @@ func quickFixes(ctx context.Context, view source.View, gof source.GoFile) ([]pro if err != nil { return nil, err } - edits, err := ToProtocolEdits(m, ca.Edits) + edits, err := source.ToProtocolEdits(m, ca.Edits) if err != nil { return nil, err } diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index bbfe73bf3d..899cf6f723 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -23,64 +23,42 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara if err != nil { return nil, err } - m, err := getMapper(ctx, f) - if err != nil { - return nil, err - } - spn, err := m.PointSpan(params.Position) - if err != nil { - return nil, err - } - rng, err := spn.Range(m.Converter) - if err != nil { - return nil, err - } - candidates, surrounding, err := source.Completion(ctx, view, f, rng.Start, source.CompletionOptions{ + candidates, surrounding, err := source.Completion(ctx, view, f, params.Position, source.CompletionOptions{ DeepComplete: s.useDeepCompletions, WantDocumentaton: s.wantCompletionDocumentation, WantFullDocumentation: s.hoverKind == fullDocumentation, WantUnimported: s.wantUnimportedCompletions, }) if err != nil { - log.Print(ctx, "no completions found", tag.Of("At", rng), tag.Of("Failure", err)) + log.Print(ctx, "no completions found", tag.Of("At", params.Position), tag.Of("Failure", err)) + } + if candidates == nil { + return &protocol.CompletionList{ + Items: []protocol.CompletionItem{}, + }, nil + } + // We might need to adjust the position to account for the prefix. + rng, err := surrounding.Range() + if err != nil { + return nil, err } - return &protocol.CompletionList{ - // When using deep completions/fuzzy matching, report results as incomplete so - // client fetches updated completions after every key stroke. - IsIncomplete: s.useDeepCompletions, - Items: s.toProtocolCompletionItems(ctx, view, m, candidates, params.Position, surrounding), - }, nil -} - -func (s *Server) toProtocolCompletionItems(ctx context.Context, view source.View, m *protocol.ColumnMapper, candidates []source.CompletionItem, pos protocol.Position, surrounding *source.Selection) []protocol.CompletionItem { // Sort the candidates by score, since that is not supported by LSP yet. sort.SliceStable(candidates, func(i, j int) bool { return candidates[i].Score > candidates[j].Score }) - // We might need to adjust the position to account for the prefix. - insertionRange := protocol.Range{ - Start: pos, - End: pos, - } - if surrounding != nil { - spn, err := surrounding.Range.Span() - if err != nil { - log.Print(ctx, "failed to get span for surrounding position: %s:%v:%v: %v", tag.Of("Position", pos), tag.Of("Failure", err)) - } else { - rng, err := m.Range(spn) - if err != nil { - log.Print(ctx, "failed to convert surrounding position", tag.Of("Position", pos), tag.Of("Failure", err)) - } else { - insertionRange = rng - } - } - } + return &protocol.CompletionList{ + // When using deep completions/fuzzy matching, report results as incomplete so + // client fetches updated completions after every key stroke. + IsIncomplete: s.useDeepCompletions, + Items: s.toProtocolCompletionItems(candidates, rng), + }, nil +} +func (s *Server) toProtocolCompletionItems(candidates []source.CompletionItem, rng protocol.Range) []protocol.CompletionItem { var ( items = make([]protocol.CompletionItem, 0, len(candidates)) numDeepCompletionsSeen int ) - for i, candidate := range candidates { // Limit the number of deep completions to not overwhelm the user in cases // with dozens of deep completion matches. @@ -97,21 +75,16 @@ func (s *Server) toProtocolCompletionItems(ctx context.Context, view source.View if s.insertTextFormat == protocol.SnippetTextFormat { insertText = candidate.Snippet(s.usePlaceholders) } - addlEdits, err := ToProtocolEdits(m, candidate.AdditionalTextEdits) - if err != nil { - log.Error(ctx, "failed to convert to protocol edits", err) - continue - } item := protocol.CompletionItem{ Label: candidate.Label, Detail: candidate.Detail, Kind: toProtocolCompletionItemKind(candidate.Kind), TextEdit: &protocol.TextEdit{ NewText: insertText, - Range: insertionRange, + Range: rng, }, InsertTextFormat: s.insertTextFormat, - AdditionalTextEdits: addlEdits, + AdditionalTextEdits: candidate.AdditionalTextEdits, // This is a hack so that the client sorts completion results in the order // according to their score. This can be removed upon the resolution of // https://github.com/Microsoft/language-server-protocol/issues/348. diff --git a/internal/lsp/format.go b/internal/lsp/format.go index 95c9fd32f3..d819afd248 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -25,7 +25,7 @@ func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormat if err != nil { return nil, err } - return ToProtocolEdits(m, edits) + return source.ToProtocolEdits(m, edits) } func spanToRange(ctx context.Context, view source.View, spn span.Span) (source.GoFile, *protocol.ColumnMapper, span.Range, error) { @@ -52,24 +52,6 @@ func spanToRange(ctx context.Context, view source.View, spn span.Span) (source.G return f, m, rng, nil } -func ToProtocolEdits(m *protocol.ColumnMapper, edits []diff.TextEdit) ([]protocol.TextEdit, error) { - if edits == nil { - return nil, nil - } - result := make([]protocol.TextEdit, len(edits)) - for i, edit := range edits { - rng, err := m.Range(edit.Span) - if err != nil { - return nil, err - } - result[i] = protocol.TextEdit{ - Range: rng, - NewText: edit.NewText, - } - } - return result, nil -} - func FromProtocolEdits(m *protocol.ColumnMapper, edits []protocol.TextEdit) ([]diff.TextEdit, error) { if edits == nil { return nil, nil diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go index 8fb5e2cb8f..0aca1e262b 100644 --- a/internal/lsp/rename.go +++ b/internal/lsp/rename.go @@ -49,7 +49,7 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr if err != nil { return nil, err } - protocolEdits, err := ToProtocolEdits(m, textEdits) + protocolEdits, err := source.ToProtocolEdits(m, textEdits) if err != nil { return nil, err } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index d6d6c288dc..ebde3a2d36 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -13,8 +13,8 @@ import ( "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/internal/imports" - "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/fuzzy" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/snippet" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" @@ -43,7 +43,7 @@ type CompletionItem struct { // Additional text edits should be used to change text unrelated to the current cursor position // (for example adding an import statement at the top of the file if the completion item will // insert an unqualified type). - AdditionalTextEdits []diff.TextEdit + AdditionalTextEdits []protocol.TextEdit // Depth is how many levels were searched to find this completion. // For example when completing "foo<>", "fooBar" is depth 0, and @@ -199,6 +199,9 @@ type completer struct { // expensive and can be called many times for the same type while searching // for deep completions. methodSetCache map[methodSetKey]*types.MethodSet + + // mapper converts the positions in the file from which the completion originated. + mapper *protocol.ColumnMapper } type compLitInfo struct { @@ -229,13 +232,13 @@ type methodSetKey struct { // A Selection represents the cursor position and surrounding identifier. type Selection struct { - Content string - Range span.Range - Cursor token.Pos + content string + cursor token.Pos + mappedRange } func (p Selection) Prefix() string { - return p.Content[:p.Cursor-p.Range.Start] + return p.content[:p.cursor-p.spanRange.Start] } func (c *completer) setSurrounding(ident *ast.Ident) { @@ -246,9 +249,12 @@ func (c *completer) setSurrounding(ident *ast.Ident) { return } c.surrounding = &Selection{ - Content: ident.Name, - Range: span.NewRange(c.view.Session().Cache().FileSet(), ident.Pos(), ident.End()), - Cursor: c.pos, + content: ident.Name, + cursor: c.pos, + mappedRange: mappedRange{ + spanRange: span.NewRange(c.view.Session().Cache().FileSet(), ident.Pos(), ident.End()), + m: c.mapper, + }, } // Fuzzy matching shares the "useDeepCompletions" config flag, so if deep completions @@ -260,6 +266,20 @@ func (c *completer) setSurrounding(ident *ast.Ident) { } } +func (c *completer) getSurrounding() *Selection { + if c.surrounding == nil { + c.surrounding = &Selection{ + content: "", + cursor: c.pos, + mappedRange: mappedRange{ + spanRange: span.NewRange(c.view.Session().Cache().FileSet(), c.pos, c.pos), + m: c.mapper, + }, + } + } + return c.surrounding +} + // found adds a candidate completion. We will also search through the object's // members for more candidates. func (c *completer) found(obj types.Object, score float64, imp *imports.ImportInfo) { @@ -351,27 +371,51 @@ type CompletionOptions struct { // 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, view View, f GoFile, pos token.Pos, opts CompletionOptions) ([]CompletionItem, *Selection, error) { +func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position, opts CompletionOptions) ([]CompletionItem, *Selection, error) { ctx, done := trace.StartSpan(ctx, "source.Completion") defer done() - file, err := f.GetAST(ctx, ParseFull) + pkg, err := f.GetPackage(ctx) + if err != nil { + return nil, nil, err + } + var ph ParseGoHandle + for _, h := range pkg.GetHandles() { + if h.File().Identity().URI == f.URI() { + ph = h + } + } + file, err := ph.Cached(ctx) if file == nil { return nil, nil, err } - pkg, err := f.GetPackage(ctx) + data, _, err := ph.File().Read(ctx) + if err != nil { + return nil, nil, err + } + fset := view.Session().Cache().FileSet() + tok := fset.File(file.Pos()) + if tok == nil { + return nil, nil, errors.Errorf("no token.File for %s", f.URI()) + } + m := protocol.NewColumnMapper(f.URI(), f.URI().Filename(), fset, tok, data) + spn, err := m.PointSpan(pos) + if err != nil { + return nil, nil, err + } + rng, err := spn.Range(m.Converter) if err != nil { return nil, nil, err } // Completion is based on what precedes the cursor. // Find the path to the position before pos. - path, _ := astutil.PathEnclosingInterval(file, pos-1, pos-1) + path, _ := astutil.PathEnclosingInterval(file, rng.Start-1, rng.Start-1) if path == nil { return nil, nil, errors.Errorf("cannot find node enclosing position") } // Skip completion inside comments. for _, g := range file.Comments { - if g.Pos() <= pos && pos <= g.End() { + if g.Pos() <= rng.Start && rng.Start <= g.End() { return nil, nil, nil } } @@ -380,7 +424,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co return nil, nil, nil } - clInfo := enclosingCompositeLiteral(path, pos, pkg.GetTypesInfo()) + clInfo := enclosingCompositeLiteral(path, rng.Start, pkg.GetTypesInfo()) c := &completer{ types: pkg.GetTypes(), info: pkg.GetTypesInfo(), @@ -390,14 +434,15 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co filename: f.URI().Filename(), file: file, path: path, - pos: pos, + pos: rng.Start, seen: make(map[types.Object]bool), - enclosingFunction: enclosingFunction(path, pos, pkg.GetTypesInfo()), + enclosingFunction: enclosingFunction(path, rng.Start, pkg.GetTypesInfo()), enclosingCompositeLiteral: clInfo, opts: opts, // default to a matcher that always matches matcher: prefixMatcher(""), methodSetCache: make(map[methodSetKey]*types.MethodSet), + mapper: m, } c.deepState.enabled = opts.DeepComplete @@ -414,7 +459,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co if err := c.structLiteralFieldName(); err != nil { return nil, nil, err } - return c.items, c.surrounding, nil + return c.items, c.getSurrounding(), nil } switch n := path[0].(type) { @@ -424,7 +469,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co if err := c.selector(sel); err != nil { return nil, nil, err } - return c.items, c.surrounding, nil + return c.items, c.getSurrounding(), nil } // reject defining identifiers if obj, ok := pkg.GetTypesInfo().Defs[n]; ok { @@ -472,7 +517,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts Co } } - return c.items, c.surrounding, nil + return c.items, c.getSurrounding(), nil } func (c *completer) wantStructFieldCompletions() bool { diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 55e8675cc6..0ceaf5aee1 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -13,7 +13,7 @@ import ( "go/types" "strings" - "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/snippet" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" @@ -36,7 +36,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { kind CompletionItemKind plainSnippet *snippet.Builder placeholderSnippet *snippet.Builder - addlEdits []diff.TextEdit + protocolEdits []protocol.TextEdit ) // expandFuncCall mutates the completion label, detail, and snippets @@ -94,14 +94,18 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { if err != nil { return CompletionItem{}, err } - addlEdits = append(addlEdits, edit...) + addlEdits, err := ToProtocolEdits(c.mapper, edit) + if err != nil { + return CompletionItem{}, err + } + protocolEdits = append(protocolEdits, addlEdits...) } detail = strings.TrimPrefix(detail, "untyped ") item := CompletionItem{ Label: label, InsertText: insert, - AdditionalTextEdits: addlEdits, + AdditionalTextEdits: protocolEdits, Detail: detail, Kind: kind, Score: cand.score, diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index a1448cd9af..ae49558c03 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -14,6 +14,7 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/trace" @@ -234,3 +235,21 @@ func computeTextEdits(ctx context.Context, file File, formatted string) (edits [ } return diff.ComputeEdits(file.URI(), string(data), formatted) } + +func ToProtocolEdits(m *protocol.ColumnMapper, edits []diff.TextEdit) ([]protocol.TextEdit, error) { + if edits == nil { + return nil, nil + } + result := make([]protocol.TextEdit, len(edits)) + for i, edit := range edits { + rng, err := m.Range(edit.Span) + if err != nil { + return nil, err + } + result[i] = protocol.TextEdit{ + Range: rng, + NewText: edit.NewText, + } + } + return result, nil +} diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 854edaa1b7..4b1ea4c1fd 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -18,6 +18,7 @@ import ( "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/fuzzy" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/span" @@ -87,14 +88,12 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests if err != nil { t.Fatalf("failed for %v: %v", src, err) } - tok, err := f.(source.GoFile).GetToken(ctx) - if err != nil { - t.Fatalf("failed to get token for %s: %v", src.URI(), err) - } - pos := tok.Pos(src.Start().Offset()) deepComplete := strings.Contains(string(src.URI()), "deepcomplete") unimported := strings.Contains(string(src.URI()), "unimported") - list, surrounding, err := source.Completion(ctx, r.view, f.(source.GoFile), pos, source.CompletionOptions{ + list, surrounding, err := source.Completion(ctx, r.view, f.(source.GoFile), protocol.Position{ + Line: float64(src.Start().Line() - 1), + Character: float64(src.Start().Column() - 1), + }, source.CompletionOptions{ DeepComplete: deepComplete, WantDocumentaton: true, WantUnimported: unimported, @@ -144,12 +143,10 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests if err != nil { t.Fatalf("failed for %v: %v", src, err) } - tok, err := f.(source.GoFile).GetToken(ctx) - if err != nil { - t.Fatalf("failed to get token for %s: %v", src.URI(), err) - } - pos := tok.Pos(src.Start().Offset()) - list, _, err := source.Completion(ctx, r.view, f.(source.GoFile), pos, source.CompletionOptions{ + list, _, err := source.Completion(ctx, r.view, f.(source.GoFile), protocol.Position{ + Line: float64(src.Start().Line() - 1), + Character: float64(src.Start().Column() - 1), + }, source.CompletionOptions{ DeepComplete: strings.Contains(string(src.URI()), "deepcomplete"), }) if err != nil { diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index ad25225e21..6ca1a4e5bc 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -14,9 +14,38 @@ import ( "regexp" "strings" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" ) +type mappedRange struct { + spanRange span.Range + m *protocol.ColumnMapper + + // protocolRange is the result of converting the spanRange using the mapper. + // It is computed on-demand. + protocolRange *protocol.Range +} + +func (s mappedRange) Range() (protocol.Range, error) { + if s.protocolRange == nil { + spn, err := s.spanRange.Span() + if err != nil { + return protocol.Range{}, err + } + prng, err := s.m.Range(spn) + if err != nil { + return protocol.Range{}, err + } + s.protocolRange = &prng + } + return *s.protocolRange, nil +} + +func (s mappedRange) URI() span.URI { + return s.m.URI +} + func IsGenerated(ctx context.Context, view View, uri span.URI) bool { f, err := view.GetFile(ctx, uri) if err != nil {