diff --git a/gopls/internal/hooks/diff.go b/gopls/internal/hooks/diff.go index 46d7dd74bd..a307ba77fd 100644 --- a/gopls/internal/hooks/diff.go +++ b/gopls/internal/hooks/diff.go @@ -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 { diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go index cd70ccabb9..795f7ae1fb 100644 --- a/gopls/internal/regtest/completion/completion_test.go +++ b/gopls/internal/regtest/completion/completion_test.go @@ -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) + } }) } diff --git a/internal/lsp/lsppos/lsppos.go b/internal/lsp/lsppos/lsppos.go new file mode 100644 index 0000000000..f27bde5737 --- /dev/null +++ b/internal/lsp/lsppos/lsppos.go @@ -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 +} diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 6b3501c1e1..05867c4f97 100644 --- a/internal/lsp/regtest/runner.go +++ b/internal/lsp/regtest/runner.go @@ -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 { diff --git a/internal/lsp/source/completion/format.go b/internal/lsp/source/completion/format.go index 5a20633f7c..166ba553bb 100644 --- a/internal/lsp/source/completion/format.go +++ b/internal/lsp/source/completion/format.go @@ -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 } diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index ca76d2080e..0d61172a24 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -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