From 3dd056a8c5c49942ae3511fdebcf8eea0c685486 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 1 Jun 2022 13:39:54 -0400 Subject: [PATCH] internal/lsp/mod: fix broken assumptions about file base Logic in mod.Hover relied upon ColumnMapper.TokFile being the only file in its FileSet. Fix this to compare offsets with offsets. Also use safetoken.InRange in semantic.go, to fix an incorrect bounds check. Change-Id: Ia18a0799fea1120674e3772efdf0bdb724e554e7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/409934 TryBot-Result: Gopher Robot Run-TryBot: Robert Findley Reviewed-by: Alan Donovan gopls-CI: kokoro --- internal/lsp/mod/hover.go | 5 ++--- internal/lsp/protocol/span.go | 13 ++++++++++++- internal/lsp/semantic.go | 10 +--------- internal/span/utf16.go | 7 +++++++ 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/internal/lsp/mod/hover.go b/internal/lsp/mod/hover.go index 03c36792b2..1461d52edb 100644 --- a/internal/lsp/mod/hover.go +++ b/internal/lsp/mod/hover.go @@ -8,7 +8,6 @@ import ( "bytes" "context" "fmt" - "go/token" "strings" "golang.org/x/mod/modfile" @@ -39,7 +38,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, if err != nil { return nil, fmt.Errorf("getting modfile handle: %w", err) } - pos, err := pm.Mapper.Pos(position) + offset, err := pm.Mapper.Offset(position) if err != nil { return nil, fmt.Errorf("computing cursor position: %w", err) } @@ -57,7 +56,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, // 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) <= pos && pos <= token.Pos(endPos) { + if startPos <= offset && offset <= endPos { req = r break } diff --git a/internal/lsp/protocol/span.go b/internal/lsp/protocol/span.go index bb30aeb31e..744746d353 100644 --- a/internal/lsp/protocol/span.go +++ b/internal/lsp/protocol/span.go @@ -105,7 +105,7 @@ func (m *ColumnMapper) RangeToSpanRange(r Range) (span.Range, error) { return spn.Range(m.TokFile) } -// Pos returns the token.Pos corresponding to the given protocol position. +// Pos returns the token.Pos of p within the mapped file. func (m *ColumnMapper) Pos(p Position) (token.Pos, error) { start, err := m.Point(p) if err != nil { @@ -123,6 +123,17 @@ func (m *ColumnMapper) Pos(p Position) (token.Pos, error) { return rng.Start, nil } +// Offset returns the utf-8 byte offset of p within the mapped file. +func (m *ColumnMapper) Offset(p Position) (int, error) { + start, err := m.Point(p) + if err != nil { + return 0, err + } + return start.Offset(), nil +} + +// Point returns a span.Point for p within the mapped file. The resulting point +// always has an Offset. func (m *ColumnMapper) Point(p Position) (span.Point, error) { line := int(p.Line) + 1 offset, err := span.ToOffset(m.TokFile, line, 1) diff --git a/internal/lsp/semantic.go b/internal/lsp/semantic.go index 7ff8b6c20e..286d2fd160 100644 --- a/internal/lsp/semantic.go +++ b/internal/lsp/semantic.go @@ -248,7 +248,7 @@ func (e *encoded) strStack() string { loc := e.stack[len(e.stack)-1].Pos() if !safetoken.InRange(e.pgf.Tok, loc) { msg = append(msg, fmt.Sprintf("invalid position %v for %s", loc, e.pgf.URI)) - } else if locInRange(e.pgf.Tok, loc) { + } else if safetoken.InRange(e.pgf.Tok, loc) { add := e.pgf.Tok.PositionFor(loc, false) nm := filepath.Base(add.Filename) msg = append(msg, fmt.Sprintf("(%s:%d,col:%d)", nm, add.Line, add.Column)) @@ -260,14 +260,6 @@ func (e *encoded) strStack() string { return strings.Join(msg, " ") } -// avoid panic in token.PositionFor() when typing at the end of the file -// -// TODO: this looks wrong: the second check should be int(loc) <= tf.Base()+tf.Size()? -// Can we just use safetoken.InRange? -func locInRange(tf *token.File, loc token.Pos) bool { - return tf.Base() <= int(loc) && int(loc) < tf.Base()+tf.Size() -} - // find the line in the source func (e *encoded) srcLine(x ast.Node) string { file := e.pgf.Tok diff --git a/internal/span/utf16.go b/internal/span/utf16.go index dce2e8e878..f4c93a6ead 100644 --- a/internal/span/utf16.go +++ b/internal/span/utf16.go @@ -54,6 +54,13 @@ func ToUTF16Column(p Point, content []byte) (int, error) { // supplied line contents. // This is used to convert from the utf16 counts used by some editors to the // native (always in bytes) column representation. +// +// The resulting Point always has an offset. +// +// TODO: it looks like this may incorrectly confer a "position" to the +// resulting Point, when it shouldn't. If p.HasPosition() == false, the +// resulting Point will return p.HasPosition() == true, but have the wrong +// position. func FromUTF16Column(p Point, chr int, content []byte) (Point, error) { if !p.HasOffset() { return Point{}, fmt.Errorf("FromUTF16Column: point is missing offset")