gopls/internal/regtest: add a failing regtest for vscode-go#1489
Unimported completion computes invalid text edits with windows line
endings.
To enable this test, add support for windows line endings in the regtest
framework. Doing this required decoupling the txtar encoding from the
sandbox, which was a good change anyway.
For golang/vscode-go#1489
Change-Id: I6c1075fd38d24090271a7a7f33b11ddd8f9decf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319089
Trust: Robert Findley <rfindley@google.com>
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>
diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index d57a4b2..afdd494 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -503,3 +503,41 @@
}
})
}
+
+func TestUnimportedCompletion_VSCodeIssue1489(t *testing.T) {
+ t.Skip("broken due to golang/vscode-go#1489")
+ testenv.NeedsGo1Point(t, 14)
+
+ const src = `
+-- go.mod --
+module mod.com
+
+go 1.14
+
+-- main.go --
+package main
+
+import "fmt"
+
+func main() {
+ fmt.Println("a")
+ math.Sqr
+}
+`
+ WithOptions(
+ WindowsLineEndings,
+ ProxyFiles(proxy),
+ ).Run(t, src, func(t *testing.T, env *Env) {
+ // Trigger unimported completions for the example.com/blah package.
+ env.OpenFile("main.go")
+ env.Await(env.DoneWithOpen())
+ pos := env.RegexpSearch("main.go", "Sqr()")
+ completions := env.Completion("main.go", pos)
+ if len(completions.Items) == 0 {
+ t.Fatalf("no completion items")
+ }
+ env.AcceptCompletion("main.go", pos, completions.Items[0])
+ env.Await(env.DoneWithChange())
+ t.Log(env.Editor.BufferText("main.go"))
+ })
+}
diff --git a/internal/lsp/cache/view_test.go b/internal/lsp/cache/view_test.go
index cb57182..802215a 100644
--- a/internal/lsp/cache/view_test.go
+++ b/internal/lsp/cache/view_test.go
@@ -66,7 +66,7 @@
-- f/g/go.mod --
module fg
`
- dir, err := fake.Tempdir(workspace)
+ dir, err := fake.Tempdir(fake.UnpackTxt(workspace))
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/cache/workspace_test.go b/internal/lsp/cache/workspace_test.go
index fd9cb8d..8524061 100644
--- a/internal/lsp/cache/workspace_test.go
+++ b/internal/lsp/cache/workspace_test.go
@@ -269,7 +269,7 @@
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
ctx := context.Background()
- dir, err := fake.Tempdir(test.initial)
+ dir, err := fake.Tempdir(fake.UnpackTxt(test.initial))
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index de01f0b..501d32c 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -55,14 +55,19 @@
}
type buffer struct {
- version int
- path string
- lines []string
- dirty bool
+ windowsLineEndings bool
+ version int
+ path string
+ lines []string
+ dirty bool
}
func (b buffer) text() string {
- return strings.Join(b.lines, "\n")
+ eol := "\n"
+ if b.windowsLineEndings {
+ eol = "\r\n"
+ }
+ return strings.Join(b.lines, eol)
}
// EditorConfig configures the editor's LSP session. This is similar to
@@ -106,6 +111,9 @@
// the PID. This can only be set by one test.
SendPID bool
+ // Whether to edit files with windows line endings.
+ WindowsLineEndings bool
+
DirectoryFilters []string
VerboseOutput bool
@@ -338,11 +346,11 @@
continue // A race with some other operation.
}
// No need to update if the buffer content hasn't changed.
- if content == strings.Join(buf.lines, "\n") {
+ if content == buf.text() {
continue
}
// During shutdown, this call will fail. Ignore the error.
- _ = e.setBufferContentLocked(ctx, evt.Path, false, strings.Split(content, "\n"), nil)
+ _ = e.setBufferContentLocked(ctx, evt.Path, false, lines(content), nil)
}
}
e.Server.DidChangeWatchedFiles(ctx, &protocol.DidChangeWatchedFilesParams{
@@ -383,10 +391,11 @@
func (e *Editor) createBuffer(ctx context.Context, path string, dirty bool, content string) error {
buf := buffer{
- version: 1,
- path: path,
- lines: strings.Split(content, "\n"),
- dirty: dirty,
+ windowsLineEndings: e.Config.WindowsLineEndings,
+ version: 1,
+ path: path,
+ lines: lines(content),
+ dirty: dirty,
}
e.mu.Lock()
defer e.mu.Unlock()
@@ -406,6 +415,15 @@
return nil
}
+// lines returns line-ending agnostic line representation of content.
+func lines(content string) []string {
+ lines := strings.Split(content, "\n")
+ for i, l := range lines {
+ lines[i] = strings.TrimSuffix(l, "\r")
+ }
+ return lines
+}
+
// CloseBuffer removes the current buffer (regardless of whether it is saved).
func (e *Editor) CloseBuffer(ctx context.Context, path string) error {
e.mu.Lock()
@@ -528,6 +546,7 @@
// regexpRange returns the start and end of the first occurrence of either re
// or its singular subgroup. It returns ErrNoMatch if the regexp doesn't match.
func regexpRange(content, re string) (Pos, Pos, error) {
+ content = normalizeEOL(content)
var start, end int
rec, err := regexp.Compile(re)
if err != nil {
@@ -558,6 +577,10 @@
return startPos, endPos, nil
}
+func normalizeEOL(content string) string {
+ return strings.Join(lines(content), "\n")
+}
+
// RegexpRange returns the first range in the buffer bufName matching re. See
// RegexpSearch for more information on matching.
func (e *Editor) RegexpRange(bufName, re string) (Pos, Pos, error) {
@@ -615,7 +638,7 @@
func (e *Editor) SetBufferContent(ctx context.Context, path, content string) error {
e.mu.Lock()
defer e.mu.Unlock()
- lines := strings.Split(content, "\n")
+ lines := lines(content)
return e.setBufferContentLocked(ctx, path, true, lines, nil)
}
diff --git a/internal/lsp/fake/editor_test.go b/internal/lsp/fake/editor_test.go
index f1ce753..3ce5df6 100644
--- a/internal/lsp/fake/editor_test.go
+++ b/internal/lsp/fake/editor_test.go
@@ -48,7 +48,7 @@
`
func TestClientEditing(t *testing.T) {
- ws, err := NewSandbox(&SandboxConfig{Files: exampleProgram})
+ ws, err := NewSandbox(&SandboxConfig{Files: UnpackTxt(exampleProgram)})
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/fake/proxy.go b/internal/lsp/fake/proxy.go
index dbba27d..9e56efe 100644
--- a/internal/lsp/fake/proxy.go
+++ b/internal/lsp/fake/proxy.go
@@ -12,8 +12,7 @@
// WriteProxy creates a new proxy file tree using the txtar-encoded content,
// and returns its URL.
-func WriteProxy(tmpdir, txt string) (string, error) {
- files := unpackTxt(txt)
+func WriteProxy(tmpdir string, files map[string][]byte) (string, error) {
type moduleVersion struct {
modulePath, version string
}
diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go
index c14f903..34f1ba1 100644
--- a/internal/lsp/fake/sandbox.go
+++ b/internal/lsp/fake/sandbox.go
@@ -38,7 +38,7 @@
//
// For convenience, the special substring "$SANDBOX_WORKDIR" is replaced with
// the sandbox's resolved working directory before writing files.
- Files string
+ Files map[string][]byte
// InGoPath specifies that the working directory should be within the
// temporary GOPATH.
InGoPath bool
@@ -51,10 +51,9 @@
//
// This option is incompatible with InGoPath or Files.
Workdir string
-
// ProxyFiles holds a txtar-encoded archive of files to populate a file-based
// Go proxy.
- ProxyFiles string
+ ProxyFiles map[string][]byte
// GOPROXY is the explicit GOPROXY value that should be used for the sandbox.
//
// This option is incompatible with ProxyFiles.
@@ -141,12 +140,11 @@
// Tempdir creates a new temp directory with the given txtar-encoded files. It
// is the responsibility of the caller to call os.RemoveAll on the returned
// file path when it is no longer needed.
-func Tempdir(txt string) (string, error) {
+func Tempdir(files map[string][]byte) (string, error) {
dir, err := ioutil.TempDir("", "gopls-tempdir-")
if err != nil {
return "", err
}
- files := unpackTxt(txt)
for name, data := range files {
if err := WriteFileData(name, data, RelativeTo(dir)); err != nil {
return "", errors.Errorf("writing to tempdir: %w", err)
@@ -155,7 +153,7 @@
return dir, nil
}
-func unpackTxt(txt string) map[string][]byte {
+func UnpackTxt(txt string) map[string][]byte {
dataMap := make(map[string][]byte)
archive := txtar.Parse([]byte(txt))
for _, f := range archive.Files {
@@ -165,13 +163,13 @@
}
func validateConfig(config SandboxConfig) error {
- if filepath.IsAbs(config.Workdir) && (config.Files != "" || config.InGoPath) {
+ if filepath.IsAbs(config.Workdir) && (config.Files != nil || config.InGoPath) {
return errors.New("absolute Workdir cannot be set in conjunction with Files or InGoPath")
}
if config.Workdir != "" && config.InGoPath {
return errors.New("Workdir cannot be set in conjunction with InGoPath")
}
- if config.GOPROXY != "" && config.ProxyFiles != "" {
+ if config.GOPROXY != "" && config.ProxyFiles != nil {
return errors.New("GOPROXY cannot be set in conjunction with ProxyFiles")
}
return nil
diff --git a/internal/lsp/fake/workdir.go b/internal/lsp/fake/workdir.go
index 5103bdb..aa9ef84 100644
--- a/internal/lsp/fake/workdir.go
+++ b/internal/lsp/fake/workdir.go
@@ -50,7 +50,7 @@
}
func writeTxtar(txt string, rel RelativeTo) error {
- files := unpackTxt(txt)
+ files := UnpackTxt(txt)
for name, data := range files {
if err := WriteFileData(name, data, rel); err != nil {
return errors.Errorf("writing to workdir: %w", err)
@@ -96,8 +96,7 @@
return fmt.Sprintf("%x", sha256.Sum256(data))
}
-func (w *Workdir) writeInitialFiles(txt string) error {
- files := unpackTxt(txt)
+func (w *Workdir) writeInitialFiles(files map[string][]byte) error {
w.files = map[string]string{}
for name, data := range files {
w.files[name] = hashFile(data)
diff --git a/internal/lsp/fake/workdir_test.go b/internal/lsp/fake/workdir_test.go
index f57ea37..33fbb9f 100644
--- a/internal/lsp/fake/workdir_test.go
+++ b/internal/lsp/fake/workdir_test.go
@@ -30,7 +30,7 @@
t.Fatal(err)
}
wd := NewWorkdir(tmpdir)
- if err := wd.writeInitialFiles(data); err != nil {
+ if err := wd.writeInitialFiles(UnpackTxt(data)); err != nil {
t.Fatal(err)
}
cleanup := func() {
diff --git a/internal/lsp/lsprpc/lsprpc_test.go b/internal/lsp/lsprpc/lsprpc_test.go
index 5fa6b97..ef2555d 100644
--- a/internal/lsp/lsprpc/lsprpc_test.go
+++ b/internal/lsp/lsprpc/lsprpc_test.go
@@ -197,7 +197,7 @@
}`
func TestDebugInfoLifecycle(t *testing.T) {
- sb, err := fake.NewSandbox(&fake.SandboxConfig{Files: exampleProgram})
+ sb, err := fake.NewSandbox(&fake.SandboxConfig{Files: fake.UnpackTxt(exampleProgram)})
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go
index 87cfa6d..5eeacd8 100644
--- a/internal/lsp/regtest/runner.go
+++ b/internal/lsp/regtest/runner.go
@@ -109,7 +109,7 @@
// ProxyFiles configures a file proxy using the given txtar-encoded string.
func ProxyFiles(txt string) RunOption {
return optionSetter(func(opts *runConfig) {
- opts.sandbox.ProxyFiles = txt
+ opts.sandbox.ProxyFiles = fake.UnpackTxt(txt)
})
}
@@ -177,6 +177,10 @@
})
}
+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 {
@@ -269,6 +273,12 @@
if err := os.MkdirAll(rootDir, 0755); err != nil {
t.Fatal(err)
}
+ files := fake.UnpackTxt(files)
+ if config.editor.WindowsLineEndings {
+ for name, data := range files {
+ files[name] = bytes.ReplaceAll(data, []byte("\n"), []byte("\r\n"))
+ }
+ }
config.sandbox.Files = files
config.sandbox.RootDir = rootDir
sandbox, err := fake.NewSandbox(&config.sandbox)
@@ -385,7 +395,7 @@
return lsprpc.NewStreamServer(cache.New(optsHook), false)
}
-func experimentalWorkspaceModule(_ context.Context, t *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
+func experimentalWorkspaceModule(_ context.Context, _ *testing.T, optsHook func(*source.Options)) jsonrpc2.StreamServer {
options := func(o *source.Options) {
optsHook(o)
o.ExperimentalWorkspaceModule = true
