Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions integrationtests/tests/rust/rename_symbol/stale_buffer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package rename_symbol_test

import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"

"github.com/isaacphi/mcp-language-server/integrationtests/tests/rust/internal"
"github.com/isaacphi/mcp-language-server/internal/tools"
)

// TestRenameSymbolStaleBuffer reproduces silent file corruption: when a file is
// edited on disk by a non-LSP path after the server opened it, and a rename is
// issued before the server's buffer is re-synced, the server plans the
// WorkspaceEdit against its stale (pre-edit) buffer. Those ranges point at where
// references used to be; applied to the now-larger on-disk file they overwrite
// unrelated text and miss the real references, while the tool reports success.
//
// It runs the real configuration (watcher ON). rust-analyzer is warmed first so
// the rename completes in ~1ms, far inside the watcher's 300ms debounce, and wins
// the race exactly as in production. The ~200x margin keeps it deterministic: if
// the rename ever lost the race it fails loudly with ContentModified rather than
// flake. A correct bridge must sync open files to disk before renaming.
func TestRenameSymbolStaleBuffer(t *testing.T) {
suite := internal.GetTestSuite(t)

ctx, cancel := context.WithTimeout(suite.Context, 30*time.Second)
defer cancel()

for _, f := range []string{
"src/main.rs", "src/types.rs", "src/helper.rs",
"src/consumer.rs", "src/another_consumer.rs", "src/clean.rs",
} {
if err := suite.Client.OpenFile(ctx, filepath.Join(suite.WorkspaceDir, f)); err != nil {
t.Logf("open %s: %v", f, err)
}
}
time.Sleep(3 * time.Second)

// Force rust-analyzer's first (slow) analysis now so the timed rename is fast.
for i := 0; i < 3; i++ {
if _, err := tools.FindReferences(ctx, suite.Client, "SHARED_CONSTANT"); err != nil {
t.Logf("warm-up references: %v", err)
}
}

consumerPath := filepath.Join(suite.WorkspaceDir, "src/consumer.rs")
original, err := os.ReadFile(consumerPath)
if err != nil {
t.Fatalf("read consumer.rs: %v", err)
}

// Edit consumer.rs on disk without notifying the server: prepend marker lines
// so every SHARED_CONSTANT reference shifts down, off the server's stale view.
const markerCount = 8
var markers strings.Builder
for i := 0; i < markerCount; i++ {
fmt.Fprintf(&markers, "// REPRO MARKER %d - must be preserved verbatim\n", i)
}
if err := os.WriteFile(consumerPath, append([]byte(markers.String()), original...), 0644); err != nil {
t.Fatalf("write consumer.rs: %v", err)
}

// Rename the constant at its definition in types.rs (line 78, col 11).
typesPath := filepath.Join(suite.WorkspaceDir, "src/types.rs")
t0 := time.Now()
result, err := tools.RenameSymbol(ctx, suite.Client, typesPath, 78, 11, "RENAMED_CONSTANT")
t.Logf("rename took %v (watcher debounce is 300ms)", time.Since(t0))
if err != nil {
t.Fatalf("RenameSymbol returned error (likely ContentModified, watcher beat the rename): %v", err)
}
t.Logf("rename result: %s", result)

got, err := os.ReadFile(consumerPath)
if err != nil {
t.Fatalf("re-read consumer.rs: %v", err)
}
gotStr := string(got)

// A correct rename replaces both SHARED_CONSTANT references and touches nothing
// else; any other bytes are the stale-position corruption.
want := markers.String() + strings.ReplaceAll(string(original), "SHARED_CONSTANT", "RENAMED_CONSTANT")
if gotStr != want {
t.Errorf("consumer.rs corrupted by a stale-position rename.\n--- got ---\n%s\n--- want ---\n%s", gotStr, want)
}
}
21 changes: 21 additions & 0 deletions internal/lsp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,27 @@ func (c *Client) NotifyChange(ctx context.Context, filepath string) error {
return c.Notify(ctx, "textDocument/didChange", params)
}

// SyncOpenFiles re-sends every open file's current on-disk content to the server
// via didChange. Callers that plan edits from the server's view of a document
// must call this first (see RenameSymbol).
func (c *Client) SyncOpenFiles(ctx context.Context) {
c.openFilesMu.RLock()
uris := make([]string, 0, len(c.openFiles))
for uri := range c.openFiles {
uris = append(uris, uri)
}
c.openFilesMu.RUnlock()

for _, uri := range uris {
path := strings.TrimPrefix(uri, "file://")
if err := c.NotifyChange(ctx, path); err != nil {
// Unreadable (e.g. deleted): leave it stale. ApplyWorkspaceEdit will
// then error on it rather than corrupt it.
lspLogger.Debug("SyncOpenFiles: skipping %s: %v", path, err)
}
}
}

func (c *Client) CloseFile(ctx context.Context, filepath string) error {
uri := fmt.Sprintf("file://%s", filepath)

Expand Down
5 changes: 5 additions & 0 deletions internal/tools/rename-symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ func RenameSymbol(ctx context.Context, client *lsp.Client, filePath string, line
// Skip the PrepareRename check as it might not be supported by all language servers
// Execute the rename directly

// Sync the server's view of every open file to disk before planning the
// rename: an out-of-band on-disk edit after the file was opened otherwise
// leaves a stale buffer, and the rename's ranges land at stale positions.
client.SyncOpenFiles(ctx)

// Execute the rename operation
workspaceEdit, err := client.Rename(ctx, params)
if err != nil {
Expand Down