gopls/internal/span: some cleanups

This change eliminates these redundant helper functions
- cache.rangeFromPositions
- source.LineToRange
- source.ByteOffsetsToRange
and makes various other simplifications and documentation
improvements.

Change-Id: Ic820ab560d71b88bde00b005e3a919334a5d1856
Reviewed-on: https://go-review.googlesource.com/c/tools/+/442015
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This commit is contained in:
Alan Donovan 2022-10-10 22:49:16 -04:00
parent d3752388a2
commit 3beecff0f6
17 changed files with 95 additions and 100 deletions

View File

@ -729,7 +729,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost
if reference == nil {
continue
}
rng, err := rangeFromPositions(pm.Mapper, reference.Start, reference.End)
rng, err := pm.Mapper.OffsetRange(reference.Start.Byte, reference.End.Byte)
if err != nil {
return nil, err
}

View File

@ -392,8 +392,8 @@ func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, fi
}
case source.Mod:
if pmf, err := s.ParseMod(ctx, fh); err == nil {
if pmf.File.Module != nil && pmf.File.Module.Syntax != nil {
rng, _ = rangeFromPositions(pmf.Mapper, pmf.File.Module.Syntax.Start, pmf.File.Module.Syntax.End)
if mod := pmf.File.Module; mod != nil && mod.Syntax != nil {
rng, _ = pmf.Mapper.OffsetRange(mod.Syntax.Start.Byte, mod.Syntax.End.Byte)
}
}
}

View File

@ -79,7 +79,7 @@ func parseModImpl(ctx context.Context, fh source.FileHandle) (*source.ParsedModu
return nil, fmt.Errorf("unexpected parse error type %v", parseErr)
}
for _, mfErr := range mfErrList {
rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
rng, err := m.OffsetRange(mfErr.Pos.Byte, mfErr.Pos.Byte)
if err != nil {
return nil, err
}
@ -155,7 +155,7 @@ func parseWorkImpl(ctx context.Context, fh source.FileHandle) (*source.ParsedWor
return nil, fmt.Errorf("unexpected parse error type %v", parseErr)
}
for _, mfErr := range mfErrList {
rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
rng, err := m.OffsetRange(mfErr.Pos.Byte, mfErr.Pos.Byte)
if err != nil {
return nil, err
}

View File

@ -287,7 +287,7 @@ func modTidyDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.Pars
// unusedDiagnostic returns a source.Diagnostic for an unused require.
func unusedDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, onlyDiagnostic bool) (*source.Diagnostic, error) {
rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
rng, err := m.OffsetRange(req.Syntax.Start.Byte, req.Syntax.End.Byte)
if err != nil {
return nil, err
}
@ -313,7 +313,7 @@ func unusedDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, onlyDiagno
// directnessDiagnostic extracts errors when a dependency is labeled indirect when
// it should be direct and vice versa.
func directnessDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, computeEdits source.DiffFunction) (*source.Diagnostic, error) {
rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
rng, err := m.OffsetRange(req.Syntax.Start.Byte, req.Syntax.End.Byte)
if err != nil {
return nil, err
}
@ -325,8 +325,8 @@ func directnessDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, comput
if comments := req.Syntax.Comment(); comments != nil && len(comments.Suffix) > 0 {
end := comments.Suffix[0].Start
end.LineRune += len(comments.Suffix[0].Token)
end.Byte += len([]byte(comments.Suffix[0].Token))
rng, err = rangeFromPositions(m, comments.Suffix[0].Start, end)
end.Byte += len(comments.Suffix[0].Token)
rng, err = m.OffsetRange(comments.Suffix[0].Start.Byte, end.Byte)
if err != nil {
return nil, err
}
@ -359,7 +359,7 @@ func missingModuleDiagnostic(pm *source.ParsedModule, req *modfile.Require) (*so
if pm.File != nil && pm.File.Module != nil && pm.File.Module.Syntax != nil {
start, end := pm.File.Module.Syntax.Span()
var err error
rng, err = rangeFromPositions(pm.Mapper, start, end)
rng, err = pm.Mapper.OffsetRange(start.Byte, end.Byte)
if err != nil {
return nil, err
}
@ -429,11 +429,7 @@ func missingModuleForImport(file *token.File, m *protocol.ColumnMapper, imp *ast
if req.Syntax == nil {
return nil, fmt.Errorf("no syntax for %v", req)
}
spn, err := span.NewRange(file, imp.Path.Pos(), imp.Path.End()).Span()
if err != nil {
return nil, err
}
rng, err := m.Range(spn)
rng, err := m.PosRange(imp.Path.Pos(), imp.Path.End())
if err != nil {
return nil, err
}
@ -447,14 +443,6 @@ func missingModuleForImport(file *token.File, m *protocol.ColumnMapper, imp *ast
}, nil
}
func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
spn, err := spanFromPositions(m, s, e)
if err != nil {
return protocol.Range{}, err
}
return m.Range(spn)
}
func spanFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (span.Span, error) {
toPoint := func(offset int) (span.Point, error) {
l, c, err := span.ToPosition(m.TokFile, offset)

View File

@ -545,11 +545,7 @@ func (s *Server) checkForOrphanedFile(ctx context.Context, snapshot source.Snaps
if !pgf.File.Name.Pos().IsValid() {
return nil
}
spn, err := span.NewRange(pgf.Tok, pgf.File.Name.Pos(), pgf.File.Name.End()).Span()
if err != nil {
return nil
}
rng, err := pgf.Mapper.Range(spn)
rng, err := pgf.Mapper.PosRange(pgf.File.Name.Pos(), pgf.File.Name.End())
if err != nil {
return nil
}

View File

@ -360,26 +360,18 @@ func foldRanges(m *protocol.ColumnMapper, contents string, ranges []protocol.Fol
res := contents
// Apply the edits from the end of the file forward
// to preserve the offsets
// TODO(adonovan): factor to use diff.ApplyEdits, which validates the input.
for i := len(ranges) - 1; i >= 0; i-- {
fRange := ranges[i]
spn, err := m.RangeSpan(protocol.Range{
Start: protocol.Position{
Line: fRange.StartLine,
Character: fRange.StartCharacter,
},
End: protocol.Position{
Line: fRange.EndLine,
Character: fRange.EndCharacter,
},
})
r := ranges[i]
start, err := m.Point(protocol.Position{r.StartLine, r.StartCharacter})
if err != nil {
return "", err
}
start := spn.Start().Offset()
end := spn.End().Offset()
tmp := res[0:start] + foldedText
res = tmp + res[end:]
end, err := m.Point(protocol.Position{r.EndLine, r.EndCharacter})
if err != nil {
return "", err
}
res = res[:start.Offset()] + foldedText + res[end.Offset():]
}
return res, nil
}

View File

@ -134,7 +134,7 @@ func moduleStmtRange(fh source.FileHandle, pm *source.ParsedModule) (protocol.Ra
return protocol.Range{}, fmt.Errorf("no module statement in %s", fh.URI())
}
syntax := pm.File.Module.Syntax
return source.LineToRange(pm.Mapper, fh.URI(), syntax.Start, syntax.End)
return pm.Mapper.OffsetRange(syntax.Start.Byte, syntax.End.Byte)
}
// firstRequireRange returns the range for the first "require" in the given
@ -155,7 +155,7 @@ func firstRequireRange(fh source.FileHandle, pm *source.ParsedModule) (protocol.
if start.Byte == 0 || firstRequire.Start.Byte < start.Byte {
start, end = firstRequire.Start, firstRequire.End
}
return source.LineToRange(pm.Mapper, fh.URI(), start, end)
return pm.Mapper.OffsetRange(start.Byte, end.Byte)
}
func vulncheckLenses(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]protocol.CodeLens, error) {

View File

@ -136,7 +136,7 @@ func ModUpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot, fh sou
if !ok || req.Mod.Version == ver {
continue
}
rng, err := source.LineToRange(pm.Mapper, fh.URI(), req.Syntax.Start, req.Syntax.End)
rng, err := pm.Mapper.OffsetRange(req.Syntax.Start.Byte, req.Syntax.End.Byte)
if err != nil {
return nil, err
}
@ -205,7 +205,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
if !ok {
continue
}
rng, err := source.LineToRange(pm.Mapper, fh.URI(), req.Syntax.Start, req.Syntax.End)
rng, err := pm.Mapper.OffsetRange(req.Syntax.Start.Byte, req.Syntax.End.Byte)
if err != nil {
return nil, err
}

View File

@ -11,9 +11,9 @@ import (
"strings"
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/internal/event"
)
func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) (*protocol.Hover, error) {
@ -78,7 +78,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle,
}
// Get the range to highlight for the hover.
rng, err := source.ByteOffsetsToRange(pm.Mapper, fh.URI(), startPos, endPos)
rng, err := pm.Mapper.OffsetRange(startPos, endPos)
if err != nil {
return nil, err
}

View File

@ -4,6 +4,34 @@
// this file contains protocol<->span converters
// Here's a handy guide for your tour of the location zoo:
//
// Imports: source --> lsppos --> protocol --> span --> token
//
// source.MappedRange = (span.Range, protocol.ColumnMapper)
//
// lsppos.TokenMapper = (token.File, lsppos.Mapper)
// lsppos.Mapper = (line offset table, content)
//
// protocol.ColumnMapper = (URI, token.File, content)
// protocol.Location = (URI, protocol.Range)
// protocol.Range = (start, end Position)
// protocol.Position = (line, char uint32) 0-based UTF-16
//
// span.Point = (line?, col?, offset?) 1-based UTF-8
// span.Span = (uri URI, start, end span.Point)
// span.Range = (file token.File, start, end token.Pos)
//
// token.Pos
// token.FileSet
// offset int
//
// TODO(adonovan): simplify this picture. Eliminate the optionality of
// span.{Span,Point}'s position and offset fields: work internally in
// terms of offsets (like span.Range), and require a mapper to convert
// them to protocol (UTF-16) line/col form. Stop honoring //line
// directives.
package protocol
import (
@ -12,6 +40,7 @@ import (
"go/token"
"unicode/utf8"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
"golang.org/x/tools/gopls/internal/span"
)
@ -67,7 +96,7 @@ func (m *ColumnMapper) Range(s span.Span) (Range, error) {
if span.CompareURI(m.URI, s.URI()) != 0 {
return Range{}, fmt.Errorf("column mapper is for file %q instead of %q", m.URI, s.URI())
}
s, err := s.WithAll(m.TokFile)
s, err := s.WithOffset(m.TokFile)
if err != nil {
return Range{}, err
}
@ -97,6 +126,19 @@ func (m *ColumnMapper) OffsetRange(start, end int) (Range, error) {
return Range{Start: startPosition, End: endPosition}, nil
}
// PosRange returns a protocol Range for the token.Pos interval Content[start:end].
func (m *ColumnMapper) PosRange(start, end token.Pos) (Range, error) {
startOffset, err := safetoken.Offset(m.TokFile, start)
if err != nil {
return Range{}, fmt.Errorf("start: %v", err)
}
endOffset, err := safetoken.Offset(m.TokFile, end)
if err != nil {
return Range{}, fmt.Errorf("end: %v", err)
}
return m.OffsetRange(startOffset, endOffset)
}
// Position returns the protocol position for the specified point,
// which must have a byte offset.
func (m *ColumnMapper) Position(p span.Point) (Position, error) {
@ -163,6 +205,8 @@ func (m *ColumnMapper) Span(l Location) (span.Span, error) {
return m.RangeSpan(l.Range)
}
// RangeSpan converts a UTF-16 range to a Span with both the
// position (line/col) and offset fields populated.
func (m *ColumnMapper) RangeSpan(r Range) (span.Span, error) {
start, err := m.Point(r.Start)
if err != nil {
@ -183,22 +227,13 @@ func (m *ColumnMapper) RangeToSpanRange(r Range) (span.Range, error) {
return spn.Range(m.TokFile)
}
// Pos returns the token.Pos of p within the mapped file.
// Pos returns the token.Pos of protocol position p within the mapped file.
func (m *ColumnMapper) Pos(p Position) (token.Pos, error) {
start, err := m.Point(p)
if err != nil {
return token.NoPos, err
}
// 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
return safetoken.Pos(m.TokFile, start.Offset())
}
// Offset returns the utf-8 byte offset of p within the mapped file.
@ -210,10 +245,12 @@ func (m *ColumnMapper) Offset(p Position) (int, error) {
return start.Offset(), nil
}
// Point returns a span.Point for p within the mapped file. The resulting point
// always has an Offset.
// Point returns a span.Point for the protocol position p within the mapped file.
// The resulting point has a valid Position and Offset.
func (m *ColumnMapper) Point(p Position) (span.Point, error) {
line := int(p.Line) + 1
// Find byte offset of start of containing line.
offset, err := span.ToOffset(m.TokFile, line, 1)
if err != nil {
return span.Point{}, err

View File

@ -93,15 +93,11 @@ func ApplyFix(ctx context.Context, fix string, snapshot Snapshot, fh VersionedFi
if !end.IsValid() {
end = edit.Pos
}
spn, err := span.NewRange(tokFile, edit.Pos, end).Span()
fh, err := snapshot.GetVersionedFile(ctx, span.URIFromPath(tokFile.Name()))
if err != nil {
return nil, err
}
fh, err := snapshot.GetVersionedFile(ctx, spn.URI())
if err != nil {
return nil, err
}
te, ok := editsPerFile[spn.URI()]
te, ok := editsPerFile[fh.URI()]
if !ok {
te = &protocol.TextDocumentEdit{
TextDocument: protocol.OptionalVersionedTextDocumentIdentifier{
@ -111,13 +107,13 @@ func ApplyFix(ctx context.Context, fix string, snapshot Snapshot, fh VersionedFi
},
},
}
editsPerFile[spn.URI()] = te
editsPerFile[fh.URI()] = te
}
_, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage)
if err != nil {
return nil, err
}
rng, err := pgf.Mapper.Range(spn)
rng, err := pgf.Mapper.PosRange(edit.Pos, end)
if err != nil {
return nil, err
}

View File

@ -17,7 +17,6 @@ import (
"strconv"
"strings"
"golang.org/x/mod/modfile"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/bug"
@ -26,6 +25,9 @@ import (
// MappedRange provides mapped protocol.Range for a span.Range, accounting for
// UTF-16 code points.
//
// TOOD(adonovan): stop treating //line directives specially, and
// eliminate this type. All callers need either m, or a protocol.Range.
type MappedRange struct {
spanRange span.Range // the range in the compiled source (package.CompiledGoFiles)
m *protocol.ColumnMapper // a mapper of the edited source (package.GoFiles)
@ -555,26 +557,6 @@ func IsCommandLineArguments(s string) bool {
return strings.Contains(s, "command-line-arguments")
}
// LineToRange creates a Range spanning start and end.
func LineToRange(m *protocol.ColumnMapper, uri span.URI, start, end modfile.Position) (protocol.Range, error) {
return ByteOffsetsToRange(m, uri, start.Byte, end.Byte)
}
// ByteOffsetsToRange creates a range spanning start and end.
func ByteOffsetsToRange(m *protocol.ColumnMapper, uri span.URI, start, end int) (protocol.Range, error) {
line, col, err := span.ToPosition(m.TokFile, start)
if err != nil {
return protocol.Range{}, err
}
s := span.NewPoint(line, col, start)
line, col, err = span.ToPosition(m.TokFile, end)
if err != nil {
return protocol.Range{}, err
}
e := span.NewPoint(line, col, end)
return m.Range(span.New(uri, s, e))
}
// RecvIdent returns the type identifier of a method receiver.
// e.g. A for all of A, *A, A[T], *A[T], etc.
func RecvIdent(recv *ast.FieldList) *ast.Ident {

View File

@ -356,6 +356,9 @@ func (s *Server) applyIncrementalChanges(ctx context.Context, uri span.URI, chan
return nil, fmt.Errorf("%w: file not found (%v)", jsonrpc2.ErrInternal, err)
}
for _, change := range changes {
// TODO(adonovan): refactor to use diff.Apply, which is robust w.r.t.
// out-of-order or overlapping changes---and much more efficient.
// Make sure to update column mapper along with the content.
m := protocol.NewColumnMapper(uri, content)
if change.Range == nil {
@ -365,9 +368,6 @@ func (s *Server) applyIncrementalChanges(ctx context.Context, uri span.URI, chan
if err != nil {
return nil, err
}
if !spn.HasOffset() {
return nil, fmt.Errorf("%w: invalid range for content change", jsonrpc2.ErrInternal)
}
start, end := spn.Start().Offset(), spn.End().Offset()
if end < start {
return nil, fmt.Errorf("%w: invalid range for content change", jsonrpc2.ErrInternal)

View File

@ -59,7 +59,7 @@ func DiagnosticsForWork(ctx context.Context, snapshot source.Snapshot, fh source
// Add diagnostic if a directory does not contain a module.
var diagnostics []*source.Diagnostic
for _, use := range pw.File.Use {
rng, err := source.LineToRange(pw.Mapper, fh.URI(), use.Syntax.Start, use.Syntax.End)
rng, err := pw.Mapper.OffsetRange(use.Syntax.Start.Byte, use.Syntax.End.Byte)
if err != nil {
return nil, err
}

View File

@ -11,9 +11,9 @@ import (
"go/token"
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/internal/event"
)
func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) (*protocol.Hover, error) {
@ -56,7 +56,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle,
mod := pm.File.Module.Mod
// Get the range to highlight for the hover.
rng, err := source.ByteOffsetsToRange(pw.Mapper, fh.URI(), pathStart, pathEnd)
rng, err := pw.Mapper.OffsetRange(pathStart, pathEnd)
if err != nil {
return nil, err
}

View File

@ -209,7 +209,8 @@ func (s Span) Format(f fmt.State, c rune) {
}
}
func (s Span) WithPosition(tf *token.File) (Span, error) {
// (Currently unused, but we gain little yet by deleting it.)
func (s Span) withPosition(tf *token.File) (Span, error) {
if err := s.update(tf, true, false); err != nil {
return Span{}, err
}

View File

@ -13,6 +13,9 @@ import (
// supplied file contents.
// This is used to convert from the native (always in bytes) column
// representation and the utf16 counts used by some editors.
//
// TODO(adonovan): this function is unused except by its test. Delete,
// or consolidate with (*protocol.ColumnMapper).utf16Column.
func ToUTF16Column(p Point, content []byte) (int, error) {
if !p.HasPosition() {
return -1, fmt.Errorf("ToUTF16Column: point is missing position")