internal/lsp/fake: reflect on-disk changes in clean buffers
If an open buffer is unmodified from the on-disk version, and that
on-disk content changes, VS Code will update the buffer with the change.
This is relevant for our quick fixes that use the go command to make
changes to go.mod/sum -- we want tests to pick up the changes without
needing to close/reopen the buffer.
For whatever reason, VS Code doesn't do this for deletions, and it made
the implementation easier, so I followed suit. I also mimicked its
behavior of sending both in-memory and on-disk change notifications.
Change-Id: I838a64b2f48f3cbe1a86035293923951b53aecf3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267577
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/codelens_test.go b/gopls/internal/regtest/codelens_test.go
index 9dfafde..a5fca84 100644
--- a/gopls/internal/regtest/codelens_test.go
+++ b/gopls/internal/regtest/codelens_test.go
@@ -9,6 +9,7 @@
"strings"
"testing"
+ "golang.org/x/tools/internal/lsp"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/tests"
@@ -110,7 +111,8 @@
runner.Run(t, shouldUpdateDep, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.ExecuteCodeLensCommand("go.mod", source.CommandUpgradeDependency)
- got := env.ReadWorkspaceFile("go.mod")
+ env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1))
+ got := env.Editor.BufferText("go.mod")
const wantGoMod = `module mod.com
go 1.12
@@ -164,7 +166,8 @@
runner.Run(t, shouldRemoveDep, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
env.ExecuteCodeLensCommand("go.mod", source.CommandTidy)
- got := env.ReadWorkspaceFile("go.mod")
+ env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1))
+ got := env.Editor.BufferText("go.mod")
const wantGoMod = `module mod.com
go 1.14
diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go
index 83decca..d3685dd 100644
--- a/gopls/internal/regtest/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics_test.go
@@ -639,7 +639,6 @@
// Test for golang/go#38211.
func Test_Issue38211(t *testing.T) {
- t.Skip("Requires CL 267577 to work without the save hook.")
testenv.NeedsGo1Point(t, 14)
const ardanLabs = `
-- go.mod --
diff --git a/gopls/internal/regtest/modfile_test.go b/gopls/internal/regtest/modfile_test.go
index 69ff5e9..2f897a2 100644
--- a/gopls/internal/regtest/modfile_test.go
+++ b/gopls/internal/regtest/modfile_test.go
@@ -379,10 +379,8 @@
example.com/blah/v2 v2.0.0
)
`
- // We need to read from disk even though go.mod is open -- the regtests
- // currently don't apply on-disk changes to open but unmodified buffers
- // like most editors would.
- if got := env.ReadWorkspaceFile("go.mod"); got != want {
+ env.Await(EmptyDiagnostics("go.mod"))
+ if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("suggested fixes failed:\n%s", tests.Diff(want, got))
}
})
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index 9ef80fe..3cc1c06 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -47,6 +47,7 @@
version int
path string
content []string
+ dirty bool
}
func (b buffer) text() string {
@@ -270,10 +271,28 @@
if e.Server == nil {
return
}
+ e.mu.Lock()
var lspevts []protocol.FileEvent
for _, evt := range evts {
+ // Always send an on-disk change, even for events that seem useless
+ // because they're shadowed by an open buffer.
lspevts = append(lspevts, evt.ProtocolEvent)
+
+ if buf, ok := e.buffers[evt.Path]; ok {
+ // Following VS Code, don't honor deletions or changes to dirty buffers.
+ if buf.dirty || evt.ProtocolEvent.Type == protocol.Deleted {
+ continue
+ }
+
+ content, err := e.sandbox.Workdir.ReadFile(evt.Path)
+ if err != nil {
+ continue // A race with some other operation.
+ }
+ // During shutdown, this call will fail. Ignore the error.
+ _ = e.setBufferContentLocked(ctx, evt.Path, false, strings.Split(content, "\n"), nil)
+ }
}
+ e.mu.Unlock()
e.Server.DidChangeWatchedFiles(ctx, &protocol.DidChangeWatchedFilesParams{
Changes: lspevts,
})
@@ -285,15 +304,7 @@
if err != nil {
return err
}
- return e.CreateBuffer(ctx, path, content)
-}
-
-func newBuffer(path, content string) buffer {
- return buffer{
- version: 1,
- path: path,
- content: strings.Split(content, "\n"),
- }
+ return e.createBuffer(ctx, path, false, content)
}
func textDocumentItem(wd *Workdir, buf buffer) protocol.TextDocumentItem {
@@ -314,7 +325,16 @@
// CreateBuffer creates a new unsaved buffer corresponding to the workdir path,
// containing the given textual content.
func (e *Editor) CreateBuffer(ctx context.Context, path, content string) error {
- buf := newBuffer(path, content)
+ return e.createBuffer(ctx, path, true, content)
+}
+
+func (e *Editor) createBuffer(ctx context.Context, path string, dirty bool, content string) error {
+ buf := buffer{
+ version: 1,
+ path: path,
+ content: strings.Split(content, "\n"),
+ dirty: dirty,
+ }
e.mu.Lock()
e.buffers[path] = buf
item := textDocumentItem(e.sandbox.Workdir, buf)
@@ -396,6 +416,12 @@
if err := e.sandbox.Workdir.WriteFile(ctx, path, content); err != nil {
return errors.Errorf("writing %q: %w", path, err)
}
+
+ e.mu.Lock()
+ buf.dirty = false
+ e.buffers[path] = buf
+ e.mu.Unlock()
+
if e.Server != nil {
params := &protocol.DidSaveTextDocumentParams{
TextDocument: protocol.VersionedTextDocumentIdentifier{
@@ -534,7 +560,7 @@
e.mu.Lock()
defer e.mu.Unlock()
lines := strings.Split(content, "\n")
- return e.setBufferContentLocked(ctx, path, lines, nil)
+ return e.setBufferContentLocked(ctx, path, true, lines, nil)
}
// BufferText returns the content of the buffer with the given name.
@@ -563,16 +589,17 @@
if err != nil {
return err
}
- return e.setBufferContentLocked(ctx, path, content, edits)
+ return e.setBufferContentLocked(ctx, path, true, content, edits)
}
-func (e *Editor) setBufferContentLocked(ctx context.Context, path string, content []string, fromEdits []Edit) error {
+func (e *Editor) setBufferContentLocked(ctx context.Context, path string, dirty bool, content []string, fromEdits []Edit) error {
buf, ok := e.buffers[path]
if !ok {
return fmt.Errorf("unknown buffer %q", path)
}
buf.content = content
buf.version++
+ buf.dirty = dirty
e.buffers[path] = buf
// A simple heuristic: if there is only one edit, send it incrementally.
// Otherwise, send the entire content.
@@ -739,7 +766,16 @@
if !match {
return nil, fmt.Errorf("unsupported command %q", params.Command)
}
- return e.Server.ExecuteCommand(ctx, params)
+ result, err := e.Server.ExecuteCommand(ctx, params)
+ if err != nil {
+ return nil, err
+ }
+ // Some commands use the go command, which writes directly to disk.
+ // For convenience, check for those changes.
+ if err := e.sandbox.Workdir.CheckForFileChanges(ctx); err != nil {
+ return nil, err
+ }
+ return result, nil
}
func convertEdits(protocolEdits []protocol.TextEdit) []Edit {
