internal/lsp/source: eliminate ColumnMapper.PointSpan in favor of Pos
It turns out PointSpan was only ever used as part of an oft repeated
pattern to get a token.Pos from a protocol position. Cut out the
middle.
Change-Id: I0b2c0fc3d335e6bbd3c1ac72c6f75e2c40c60ca5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408717
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/internal/lsp/mod/hover.go b/internal/lsp/mod/hover.go
index 96bc0c0..03c3679 100644
--- a/internal/lsp/mod/hover.go
+++ b/internal/lsp/mod/hover.go
@@ -39,14 +39,10 @@
if err != nil {
return nil, fmt.Errorf("getting modfile handle: %w", err)
}
- spn, err := pm.Mapper.PointSpan(position)
+ pos, err := pm.Mapper.Pos(position)
if err != nil {
return nil, fmt.Errorf("computing cursor position: %w", err)
}
- hoverRng, err := spn.Range(pm.Mapper.TokFile)
- if err != nil {
- return nil, fmt.Errorf("computing hover range: %w", err)
- }
// Confirm that the cursor is at the position of a require statement.
var req *modfile.Require
@@ -61,7 +57,7 @@
// Shift the start position to the location of the
// dependency within the require statement.
startPos, endPos = s+i, s+i+len(dep)
- if token.Pos(startPos) <= hoverRng.Start && hoverRng.Start <= token.Pos(endPos) {
+ if token.Pos(startPos) <= pos && pos <= token.Pos(endPos) {
req = r
break
}
diff --git a/internal/lsp/protocol/span.go b/internal/lsp/protocol/span.go
index dac53cb..bb30aeb 100644
--- a/internal/lsp/protocol/span.go
+++ b/internal/lsp/protocol/span.go
@@ -105,12 +105,22 @@
return spn.Range(m.TokFile)
}
-func (m *ColumnMapper) PointSpan(p Position) (span.Span, error) {
+// Pos returns the token.Pos corresponding to the given protocol position.
+func (m *ColumnMapper) Pos(p Position) (token.Pos, error) {
start, err := m.Point(p)
if err != nil {
- return span.Span{}, err
+ return token.NoPos, err
}
- return span.New(m.URI, start, start).WithAll(m.TokFile)
+ // TODO: refactor the span package to avoid creating this unnecessary end position.
+ spn, err := span.New(m.URI, start, start).WithAll(m.TokFile)
+ if err != nil {
+ return token.NoPos, err
+ }
+ rng, err := spn.Range(m.TokFile)
+ if err != nil {
+ return token.NoPos, err
+ }
+ return rng.Start, nil
}
func (m *ColumnMapper) Point(p Position) (span.Point, error) {
diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go
index 11b9497..bb1c68d 100644
--- a/internal/lsp/source/completion/completion.go
+++ b/internal/lsp/source/completion/completion.go
@@ -445,23 +445,17 @@
}
return items, surrounding, nil
}
- spn, err := pgf.Mapper.PointSpan(protoPos)
- if err != nil {
- return nil, nil, err
- }
- rng, err := spn.Range(pgf.Mapper.TokFile)
+ pos, err := pgf.Mapper.Pos(protoPos)
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(pgf.File, rng.Start-1, rng.Start-1)
+ path, _ := astutil.PathEnclosingInterval(pgf.File, pos-1, pos-1)
if path == nil {
return nil, nil, fmt.Errorf("cannot find node enclosing position")
}
- pos := rng.Start
-
// Check if completion at this position is valid. If not, return early.
switch n := path[0].(type) {
case *ast.BasicLit:
@@ -524,7 +518,7 @@
pos: pos,
seen: make(map[types.Object]bool),
enclosingFunc: enclosingFunction(path, pkg.GetTypesInfo()),
- enclosingCompositeLiteral: enclosingCompositeLiteral(path, rng.Start, pkg.GetTypesInfo()),
+ enclosingCompositeLiteral: enclosingCompositeLiteral(path, pos, pkg.GetTypesInfo()),
deepState: deepCompletionState{
enabled: opts.DeepCompletion,
},
diff --git a/internal/lsp/source/completion/package.go b/internal/lsp/source/completion/package.go
index ed9f8fd..21244ef 100644
--- a/internal/lsp/source/completion/package.go
+++ b/internal/lsp/source/completion/package.go
@@ -28,7 +28,7 @@
// packageClauseCompletions offers completions for a package declaration when
// one is not present in the given file.
-func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, pos protocol.Position) ([]CompletionItem, *Selection, error) {
+func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) ([]CompletionItem, *Selection, error) {
// We know that the AST for this file will be empty due to the missing
// package declaration, but parse it anyway to get a mapper.
pgf, err := snapshot.ParseGo(ctx, fh, source.ParseFull)
@@ -36,16 +36,12 @@
return nil, nil, err
}
- cursorSpan, err := pgf.Mapper.PointSpan(pos)
- if err != nil {
- return nil, nil, err
- }
- rng, err := cursorSpan.Range(pgf.Mapper.TokFile)
+ pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, nil, err
}
- surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), pgf, rng.Start)
+ surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), pgf, pos)
if err != nil {
return nil, nil, fmt.Errorf("invalid position for package completion: %w", err)
}
diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go
index a3887af..4be078b 100644
--- a/internal/lsp/source/highlight.go
+++ b/internal/lsp/source/highlight.go
@@ -17,7 +17,7 @@
"golang.org/x/tools/internal/lsp/protocol"
)
-func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) ([]protocol.Range, error) {
+func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) ([]protocol.Range, error) {
ctx, done := event.Start(ctx, "source.Highlight")
defer done()
@@ -33,24 +33,20 @@
return nil, fmt.Errorf("getting file for Highlight: %w", err)
}
- spn, err := pgf.Mapper.PointSpan(pos)
+ pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, err
}
- rng, err := spn.Range(pgf.Mapper.TokFile)
- if err != nil {
- return nil, err
- }
- path, _ := astutil.PathEnclosingInterval(pgf.File, rng.Start, rng.Start)
+ path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos)
if len(path) == 0 {
- return nil, fmt.Errorf("no enclosing position found for %v:%v", int(pos.Line), int(pos.Character))
+ return nil, fmt.Errorf("no enclosing position found for %v:%v", position.Line, position.Character)
}
// If start == end for astutil.PathEnclosingInterval, the 1-char interval
// following start is used instead. As a result, we might not get an exact
// match so we should check the 1-char interval to the left of the passed
// in position to see if that is an exact match.
if _, ok := path[0].(*ast.Ident); !ok {
- if p, _ := astutil.PathEnclosingInterval(pgf.File, rng.Start-1, rng.Start-1); p != nil {
+ if p, _ := astutil.PathEnclosingInterval(pgf.File, pos-1, pos-1); p != nil {
switch p[0].(type) {
case *ast.Ident, *ast.SelectorExpr:
path = p // use preceding ident/selector
diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go
index d764cdd..58ea969 100644
--- a/internal/lsp/source/hover.go
+++ b/internal/lsp/source/hover.go
@@ -142,15 +142,10 @@
if err != nil {
return 0, MappedRange{}, err
}
- spn, err := pgf.Mapper.PointSpan(position)
+ pos, err := pgf.Mapper.Pos(position)
if err != nil {
return 0, MappedRange{}, err
}
- rng, err := spn.Range(pgf.Mapper.TokFile)
- if err != nil {
- return 0, MappedRange{}, err
- }
- pos := rng.Start
// Find the basic literal enclosing the given position, if there is one.
var lit *ast.BasicLit
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index f2938f2..40655e2 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -77,7 +77,7 @@
// Identifier returns identifier information for a position
// in a file, accounting for a potentially incomplete selector.
-func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) (*IdentifierInfo, error) {
+func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) (*IdentifierInfo, error) {
ctx, done := event.Start(ctx, "source.Identifier")
defer done()
@@ -104,16 +104,12 @@
bug.Report("missing package file", bug.Data{"pkg": pkg.ID(), "file": fh.URI()})
return nil, err
}
- spn, err := pgf.Mapper.PointSpan(pos)
- if err != nil {
- return nil, err
- }
- rng, err := spn.Range(pgf.Mapper.TokFile)
+ pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, err
}
var ident *IdentifierInfo
- ident, findErr = findIdentifier(ctx, snapshot, pkg, pgf, rng.Start)
+ ident, findErr = findIdentifier(ctx, snapshot, pkg, pgf, pos)
if findErr == nil {
return ident, nil
}
diff --git a/internal/lsp/source/implementation.go b/internal/lsp/source/implementation.go
index b122a5d..6666605 100644
--- a/internal/lsp/source/implementation.go
+++ b/internal/lsp/source/implementation.go
@@ -227,15 +227,11 @@
if err != nil {
return nil, err
}
- spn, err := pgf.Mapper.PointSpan(pp)
+ pos, err := pgf.Mapper.Pos(pp)
if err != nil {
return nil, err
}
- rng, err := spn.Range(pgf.Mapper.TokFile)
- if err != nil {
- return nil, err
- }
- offset, err := safetoken.Offset(pgf.Tok, rng.Start)
+ offset, err := safetoken.Offset(pgf.Tok, pos)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go
index 1d35931..813f67e 100644
--- a/internal/lsp/source/signature_help.go
+++ b/internal/lsp/source/signature_help.go
@@ -16,7 +16,7 @@
"golang.org/x/tools/internal/lsp/protocol"
)
-func SignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) (*protocol.SignatureInformation, int, error) {
+func SignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) (*protocol.SignatureInformation, int, error) {
ctx, done := event.Start(ctx, "source.SignatureHelp")
defer done()
@@ -24,17 +24,13 @@
if err != nil {
return nil, 0, fmt.Errorf("getting file for SignatureHelp: %w", err)
}
- spn, err := pgf.Mapper.PointSpan(pos)
- if err != nil {
- return nil, 0, err
- }
- rng, err := spn.Range(pgf.Mapper.TokFile)
+ pos, err := pgf.Mapper.Pos(position)
if err != nil {
return nil, 0, err
}
// Find a call expression surrounding the query position.
var callExpr *ast.CallExpr
- path, _ := astutil.PathEnclosingInterval(pgf.File, rng.Start, rng.Start)
+ path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos)
if path == nil {
return nil, 0, fmt.Errorf("cannot find node enclosing position")
}
@@ -42,7 +38,7 @@
for _, node := range path {
switch node := node.(type) {
case *ast.CallExpr:
- if rng.Start >= node.Lparen && rng.Start <= node.Rparen {
+ if pos >= node.Lparen && pos <= node.Rparen {
callExpr = node
break FindCall
}
@@ -77,7 +73,7 @@
// Handle builtin functions separately.
if obj, ok := obj.(*types.Builtin); ok {
- return builtinSignature(ctx, snapshot, callExpr, obj.Name(), rng.Start)
+ return builtinSignature(ctx, snapshot, callExpr, obj.Name(), pos)
}
// Get the type information for the function being called.
@@ -91,7 +87,7 @@
return nil, 0, fmt.Errorf("cannot find signature for Fun %[1]T (%[1]v)", callExpr.Fun)
}
- activeParam := activeParameter(callExpr, sig.Params().Len(), sig.Variadic(), rng.Start)
+ activeParam := activeParameter(callExpr, sig.Params().Len(), sig.Variadic(), pos)
var (
name string
diff --git a/internal/lsp/work/completion.go b/internal/lsp/work/completion.go
index 300d13c..c7227bc 100644
--- a/internal/lsp/work/completion.go
+++ b/internal/lsp/work/completion.go
@@ -28,17 +28,13 @@
if err != nil {
return nil, fmt.Errorf("getting go.work file handle: %w", err)
}
- spn, err := pw.Mapper.PointSpan(position)
+ pos, err := pw.Mapper.Pos(position)
if err != nil {
return nil, fmt.Errorf("computing cursor position: %w", err)
}
- rng, err := spn.Range(pw.Mapper.TokFile)
- if err != nil {
- return nil, fmt.Errorf("computing range: %w", err)
- }
// Find the use statement the user is in.
- cursor := rng.Start - 1
+ cursor := pos - 1
use, pathStart, _ := usePath(pw, cursor)
if use == nil {
return &protocol.CompletionList{}, nil
diff --git a/internal/lsp/work/hover.go b/internal/lsp/work/hover.go
index 5fa6828..8f7822d 100644
--- a/internal/lsp/work/hover.go
+++ b/internal/lsp/work/hover.go
@@ -30,18 +30,14 @@
if err != nil {
return nil, fmt.Errorf("getting go.work file handle: %w", err)
}
- spn, err := pw.Mapper.PointSpan(position)
+ pos, err := pw.Mapper.Pos(position)
if err != nil {
return nil, fmt.Errorf("computing cursor position: %w", err)
}
- hoverRng, err := spn.Range(pw.Mapper.TokFile)
- if err != nil {
- return nil, fmt.Errorf("computing hover range: %w", err)
- }
// Confirm that the cursor is inside a use statement, and then find
// the position of the use statement's directory path.
- use, pathStart, pathEnd := usePath(pw, hoverRng.Start)
+ use, pathStart, pathEnd := usePath(pw, pos)
// The cursor position is not on a use statement.
if use == nil {
