diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go index d57a4b237b1..afdd494a8b1 100644 --- a/gopls/internal/regtest/completion/completion_test.go +++ b/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")) + }) +} diff --git a/internal/lsp/cache/view_test.go b/internal/lsp/cache/view_test.go index cb57182353f..802215a5aac 100644 --- a/internal/lsp/cache/view_test.go +++ b/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) } diff --git a/internal/lsp/cache/workspace_test.go b/internal/lsp/cache/workspace_test.go index fd9cb8d1d3c..85240611645 100644 --- a/internal/lsp/cache/workspace_test.go +++ b/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) } diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index de01f0b38a8..501d32c1806 100644 --- a/internal/lsp/fake/editor.go +++ b/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) } diff --git a/internal/lsp/fake/editor_test.go b/internal/lsp/fake/editor_test.go index f1ce7537ae5..3ce5df6e08f 100644 --- a/internal/lsp/fake/editor_test.go +++ b/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) } diff --git a/internal/lsp/fake/proxy.go b/internal/lsp/fake/proxy.go index dbba27d76f7..9e56efeb17f 100644 --- a/internal/lsp/fake/proxy.go +++ b/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 } diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go index c14f9035ac3..34f1ba180c0 100644 --- a/internal/lsp/fake/sandbox.go +++ b/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 diff --git a/internal/lsp/fake/workdir.go b/internal/lsp/fake/workdir.go index 5103bdb4e19..aa9ef84df67 100644 --- a/internal/lsp/fake/workdir.go +++ b/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) diff --git a/internal/lsp/fake/workdir_test.go b/internal/lsp/fake/workdir_test.go index f57ea37e13b..33fbb9fa1d5 100644 --- a/internal/lsp/fake/workdir_test.go +++ b/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() { diff --git a/internal/lsp/lsprpc/lsprpc_test.go b/internal/lsp/lsprpc/lsprpc_test.go index 5fa6b9715f5..ef2555d9f1c 100644 --- a/internal/lsp/lsprpc/lsprpc_test.go +++ b/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) } diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 87cfa6de63a..5eeacd8634e 100644 --- a/internal/lsp/regtest/runner.go +++ b/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