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>

gopls/internal/regtest/completion/completion_test.go

@@ -503,3 +503,41 @@ func doit() {
 		}
 	})
 }
+
+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"))
+	})
+}

internal/lsp/cache/view_test.go

@@ -66,7 +66,7 @@ module de
 -- f/g/go.mod --
 module fg
 `
-	dir, err := fake.Tempdir(workspace)
+	dir, err := fake.Tempdir(fake.UnpackTxt(workspace))
 	if err != nil {
 		t.Fatal(err)
 	}

internal/lsp/cache/workspace_test.go

@@ -269,7 +269,7 @@ replace gopls.test => ../../gopls.test2`, false},
 	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)
 			}

internal/lsp/fake/editor.go

@@ -55,14 +55,19 @@ type CallCounts struct {
 }
 
 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 @@ type EditorConfig struct {
 	// 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 @@ func (e *Editor) onFileChanges(ctx context.Context, evts []FileEvent) {
 					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, content string) error {
 
 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 @@ func (e *Editor) createBuffer(ctx context.Context, path string, dirty bool, cont
 	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 @@ var (
 // 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 @@ func regexpRange(content, re string) (Pos, Pos, error) {
 	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) EditBuffer(ctx context.Context, path string, edits []Edit) erro
 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)
 }
 

internal/lsp/fake/editor_test.go

@@ -48,7 +48,7 @@ func main() {
 `
 
 func TestClientEditing(t *testing.T) {
-	ws, err := NewSandbox(&SandboxConfig{Files: exampleProgram})
+	ws, err := NewSandbox(&SandboxConfig{Files: UnpackTxt(exampleProgram)})
 	if err != nil {
 		t.Fatal(err)
 	}

internal/lsp/fake/proxy.go

@@ -12,8 +12,7 @@ import (
 
 // 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
 	}

internal/lsp/fake/sandbox.go

@@ -38,7 +38,7 @@ type SandboxConfig struct {
 	//
 	// 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 @@ type SandboxConfig struct {
 	//
 	// 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 @@ func NewSandbox(config *SandboxConfig) (_ *Sandbox, err error) {
 // 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 @@ func Tempdir(txt string) (string, error) {
 	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 unpackTxt(txt string) map[string][]byte {
 }
 
 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

internal/lsp/fake/workdir.go

@@ -50,7 +50,7 @@ func (r RelativeTo) RelPath(fp string) string {
 }
 
 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 @@ func hashFile(data []byte) string {
 	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)

internal/lsp/fake/workdir_test.go

@@ -30,7 +30,7 @@ func newWorkdir(t *testing.T) (*Workdir, <-chan []FileEvent, func()) {
 		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() {

internal/lsp/lsprpc/lsprpc_test.go

@@ -197,7 +197,7 @@ func main() {
 }`
 
 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)
 	}

internal/lsp/regtest/runner.go

@@ -109,7 +109,7 @@ func Timeout(d time.Duration) RunOption {
 // 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 @@ 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 {
@@ -269,6 +273,12 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
 			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 @@ func singletonServer(ctx context.Context, t *testing.T, optsHook func(*source.Op
 	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