mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp/source: remove workaround for newline terminated files 

Eliminate the need to work around newline terminated files in
completion, by storing selection ranges as token.Pos and using an
lsppos.TokenMapper derived from the file content, which does not have
problems with newline termination.

This simplifies the completion logic, and removes the last use of
MappedRange.SpanRange, which is an inconisitent API in that it returns
positions in the compiled source, rather than edited source.

Change-Id: I65232787956325172b48fd42d85cbb598039ee5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407889
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Robert Findley 2022-05-23 23:01:49 -04:00
parent 6b760fce76
commit de3ef4aa45
5 changed files with 41 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
internal/lsp/completion.go
View File

@ -5,19 +5,18 @@
package lsp
import (
"bytes"
"context"
"fmt"
"strings"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/lsppos"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/source/completion"
"golang.org/x/tools/internal/lsp/template"
"golang.org/x/tools/internal/lsp/work"
"golang.org/x/tools/internal/span"
)
func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) {
@ -56,55 +55,20 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
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
}
// internal/span treats end of file as the beginning of the next line, even
// when it's not newline-terminated. We correct for that behaviour here if
// end of file is not newline-terminated. See golang/go#41029.
// Map positions to LSP positions using the original content, rather than
// internal/span, as the latter treats end of file as the beginning of the
// next line, even when it's not newline-terminated. See golang/go#41029 for
// more details.
src, err := fh.Read()
if err != nil {
return nil, err
}
numLines := len(bytes.Split(src, []byte("\n")))
tok := snapshot.FileSet().File(surrounding.Start())
eof := tok.Pos(tok.Size())
// For newline-terminated files, the line count reported by go/token should
// be lower than the actual number of lines we see when splitting by \n. If
// they're the same, the file isn't newline-terminated.
if tok.Size() > 0 && tok.LineCount() == numLines {
// Get the span for the last character in the file-1. This is
// technically incorrect, but will get span to point to the previous
// line.
spn, err := span.NewRange(snapshot.FileSet(), eof-1, eof-1).Span()
if err != nil {
return nil, err
}
m := &protocol.ColumnMapper{
URI: fh.URI(),
Converter: span.NewContentConverter(fh.URI().Filename(), src),
Content: src,
}
eofRng, err := m.Range(spn)
if err != nil {
return nil, err
}
// Instead of using the computed range, correct for our earlier
// position adjustment by adding 1 to the column, not the line number.
pos := protocol.Position{
Line: eofRng.Start.Line,
Character: eofRng.Start.Character + 1,
}
if surrounding.Start() >= eof {
rng.Start = pos
}
if surrounding.End() >= eof {
rng.End = pos
}
tf := snapshot.FileSet().File(surrounding.Start())
mapper := lsppos.NewTokenMapper(src, tf)
rng, err := mapper.Range(surrounding.Start(), surrounding.End())
if err != nil {
return nil, err
}
// When using deep completions/fuzzy matching, report results as incomplete so

30
internal/lsp/source/completion/completion.go
View File

@ -29,6 +29,7 @@ import (
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/typeparams"
)
@ -289,7 +290,7 @@ type completionContext struct {
type Selection struct {
content string
cursor token.Pos
source.MappedRange
rng span.Range
}
func (p Selection) Content() string {
@ -297,19 +298,19 @@ func (p Selection) Content() string {
}
func (p Selection) Start() token.Pos {
return p.MappedRange.SpanRange().Start
return p.rng.Start
}
func (p Selection) End() token.Pos {
return p.MappedRange.SpanRange().End
return p.rng.End
}
func (p Selection) Prefix() string {
return p.content[:p.cursor-p.SpanRange().Start]
return p.content[:p.cursor-p.rng.Start]
}
func (p Selection) Suffix() string {
return p.content[p.cursor-p.SpanRange().Start:]
return p.content[p.cursor-p.rng.Start:]
}
func (c *completer) setSurrounding(ident *ast.Ident) {
@ -324,7 +325,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) {
content: ident.Name,
cursor: c.pos,
// Overwrite the prefix only.
MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper, ident.Pos(), ident.End()),
rng: span.NewRange(c.snapshot.FileSet(), ident.Pos(), ident.End()),
}
c.setMatcherFromPrefix(c.surrounding.Prefix())
@ -344,9 +345,9 @@ func (c *completer) setMatcherFromPrefix(prefix string) {
func (c *completer) getSurrounding() *Selection {
if c.surrounding == nil {
c.surrounding = &Selection{
content: "",
cursor: c.pos,
MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper, c.pos, c.pos),
content: "",
cursor: c.pos,
rng: span.NewRange(c.snapshot.FileSet(), c.pos, c.pos),
}
}
return c.surrounding
@ -491,7 +492,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
qual := types.RelativeTo(pkg.GetTypes())
objStr = types.ObjectString(obj, qual)
}
ans, sel := definition(path, obj, snapshot.FileSet(), pgf.Mapper, fh)
ans, sel := definition(path, obj, snapshot.FileSet(), fh)
if ans != nil {
sort.Slice(ans, func(i, j int) bool {
return ans[i].Score > ans[j].Score
@ -801,9 +802,9 @@ func (c *completer) populateImportCompletions(ctx context.Context, searchImport
}
c.surrounding = &Selection{
content: content,
cursor: c.pos,
MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper, start, end),
content: content,
cursor: c.pos,
rng: span.NewRange(c.snapshot.FileSet(), start, end),
}
seenImports := make(map[string]struct{})
@ -1023,8 +1024,7 @@ func (c *completer) setSurroundingForComment(comments *ast.CommentGroup) {
c.surrounding = &Selection{
content: cursorComment.Text[start:end],
cursor: c.pos,
MappedRange: source.NewMappedRange(c.snapshot.FileSet(), c.mapper,
token.Pos(int(cursorComment.Slash)+start), token.Pos(int(cursorComment.Slash)+end)),
rng: span.NewRange(c.snapshot.FileSet(), token.Pos(int(cursorComment.Slash)+start), token.Pos(int(cursorComment.Slash)+end)),
}
c.setMatcherFromPrefix(c.surrounding.Prefix())
}

9
internal/lsp/source/completion/definition.go
View File

@ -15,6 +15,7 @@ import (
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
)
// some definitions can be completed
@ -22,7 +23,7 @@ import (
// BenchmarkFoo(b *testing.B), FuzzFoo(f *testing.F)
// path[0] is known to be *ast.Ident
func definition(path []ast.Node, obj types.Object, fset *token.FileSet, mapper *protocol.ColumnMapper, fh source.FileHandle) ([]CompletionItem, *Selection) {
func definition(path []ast.Node, obj types.Object, fset *token.FileSet, fh source.FileHandle) ([]CompletionItem, *Selection) {
if _, ok := obj.(*types.Func); !ok {
return nil, nil // not a function at all
}
@ -37,9 +38,9 @@ func definition(path []ast.Node, obj types.Object, fset *token.FileSet, mapper *
}
pos := path[0].Pos()
sel := &Selection{
content: "",
cursor: pos,
MappedRange: source.NewMappedRange(fset, mapper, pos, pos),
content: "",
cursor: pos,
rng: span.NewRange(fset, pos, pos),
}
var ans []CompletionItem

27
internal/lsp/source/completion/package.go
View File

@ -45,7 +45,7 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh
return nil, nil, err
}
surrounding, err := packageCompletionSurrounding(ctx, snapshot.FileSet(), pgf, rng.Start)
surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), pgf, rng.Start)
if err != nil {
return nil, nil, fmt.Errorf("invalid position for package completion: %w", err)
}
@ -72,7 +72,7 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh
// packageCompletionSurrounding returns surrounding for package completion if a
// package completions can be suggested at a given position. A valid location
// for package completion is above any declarations or import statements.
func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf *source.ParsedGoFile, pos token.Pos) (*Selection, error) {
func packageCompletionSurrounding(fset *token.FileSet, pgf *source.ParsedGoFile, pos token.Pos) (*Selection, error) {
// If the file lacks a package declaration, the parser will return an empty
// AST. As a work-around, try to parse an expression from the file contents.
filename := pgf.URI.Filename()
@ -96,11 +96,6 @@ func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf
return nil, fmt.Errorf("cursor out of bounds")
}
cursor := tok.Pos(offset)
m := &protocol.ColumnMapper{
URI: pgf.URI,
Content: pgf.Src,
Converter: span.NewContentConverter(filename, pgf.Src),
}
// If we were able to parse out an identifier as the first expression from
// the file, it may be the beginning of a package declaration ("pack ").
@ -111,9 +106,9 @@ func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf
return nil, fmt.Errorf("cursor in non-matching ident")
}
return &Selection{
content: name.Name,
cursor: cursor,
MappedRange: source.NewMappedRange(fset, m, name.Pos(), name.End()),
content: name.Name,
cursor: cursor,
rng: span.NewRange(fset, name.Pos(), name.End()),
}, nil
}
}
@ -148,9 +143,9 @@ func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf
// otherwise fallback to the general case.
if cursor >= start && cursor <= end {
return &Selection{
content: content,
cursor: cursor,
MappedRange: source.NewMappedRange(fset, m, start, end),
content: content,
cursor: cursor,
rng: span.NewRange(fset, start, end),
}, nil
}
}
@ -175,9 +170,9 @@ func packageCompletionSurrounding(ctx context.Context, fset *token.FileSet, pgf
}
return &Selection{
content: "",
cursor: cursor,
MappedRange: source.NewMappedRange(fset, m, start, end),
content: "",
cursor: cursor,
rng: span.NewRange(fset, start, end),
}, nil
}

8
internal/lsp/source/util.go
View File

@ -84,14 +84,6 @@ func (s MappedRange) Span() (span.Span, error) {
return span.FileSpan(s.spanRange.TokFile, s.m.Converter, s.spanRange.Start, s.spanRange.End)
}
// SpanRange returns a span.Range in the compiled source.
//
// See the documentation of NewMappedRange for information on edited vs
// compiled source.
func (s MappedRange) SpanRange() span.Range {
return s.spanRange
}
// URI returns the URI of the edited file.
//
// See the documentation of NewMappedRange for information on edited vs