internal/lsp/source: compute imports text edits from scratch

Fix the imports text edits by computing it on first principles. This
fixes at least a couple bugs:

 - Incorrect handling of positions between \r and \n with windows line
   endings.
 - Incorrect computation of edits when the imports source prefix is
   synthetically terminated with just \n, but the actual source has
   \r\n.

This is an unsatisfying solution, necessary because of the interaction
of token.File with file termination (token.File does not capture this
information).

Efforts to fix token.File proved complicated, and this is causing
problems for our users, so I think we should do this for now.

For golang/vscode-go#1489

Change-Id: I235caf3960c7201af93800d65546fbab5c6e3f4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319129
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Rob Findley 2021-05-12 00:21:14 -04:00 committed by Robert Findley
parent 412ee174ef
commit 251092de1b
6 changed files with 131 additions and 10 deletions

View File

@ -14,7 +14,7 @@ import (
func ComputeEdits(uri span.URI, before, after string) (edits []diff.TextEdit, err error) {
// The go-diff library has an unresolved panic (see golang/go#278774).
// TOOD(rstambler): Remove the recover once the issue has been fixed
// TODO(rstambler): Remove the recover once the issue has been fixed
// upstream.
defer func() {
if r := recover(); r != nil {

View File

@ -504,7 +504,6 @@ func doit() {
}
func TestUnimportedCompletion_VSCodeIssue1489(t *testing.T) {
t.Skip("broken due to golang/vscode-go#1489")
testenv.NeedsGo1Point(t, 14)
const src = `
@ -524,8 +523,7 @@ func main() {
}
`
WithOptions(
WindowsLineEndings,
ProxyFiles(proxy),
EditorConfig{WindowsLineEndings: true},
).Run(t, src, func(t *testing.T, env *Env) {
// Trigger unimported completions for the example.com/blah package.
env.OpenFile("main.go")
@ -537,6 +535,10 @@ func main() {
}
env.AcceptCompletion("main.go", pos, completions.Items[0])
env.Await(env.DoneWithChange())
t.Log(env.Editor.BufferText("main.go"))
got := env.Editor.BufferText("main.go")
want := "package main\r\n\r\nimport (\r\n\t\"fmt\"\r\n\t\"math\"\r\n)\r\n\r\nfunc main() {\r\n\tfmt.Println(\"a\")\r\n\tmath.Sqrt(${1:})\r\n}\r\n"
if got != want {
t.Errorf("unimported completion: got %q, want %q", got, want)
}
})
}

View File

@ -0,0 +1,89 @@
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Package lsppos provides utilities for working with LSP positions.
//
// See https://microsoft.github.io/language-server-protocol/specification#textDocuments
// for a description of LSP positions. Notably:
// - Positions are specified by a 0-based line count and 0-based utf-16
// character offset.
// - Positions are line-ending agnostic: there is no way to specify \r|\n or
// \n|. Instead the former maps to the end of the current line, and the
// latter to the start of the next line.
package lsppos
import (
"sort"
"unicode/utf8"
)
type Mapper struct {
nonASCII bool
src []byte
// Start-of-line positions. If src is newline-terminated, the final entry will be empty.
lines []int
}
func NewMapper(src []byte) *Mapper {
m := &Mapper{src: src}
if len(src) == 0 {
return m
}
m.lines = []int{0}
for offset, b := range src {
if b == '\n' {
m.lines = append(m.lines, offset+1)
}
if b >= utf8.RuneSelf {
m.nonASCII = true
}
}
return m
}
func (m *Mapper) Position(offset int) (line, char int) {
if offset < 0 || offset > len(m.src) {
return -1, -1
}
nextLine := sort.Search(len(m.lines), func(i int) bool {
return offset < m.lines[i]
})
if nextLine == 0 {
return -1, -1
}
line = nextLine - 1
start := m.lines[line]
var charOffset int
if m.nonASCII {
charOffset = UTF16len(m.src[start:offset])
} else {
charOffset = offset - start
}
var eol int
if line == len(m.lines)-1 {
eol = len(m.src)
} else {
eol = m.lines[line+1] - 1
}
// Adjustment for line-endings: \r|\n is the same as |\r\n.
if offset == eol && offset > 0 && m.src[offset-1] == '\r' {
charOffset--
}
return line, charOffset
}
func UTF16len(buf []byte) int {
cnt := 0
for _, r := range string(buf) {
cnt++
if r >= 1<<16 {
cnt++
}
}
return cnt
}

View File

@ -178,10 +178,6 @@ func DebugAddress(addr string) RunOption {
})
}
var WindowsLineEndings = optionSetter(func(opts *runConfig) {
opts.editor.WindowsLineEndings = true
})
// SkipLogs skips the buffering of logs during test execution. It is intended
// for long-running stress tests.
func SkipLogs() RunOption {

View File

@ -143,6 +143,7 @@ Suffixes:
// add the additional text edits needed.
if cand.imp != nil {
addlEdits, err := c.importEdits(cand.imp)
if err != nil {
return CompletionItem{}, err
}

View File

@ -19,7 +19,9 @@ import (
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/lsppos"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/span"
)
// Format formats a file with a given range.
@ -177,7 +179,7 @@ func computeFixEdits(snapshot Snapshot, pgf *ParsedGoFile, options *imports.Opti
if err != nil {
return nil, err
}
return ToProtocolEdits(pgf.Mapper, edits)
return ProtocolEditsFromSource([]byte(left), edits, pgf.Mapper.Converter)
}
// importPrefix returns the prefix of the given file content through the final
@ -280,6 +282,37 @@ func computeTextEdits(ctx context.Context, snapshot Snapshot, pgf *ParsedGoFile,
return ToProtocolEdits(pgf.Mapper, edits)
}
// ProtocolEditsFromSource converts text edits to LSP edits using the original
// source.
func ProtocolEditsFromSource(src []byte, edits []diff.TextEdit, converter span.Converter) ([]protocol.TextEdit, error) {
m := lsppos.NewMapper(src)
var result []protocol.TextEdit
for _, edit := range edits {
spn, err := edit.Span.WithOffset(converter)
if err != nil {
return nil, fmt.Errorf("computing offsets: %v", err)
}
startLine, startChar := m.Position(spn.Start().Offset())
endLine, endChar := m.Position(spn.End().Offset())
if startLine < 0 || endLine < 0 {
return nil, fmt.Errorf("out of bound span: %v", spn)
}
pstart := protocol.Position{Line: uint32(startLine), Character: uint32(startChar)}
pend := protocol.Position{Line: uint32(endLine), Character: uint32(endChar)}
if pstart == pend && edit.NewText == "" {
// Degenerate case, which may result from a diff tool wanting to delete
// '\r' in line endings. Filter it out.
continue
}
result = append(result, protocol.TextEdit{
Range: protocol.Range{Start: pstart, End: pend},
NewText: edit.NewText,
})
}
return result, nil
}
func ToProtocolEdits(m *protocol.ColumnMapper, edits []diff.TextEdit) ([]protocol.TextEdit, error) {
if edits == nil {
return nil, nil