internal/lsp/fake: add rename file support for testing

This CL implements the fake.Editor.RenameFile method, which mimic's the
behavior of VS Code when renaming files. Specifically:
- open buffers affected by the renaming are closed, and then reopened
  with new URIs
- files are moved on disk
- didChangeWatchedFile notifications are sent

Along the way, add missing comments and fix one place where the editor
mutex was held while sending notifications (in Editor.CreateBuffer).
Generally, the editor should not hold any mutex while making a remote
call.

For golang/go#41567

Change-Id: I2abfa846e923de566a21c096502a68f125e7e671
Reviewed-on: https://go-review.googlesource.com/c/tools/+/427903
Auto-Submit: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: findleyr

gopls/internal/lsp/fake/client.go

@@ -121,14 +121,8 @@ func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceE
 		return &protocol.ApplyWorkspaceEditResult{FailureReason: "Edit.Changes is unsupported"}, nil
 	}
 	for _, change := range params.Edit.DocumentChanges {
-		// Todo: Add a handler for RenameFile edits
-		if change.RenameFile != nil {
-			panic("Fake editor does not support the RenameFile edits.")
-		}
-		if change.TextDocumentEdit != nil {
-			if err := c.editor.applyProtocolEdit(ctx, *change.TextDocumentEdit); err != nil {
-				return nil, err
-			}
+		if err := c.editor.applyDocumentChange(ctx, change); err != nil {
+			return nil, err
 		}
 	}
 	return &protocol.ApplyWorkspaceEditResult{Applied: true}, nil

gopls/internal/lsp/fake/editor.go

@@ -16,10 +16,11 @@ import (
 	"strings"
 	"sync"
 
-	"golang.org/x/tools/internal/jsonrpc2"
-	"golang.org/x/tools/internal/jsonrpc2/servertest"
 	"golang.org/x/tools/gopls/internal/lsp/command"
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
+	"golang.org/x/tools/gopls/internal/lsp/source"
+	"golang.org/x/tools/internal/jsonrpc2"
+	"golang.org/x/tools/internal/jsonrpc2/servertest"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/xcontext"
 )
@@ -39,7 +40,7 @@ type Editor struct {
 
 	mu                 sync.Mutex                  // guards config, buffers, serverCapabilities
 	config             EditorConfig                // editor configuration
-	buffers            map[string]buffer           // open buffers
+	buffers            map[string]buffer           // open buffers (relative path -> buffer content)
 	serverCapabilities protocol.ServerCapabilities // capabilities / options
 
 	// Call metrics for the purpose of expectations. This is done in an ad-hoc
@@ -50,16 +51,18 @@ type Editor struct {
 	calls   CallCounts
 }
 
+// CallCounts tracks the number of protocol notifications of different types.
 type CallCounts struct {
 	DidOpen, DidChange, DidSave, DidChangeWatchedFiles, DidClose uint64
 }
 
+// buffer holds information about an open buffer in the editor.
 type buffer struct {
-	windowsLineEndings bool
-	version            int
-	path               string
-	lines              []string
-	dirty              bool
+	windowsLineEndings bool     // use windows line endings when merging lines
+	version            int      // monotonic version; incremented on edits
+	path               string   // relative path in the workspace
+	lines              []string // line content
+	dirty              bool     // if true, content is unsaved (TODO(rfindley): rename this field)
 }
 
 func (b buffer) text() string {
@@ -374,7 +377,6 @@ 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 {
 	e.mu.Lock()
-	defer e.mu.Unlock()
 
 	buf := buffer{
 		windowsLineEndings: e.config.WindowsLineEndings,
@@ -385,13 +387,25 @@ func (e *Editor) createBuffer(ctx context.Context, path string, dirty bool, cont
 	}
 	e.buffers[path] = buf
 
-	item := protocol.TextDocumentItem{
+	item := e.textDocumentItem(buf)
+	e.mu.Unlock()
+
+	return e.sendDidOpen(ctx, item)
+}
+
+// textDocumentItem builds a protocol.TextDocumentItem for the given buffer.
+//
+// Precondition: e.mu must be held.
+func (e *Editor) textDocumentItem(buf buffer) protocol.TextDocumentItem {
+	return protocol.TextDocumentItem{
 		URI:        e.sandbox.Workdir.URI(buf.path),
 		LanguageID: languageID(buf.path, e.config.FileAssociations),
 		Version:    int32(buf.version),
 		Text:       buf.text(),
 	}
+}
 
+func (e *Editor) sendDidOpen(ctx context.Context, item protocol.TextDocumentItem) error {
 	if e.Server != nil {
 		if err := e.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{
 			TextDocument: item,
@@ -451,9 +465,13 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error {
 	delete(e.buffers, path)
 	e.mu.Unlock()
 
+	return e.sendDidClose(ctx, e.TextDocumentIdentifier(path))
+}
+
+func (e *Editor) sendDidClose(ctx context.Context, doc protocol.TextDocumentIdentifier) error {
 	if e.Server != nil {
 		if err := e.Server.DidClose(ctx, &protocol.DidCloseTextDocumentParams{
-			TextDocument: e.TextDocumentIdentifier(path),
+			TextDocument: doc,
 		}); err != nil {
 			return fmt.Errorf("DidClose: %w", err)
 		}
@@ -1157,16 +1175,91 @@ func (e *Editor) Rename(ctx context.Context, path string, pos Pos, newName strin
 		return err
 	}
 	for _, change := range wsEdits.DocumentChanges {
-		if change.TextDocumentEdit != nil {
-			if err := e.applyProtocolEdit(ctx, *change.TextDocumentEdit); err != nil {
-				return err
-			}
+		if err := e.applyDocumentChange(ctx, change); err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
-func (e *Editor) applyProtocolEdit(ctx context.Context, change protocol.TextDocumentEdit) error {
+func (e *Editor) RenameFile(ctx context.Context, oldPath, newPath string) error {
+	closed, opened, err := e.renameBuffers(ctx, oldPath, newPath)
+	if err != nil {
+		return err
+	}
+
+	for _, c := range closed {
+		if err := e.sendDidClose(ctx, c); err != nil {
+			return err
+		}
+	}
+	for _, o := range opened {
+		if err := e.sendDidOpen(ctx, o); err != nil {
+			return err
+		}
+	}
+
+	// Finally, perform the renaming on disk.
+	return e.sandbox.Workdir.RenameFile(ctx, oldPath, newPath)
+}
+
+// renameBuffers renames in-memory buffers affected by the renaming of
+// oldPath->newPath, returning the resulting text documents that must be closed
+// and opened over the LSP.
+func (e *Editor) renameBuffers(ctx context.Context, oldPath, newPath string) (closed []protocol.TextDocumentIdentifier, opened []protocol.TextDocumentItem, _ error) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	// In case either oldPath or newPath is absolute, convert to absolute paths
+	// before checking for containment.
+	oldAbs := e.sandbox.Workdir.AbsPath(oldPath)
+	newAbs := e.sandbox.Workdir.AbsPath(newPath)
+
+	// Collect buffers that are affected by the given file or directory renaming.
+	buffersToRename := make(map[string]string) // old path -> new path
+
+	for path := range e.buffers {
+		abs := e.sandbox.Workdir.AbsPath(path)
+		if oldAbs == abs || source.InDirLex(oldAbs, abs) {
+			rel, err := filepath.Rel(oldAbs, abs)
+			if err != nil {
+				return nil, nil, fmt.Errorf("filepath.Rel(%q, %q): %v", oldAbs, abs, err)
+			}
+			nabs := filepath.Join(newAbs, rel)
+			newPath := e.sandbox.Workdir.RelPath(nabs)
+			buffersToRename[path] = newPath
+		}
+	}
+
+	// Update buffers, and build protocol changes.
+	for old, new := range buffersToRename {
+		buf := e.buffers[old]
+		delete(e.buffers, old)
+		buf.version = 1
+		buf.path = new
+		e.buffers[new] = buf
+
+		closed = append(closed, e.TextDocumentIdentifier(old))
+		opened = append(opened, e.textDocumentItem(buf))
+	}
+
+	return closed, opened, nil
+}
+
+func (e *Editor) applyDocumentChange(ctx context.Context, change protocol.DocumentChanges) error {
+	if change.RenameFile != nil {
+		oldPath := e.sandbox.Workdir.URIToPath(change.RenameFile.OldURI)
+		newPath := e.sandbox.Workdir.URIToPath(change.RenameFile.NewURI)
+
+		return e.RenameFile(ctx, oldPath, newPath)
+	}
+	if change.TextDocumentEdit != nil {
+		return e.applyTextDocumentEdit(ctx, *change.TextDocumentEdit)
+	}
+	panic("Internal error: one of RenameFile or TextDocumentEdit must be set")
+}
+
+func (e *Editor) applyTextDocumentEdit(ctx context.Context, change protocol.TextDocumentEdit) error {
 	path := e.sandbox.Workdir.URIToPath(change.TextDocument.URI)
 	if ver := int32(e.BufferVersion(path)); ver != change.TextDocument.Version {
 		return fmt.Errorf("buffer versions for %q do not match: have %d, editing %d", path, ver, change.TextDocument.Version)

gopls/internal/lsp/fake/workdir.go

@@ -315,13 +315,39 @@ func (w *Workdir) writeFile(ctx context.Context, path, content string) (FileEven
 	if err := WriteFileData(path, []byte(content), w.RelativeTo); err != nil {
 		return FileEvent{}, err
 	}
+	return w.fileEvent(path, changeType), nil
+}
+
+func (w *Workdir) fileEvent(path string, changeType protocol.FileChangeType) FileEvent {
 	return FileEvent{
 		Path: path,
 		ProtocolEvent: protocol.FileEvent{
 			URI:  w.URI(path),
 			Type: changeType,
 		},
-	}, nil
+	}
+}
+
+// RenameFile performs an on disk-renaming of the workdir-relative oldPath to
+// workdir-relative newPath.
+func (w *Workdir) RenameFile(ctx context.Context, oldPath, newPath string) error {
+	oldAbs := w.AbsPath(oldPath)
+	newAbs := w.AbsPath(newPath)
+
+	if err := os.Rename(oldAbs, newAbs); err != nil {
+		return err
+	}
+
+	// Send synthetic file events for the renaming. Renamed files are handled as
+	// Delete+Create events:
+	// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#fileChangeType
+	events := []FileEvent{
+		w.fileEvent(oldPath, protocol.Deleted),
+		w.fileEvent(newPath, protocol.Created),
+	}
+	w.sendEvents(ctx, events)
+
+	return nil
 }
 
 // listFiles lists files in the given directory, returning a map of relative

gopls/internal/lsp/regtest/wrappers.go

@@ -400,13 +400,22 @@ func (e *Env) References(path string, pos fake.Pos) []protocol.Location {
 	return locations
 }
 
+// Rename wraps Editor.Rename, calling t.Fatal on any error.
 func (e *Env) Rename(path string, pos fake.Pos, newName string) {
 	e.T.Helper()
 	if err := e.Editor.Rename(e.Ctx, path, pos, newName); err != nil {
 		e.T.Fatal(err)
 	}
 }
 
+// RenameFile wraps Editor.RenameFile, calling t.Fatal on any error.
+func (e *Env) RenameFile(oldPath, newPath string) {
+	e.T.Helper()
+	if err := e.Editor.RenameFile(e.Ctx, oldPath, newPath); err != nil {
+		e.T.Fatal(err)
+	}
+}
+
 // Completion executes a completion request on the server.
 func (e *Env) Completion(path string, pos fake.Pos) *protocol.CompletionList {
 	e.T.Helper()

gopls/internal/regtest/misc/rename_test.go

@@ -132,3 +132,71 @@ func main() {
 		}
 	})
 }
+
+// This is a test that rename operation initiated by the editor function as expected.
+func TestRenameFileFromEditor(t *testing.T) {
+	const files = `
+-- go.mod --
+module mod.com
+
+go 1.16
+-- a/a.go --
+package a
+
+const X = 1
+-- a/x.go --
+package a
+
+const X = 2
+-- b/b.go --
+package b
+`
+
+	Run(t, files, func(t *testing.T, env *Env) {
+		// Rename files and verify that diagnostics are affected accordingly.
+
+		// Initially, we should have diagnostics on both X's, for their duplicate declaration.
+		env.Await(
+			OnceMet(
+				InitialWorkspaceLoad,
+				env.DiagnosticAtRegexp("a/a.go", "X"),
+				env.DiagnosticAtRegexp("a/x.go", "X"),
+			),
+		)
+
+		// Moving x.go should make the diagnostic go away.
+		env.RenameFile("a/x.go", "b/x.go")
+		env.Await(
+			OnceMet(
+				env.DoneWithChangeWatchedFiles(),
+				EmptyDiagnostics("a/a.go"),                  // no more duplicate declarations
+				env.DiagnosticAtRegexp("b/b.go", "package"), // as package names mismatch
+			),
+		)
+
+		// Renaming should also work on open buffers.
+		env.OpenFile("b/x.go")
+
+		// Moving x.go back to a/ should cause the diagnostics to reappear.
+		env.RenameFile("b/x.go", "a/x.go")
+		// TODO(rfindley): enable using a OnceMet precondition here. We can't
+		// currently do this because DidClose, DidOpen and DidChangeWatchedFiles
+		// are sent, and it is not easy to use all as a precondition.
+		env.Await(
+			env.DiagnosticAtRegexp("a/a.go", "X"),
+			env.DiagnosticAtRegexp("a/x.go", "X"),
+		)
+
+		// Renaming the entire directory should move both the open and closed file.
+		env.RenameFile("a", "x")
+		env.Await(
+			env.DiagnosticAtRegexp("x/a.go", "X"),
+			env.DiagnosticAtRegexp("x/x.go", "X"),
+		)
+
+		// As a sanity check, verify that x/x.go is open.
+		if text := env.Editor.BufferText("x/x.go"); text == "" {
+			t.Fatal("got empty buffer for x/x.go")
+		}
+	})
+}