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
This commit is contained in:
@@ -2933,6 +2933,7 @@ impl TextThread {
|
||||
RenameOptions {
|
||||
overwrite: true,
|
||||
ignore_if_exists: true,
|
||||
create_parents: false,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -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")),
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1089,6 +1089,7 @@ async fn download_latest_version(
|
||||
RenameOptions {
|
||||
ignore_if_exists: true,
|
||||
overwrite: true,
|
||||
create_parents: false,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -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)) => {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user