From 61a414df77a49278b6c734b486d7cda419cf781e Mon Sep 17 00:00:00 2001 From: Dino Date: Wed, 26 Nov 2025 19:34:03 +0000 Subject: [PATCH] Fix language server renaming when parent directory does not exist (#43499) Update the `fs::RenameOptions` used by `project::lsp_store::LocalLspStore.deserialize_workspace_edit` in order to always set `create_parents` to `true`. Doing this ensures that we'll always create the folders for the new file path provided by the language server instead of failing to handle the request in case the parent - Introduce `create_parents` field to `fs::RenameOptions` - Update `fs::RealFs.rename` to ensure that the `create_parents` option is respected Closes #41820 Release Notes: - Fixed a bug where using language server's file renaming actions could fail if the parent directory of the new file did not exist --- .../assistant_text_thread/src/text_thread.rs | 1 + crates/fs/src/fs.rs | 73 +++++++++++++++++++ crates/project/src/agent_server_store.rs | 1 + crates/project/src/lsp_store.rs | 28 ++++--- crates/worktree/src/worktree_tests.rs | 2 + 5 files changed, 94 insertions(+), 11 deletions(-) diff --git a/crates/assistant_text_thread/src/text_thread.rs b/crates/assistant_text_thread/src/text_thread.rs index 613c9b862e..7f24c8f665 100644 --- a/crates/assistant_text_thread/src/text_thread.rs +++ b/crates/assistant_text_thread/src/text_thread.rs @@ -2933,6 +2933,7 @@ impl TextThread { RenameOptions { overwrite: true, ignore_if_exists: true, + create_parents: false, }, ) .await?; diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 5a6e4bdfdb..5be94ab630 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -193,6 +193,8 @@ pub struct CopyOptions { pub struct RenameOptions { pub overwrite: bool, pub ignore_if_exists: bool, + /// Whether to create parent directories if they do not exist. + pub create_parents: bool, } #[derive(Copy, Clone, Default)] @@ -579,6 +581,12 @@ impl Fs for RealFs { } } + if options.create_parents { + if let Some(parent) = target.parent() { + self.create_dir(parent).await?; + } + } + smol::fs::rename(source, target).await?; Ok(()) } @@ -2357,6 +2365,12 @@ impl Fs for FakeFs { let old_path = normalize_path(old_path); let new_path = normalize_path(new_path); + if options.create_parents { + if let Some(parent) = new_path.parent() { + self.create_dir(parent).await?; + } + } + let mut state = self.state.lock(); let moved_entry = state.write_path(&old_path, |e| { if let btree_map::Entry::Occupied(e) = e { @@ -3396,4 +3410,63 @@ mod tests { let content = std::fs::read_to_string(&file_to_be_replaced).unwrap(); assert_eq!(content, "Hello"); } + + #[gpui::test] + async fn test_rename(executor: BackgroundExecutor) { + let fs = FakeFs::new(executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + "src": { + "file_a.txt": "content a", + "file_b.txt": "content b" + } + }), + ) + .await; + + fs.rename( + Path::new(path!("/root/src/file_a.txt")), + Path::new(path!("/root/src/new/renamed_a.txt")), + RenameOptions { + create_parents: true, + ..Default::default() + }, + ) + .await + .unwrap(); + + // Assert that the `file_a.txt` file was being renamed and moved to a + // different directory that did not exist before. + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/src/file_b.txt")), + PathBuf::from(path!("/root/src/new/renamed_a.txt")), + ] + ); + + let result = fs + .rename( + Path::new(path!("/root/src/file_b.txt")), + Path::new(path!("/root/src/old/renamed_b.txt")), + RenameOptions { + create_parents: false, + ..Default::default() + }, + ) + .await; + + // Assert that the `file_b.txt` file was not renamed nor moved, as + // `create_parents` was set to `false`. + // different directory that did not exist before. + assert!(result.is_err()); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/src/file_b.txt")), + PathBuf::from(path!("/root/src/new/renamed_a.txt")), + ] + ); + } } diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index d6bd83531e..ef12e22200 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -1089,6 +1089,7 @@ async fn download_latest_version( RenameOptions { ignore_if_exists: true, overwrite: true, + create_parents: false, }, ) .await?; diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 4f7022a264..a69d255369 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -3021,17 +3021,23 @@ impl LocalLspStore { .new_uri .to_file_path() .map_err(|()| anyhow!("can't convert URI to path"))?; - fs.rename( - &source_abs_path, - &target_abs_path, - op.options - .map(|options| fs::RenameOptions { - overwrite: options.overwrite.unwrap_or(false), - ignore_if_exists: options.ignore_if_exists.unwrap_or(false), - }) - .unwrap_or_default(), - ) - .await?; + + let options = fs::RenameOptions { + overwrite: op + .options + .as_ref() + .and_then(|options| options.overwrite) + .unwrap_or(false), + ignore_if_exists: op + .options + .as_ref() + .and_then(|options| options.ignore_if_exists) + .unwrap_or(false), + create_parents: true, + }; + + fs.rename(&source_abs_path, &target_abs_path, options) + .await?; } lsp::DocumentChangeOperation::Op(lsp::ResourceOp::Delete(op)) => { diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index e8d98b3508..50e2c6acae 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/crates/worktree/src/worktree_tests.rs @@ -379,6 +379,7 @@ async fn test_renaming_case_only(cx: &mut TestAppContext) { fs::RenameOptions { overwrite: true, ignore_if_exists: true, + create_parents: false, }, ) .await @@ -1986,6 +1987,7 @@ async fn randomly_mutate_fs( fs::RenameOptions { overwrite: true, ignore_if_exists: true, + create_parents: false, }, ) .await