fs: Return trashed file location (#52012)

Dino created

Update both `Fs::trash_dir` and `Fs::trash_file` to now return the
location of the trashed directory or file, as well as adding the
`trash-rs` create dependency and updating the `RealFs` implementation
for these methods to simply leverage `trash::delete_with_info`.

* Add `fs::Fs::TrashedEntry` struct, which allows us to track the
  original file path and the new path in the OS' trash
* Update the `fs::Fs::trash_dir` and `fs::Fs::trash_file` signatures to
  now return `Result<TrashedEntry>` instead of `Result<()>`
* The `options` argument was removed because it was never used by
  implementations other than the default one, and with this change to
  the signature type, we no longer have a default implementation, so the
  `options` argument would no longer make sense
* Update `fs::RealFs::trash_dir` and `fs::RealFs::trash_file`
  implementations to simply delegate to `trash-rs` and convert the
  result to a `TrashedEntry`
* Add `fs::FakeFs::trash` so we can simulate the OS' trash during tests
  that touch the filesystem
* Add `fs::FakeFs::trash_file` implementation to leverage
  `fs::FakeFs::trash`
* Add `fs::FakeFs::trash_dir` implementation to leverage
  `fs::FakeFs::trash`

Change summary

Cargo.lock                        |  69 ++++++
crates/fs/Cargo.toml              |   5 
crates/fs/src/fs.rs               | 317 ++++++++++++++++++--------------
crates/fs/tests/integration/fs.rs |  61 ++++++
crates/worktree/src/worktree.rs   |  11 
5 files changed, 308 insertions(+), 155 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -6462,7 +6462,6 @@ dependencies = [
  "ashpd",
  "async-tar",
  "async-trait",
- "cocoa 0.26.0",
  "collections",
  "dunce",
  "fs",
@@ -6474,7 +6473,6 @@ dependencies = [
  "libc",
  "log",
  "notify 8.2.0",
- "objc",
  "parking_lot",
  "paths",
  "proto",
@@ -6485,6 +6483,7 @@ dependencies = [
  "tempfile",
  "text",
  "time",
+ "trash",
  "util",
  "windows 0.61.3",
 ]
@@ -8444,7 +8443,7 @@ dependencies = [
  "js-sys",
  "log",
  "wasm-bindgen",
- "windows-core 0.62.2",
+ "windows-core 0.57.0",
 ]
 
 [[package]]
@@ -18416,7 +18415,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "2fb391ac70462b3097a755618fbf9c8f95ecc1eb379a414f7b46f202ed10db1f"
 dependencies = [
  "cc",
- "windows-targets 0.52.6",
+ "windows-targets 0.48.5",
 ]
 
 [[package]]
@@ -18429,6 +18428,24 @@ dependencies = [
  "strength_reduce",
 ]
 
+[[package]]
+name = "trash"
+version = "5.2.5"
+source = "git+https://github.com/zed-industries/trash-rs?rev=3bf27effd4eb8699f2e484d3326b852fe3e53af7#3bf27effd4eb8699f2e484d3326b852fe3e53af7"
+dependencies = [
+ "chrono",
+ "libc",
+ "log",
+ "objc2",
+ "objc2-foundation",
+ "once_cell",
+ "percent-encoding",
+ "scopeguard",
+ "urlencoding",
+ "windows 0.56.0",
+ "windows-core 0.56.0",
+]
+
 [[package]]
 name = "tree-sitter"
 version = "0.26.8"
@@ -20539,6 +20556,16 @@ dependencies = [
  "wasmtime-internal-math",
 ]
 
+[[package]]
+name = "windows"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1de69df01bdf1ead2f4ac895dc77c9351aefff65b2f3db429a343f9cbf05e132"
+dependencies = [
+ "windows-core 0.56.0",
+ "windows-targets 0.52.6",
+]
+
 [[package]]
 name = "windows"
 version = "0.57.0"
@@ -20627,6 +20654,18 @@ dependencies = [
  "windows-core 0.62.2",
 ]
 
+[[package]]
+name = "windows-core"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4698e52ed2d08f8658ab0c39512a7c00ee5fe2688c65f8c0a4f06750d729f2a6"
+dependencies = [
+ "windows-implement 0.56.0",
+ "windows-interface 0.56.0",
+ "windows-result 0.1.2",
+ "windows-targets 0.52.6",
+]
+
 [[package]]
 name = "windows-core"
 version = "0.57.0"
@@ -20700,6 +20739,17 @@ dependencies = [
  "windows-threading 0.2.1",
 ]
 
+[[package]]
+name = "windows-implement"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f6fc35f58ecd95a9b71c4f2329b911016e6bec66b3f2e6a4aad86bd2e99e2f9b"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.117",
+]
+
 [[package]]
 name = "windows-implement"
 version = "0.57.0"
@@ -20733,6 +20783,17 @@ dependencies = [
  "syn 2.0.117",
 ]
 
+[[package]]
+name = "windows-interface"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "08990546bf4edef8f431fa6326e032865f27138718c587dc21bc0265bbcb57cc"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.117",
+]
+
 [[package]]
 name = "windows-interface"
 version = "0.57.0"

crates/fs/Cargo.toml 🔗

@@ -41,10 +41,7 @@ time.workspace = true
 util.workspace = true
 is_executable = "1.0.5"
 notify = "8.2.0"
-
-[target.'cfg(target_os = "macos")'.dependencies]
-objc.workspace = true
-cocoa = "0.26"
+trash = { git = "https://github.com/zed-industries/trash-rs", rev = "3bf27effd4eb8699f2e484d3326b852fe3e53af7" }
 
 [target.'cfg(target_os = "windows")'.dependencies]
 windows.workspace = true

crates/fs/src/fs.rs 🔗

@@ -1,13 +1,12 @@
 pub mod fs_watcher;
 
 use parking_lot::Mutex;
+use std::ffi::OsString;
 use std::sync::atomic::{AtomicU8, AtomicUsize, Ordering};
 use std::time::Instant;
 use util::maybe;
 
 use anyhow::{Context as _, Result, anyhow};
-#[cfg(any(target_os = "linux", target_os = "freebsd"))]
-use ashpd::desktop::trash;
 use futures::stream::iter;
 use gpui::App;
 use gpui::BackgroundExecutor;
@@ -110,14 +109,27 @@ pub trait Fs: Send + Sync {
     ) -> Result<()>;
     async fn copy_file(&self, source: &Path, target: &Path, options: CopyOptions) -> Result<()>;
     async fn rename(&self, source: &Path, target: &Path, options: RenameOptions) -> Result<()>;
+
+    /// Removes a directory from the filesystem.
+    /// There is no expectation that the directory will be preserved in the
+    /// system trash.
     async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()>;
-    async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.remove_dir(path, options).await
-    }
+
+    /// Moves a directory to the system trash.
+    /// Returns a [`TrashedEntry`] that can be used to keep track of the
+    /// location of the trashed directory in the system's trash.
+    async fn trash_dir(&self, path: &Path) -> Result<TrashedEntry>;
+
+    /// Removes a file from the filesystem.
+    /// There is no expectation that the file will be preserved in the system
+    /// trash.
     async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()>;
-    async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.remove_file(path, options).await
-    }
+
+    /// Moves a file to the system trash.
+    /// Returns a [`TrashedEntry`] that can be used to keep track of the
+    /// location of the trashed file in the system's trash.
+    async fn trash_file(&self, path: &Path) -> Result<TrashedEntry>;
+
     async fn open_handle(&self, path: &Path) -> Result<Arc<dyn FileHandle>>;
     async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read + Send + Sync>>;
     async fn load(&self, path: &Path) -> Result<String> {
@@ -164,6 +176,35 @@ pub trait Fs: Send + Sync {
     }
 }
 
+// We use our own type rather than `trash::TrashItem` directly to avoid carrying
+// over fields we don't need (e.g. `time_deleted`) and to insulate callers and
+// tests from changes to that crate's API surface.
+/// Represents a file or directory that has been moved to the system trash,
+/// retaining enough information to restore it to its original location.
+#[derive(Clone)]
+pub struct TrashedEntry {
+    /// Platform-specific identifier for the file/directory in the trash.
+    ///
+    /// * Freedesktop – Path to the `.trashinfo` file.
+    /// * macOS & Windows – Full path to the file/directory in the system's
+    /// trash.
+    pub id: OsString,
+    /// Original name of the file/directory before it was moved to the trash.
+    pub name: OsString,
+    /// Original parent directory.
+    pub original_parent: PathBuf,
+}
+
+impl From<trash::TrashItem> for TrashedEntry {
+    fn from(item: trash::TrashItem) -> Self {
+        Self {
+            id: item.id,
+            name: item.name,
+            original_parent: item.original_parent,
+        }
+    }
+}
+
 struct GlobalFs(Arc<dyn Fs>);
 
 impl Global for GlobalFs {}
@@ -718,93 +759,12 @@ impl Fs for RealFs {
         }
     }
 
-    #[cfg(target_os = "macos")]
-    async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> {
-        use cocoa::{
-            base::{id, nil},
-            foundation::{NSAutoreleasePool, NSString},
-        };
-        use objc::{class, msg_send, sel, sel_impl};
-
-        unsafe {
-            /// Allow NSString::alloc use here because it sets autorelease
-            #[allow(clippy::disallowed_methods)]
-            unsafe fn ns_string(string: &str) -> id {
-                unsafe { NSString::alloc(nil).init_str(string).autorelease() }
-            }
-
-            let url: id = msg_send![class!(NSURL), fileURLWithPath: ns_string(path.to_string_lossy().as_ref())];
-            let array: id = msg_send![class!(NSArray), arrayWithObject: url];
-            let workspace: id = msg_send![class!(NSWorkspace), sharedWorkspace];
-
-            let _: id = msg_send![workspace, recycleURLs: array completionHandler: nil];
-        }
-        Ok(())
-    }
-
-    #[cfg(any(target_os = "linux", target_os = "freebsd"))]
-    async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> {
-        if let Ok(Some(metadata)) = self.metadata(path).await
-            && metadata.is_symlink
-        {
-            // TODO: trash_file does not support trashing symlinks yet - https://github.com/bilelmoussaoui/ashpd/issues/255
-            return self.remove_file(path, RemoveOptions::default()).await;
-        }
-        let file = smol::fs::File::open(path).await?;
-        match trash::trash_file(&file.as_fd()).await {
-            Ok(_) => Ok(()),
-            Err(err) => {
-                log::error!("Failed to trash file: {}", err);
-                // Trashing files can fail if you don't have a trashing dbus service configured.
-                // In that case, delete the file directly instead.
-                return self.remove_file(path, RemoveOptions::default()).await;
-            }
-        }
-    }
-
-    #[cfg(target_os = "windows")]
-    async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> {
-        use util::paths::SanitizedPath;
-        use windows::{
-            Storage::{StorageDeleteOption, StorageFile},
-            core::HSTRING,
-        };
-        // todo(windows)
-        // When new version of `windows-rs` release, make this operation `async`
-        let path = path.canonicalize()?;
-        let path = SanitizedPath::new(&path);
-        let path_string = path.to_string();
-        let file = StorageFile::GetFileFromPathAsync(&HSTRING::from(path_string))?.get()?;
-        file.DeleteAsync(StorageDeleteOption::Default)?.get()?;
-        Ok(())
-    }
-
-    #[cfg(target_os = "macos")]
-    async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.trash_file(path, options).await
-    }
-
-    #[cfg(any(target_os = "linux", target_os = "freebsd"))]
-    async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.trash_file(path, options).await
+    async fn trash_file(&self, path: &Path) -> Result<TrashedEntry> {
+        Ok(trash::delete_with_info(path)?.into())
     }
 
-    #[cfg(target_os = "windows")]
-    async fn trash_dir(&self, path: &Path, _options: RemoveOptions) -> Result<()> {
-        use util::paths::SanitizedPath;
-        use windows::{
-            Storage::{StorageDeleteOption, StorageFolder},
-            core::HSTRING,
-        };
-
-        // todo(windows)
-        // When new version of `windows-rs` release, make this operation `async`
-        let path = path.canonicalize()?;
-        let path = SanitizedPath::new(&path);
-        let path_string = path.to_string();
-        let folder = StorageFolder::GetFolderFromPathAsync(&HSTRING::from(path_string))?.get()?;
-        folder.DeleteAsync(StorageDeleteOption::Default)?.get()?;
-        Ok(())
+    async fn trash_dir(&self, path: &Path) -> Result<TrashedEntry> {
+        self.trash_file(path).await
     }
 
     async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read + Send + Sync>> {
@@ -1287,6 +1247,7 @@ struct FakeFsState {
     path_write_counts: std::collections::HashMap<PathBuf, usize>,
     moves: std::collections::HashMap<u64, PathBuf>,
     job_event_subscribers: Arc<Mutex<Vec<JobEventSender>>>,
+    trash: Vec<(TrashedEntry, FakeFsEntry)>,
 }
 
 #[cfg(feature = "test-support")]
@@ -1572,6 +1533,7 @@ impl FakeFs {
                 path_write_counts: Default::default(),
                 moves: Default::default(),
                 job_event_subscribers: Arc::new(Mutex::new(Vec::new())),
+                trash: Vec::new(),
             })),
         });
 
@@ -2397,6 +2359,90 @@ impl FakeFs {
     fn simulate_random_delay(&self) -> impl futures::Future<Output = ()> {
         self.executor.simulate_random_delay()
     }
+
+    /// Returns list of all tracked trash entries.
+    pub fn trash_entries(&self) -> Vec<TrashedEntry> {
+        self.state
+            .lock()
+            .trash
+            .iter()
+            .map(|(entry, _)| entry.clone())
+            .collect()
+    }
+
+    async fn remove_dir_inner(
+        &self,
+        path: &Path,
+        options: RemoveOptions,
+    ) -> Result<Option<FakeFsEntry>> {
+        self.simulate_random_delay().await;
+
+        let path = normalize_path(path);
+        let parent_path = path.parent().context("cannot remove the root")?;
+        let base_name = path.file_name().context("cannot remove the root")?;
+
+        let mut state = self.state.lock();
+        let parent_entry = state.entry(parent_path)?;
+        let entry = parent_entry
+            .dir_entries(parent_path)?
+            .entry(base_name.to_str().unwrap().into());
+
+        let removed = match entry {
+            btree_map::Entry::Vacant(_) => {
+                if !options.ignore_if_not_exists {
+                    anyhow::bail!("{path:?} does not exist");
+                }
+
+                None
+            }
+            btree_map::Entry::Occupied(mut entry) => {
+                {
+                    let children = entry.get_mut().dir_entries(&path)?;
+                    if !options.recursive && !children.is_empty() {
+                        anyhow::bail!("{path:?} is not empty");
+                    }
+                }
+
+                Some(entry.remove())
+            }
+        };
+
+        state.emit_event([(path, Some(PathEventKind::Removed))]);
+        Ok(removed)
+    }
+
+    async fn remove_file_inner(
+        &self,
+        path: &Path,
+        options: RemoveOptions,
+    ) -> Result<Option<FakeFsEntry>> {
+        self.simulate_random_delay().await;
+
+        let path = normalize_path(path);
+        let parent_path = path.parent().context("cannot remove the root")?;
+        let base_name = path.file_name().unwrap();
+        let mut state = self.state.lock();
+        let parent_entry = state.entry(parent_path)?;
+        let entry = parent_entry
+            .dir_entries(parent_path)?
+            .entry(base_name.to_str().unwrap().into());
+        let removed = match entry {
+            btree_map::Entry::Vacant(_) => {
+                if !options.ignore_if_not_exists {
+                    anyhow::bail!("{path:?} does not exist");
+                }
+
+                None
+            }
+            btree_map::Entry::Occupied(mut entry) => {
+                entry.get_mut().file_content(&path)?;
+                Some(entry.remove())
+            }
+        };
+
+        state.emit_event([(path, Some(PathEventKind::Removed))]);
+        Ok(removed)
+    }
 }
 
 #[cfg(feature = "test-support")]
@@ -2696,62 +2742,57 @@ impl Fs for FakeFs {
     }
 
     async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.simulate_random_delay().await;
+        self.remove_dir_inner(path, options).await.map(|_| ())
+    }
 
-        let path = normalize_path(path);
-        let parent_path = path.parent().context("cannot remove the root")?;
-        let base_name = path.file_name().context("cannot remove the root")?;
+    async fn trash_dir(&self, path: &Path) -> Result<TrashedEntry> {
+        let normalized_path = normalize_path(path);
+        let parent_path = normalized_path.parent().context("cannot remove the root")?;
+        let base_name = normalized_path.file_name().unwrap();
+        let options = RemoveOptions {
+            recursive: true,
+            ..Default::default()
+        };
 
-        let mut state = self.state.lock();
-        let parent_entry = state.entry(parent_path)?;
-        let entry = parent_entry
-            .dir_entries(parent_path)?
-            .entry(base_name.to_str().unwrap().into());
+        match self.remove_dir_inner(path, options).await? {
+            Some(fake_entry) => {
+                let trashed_entry = TrashedEntry {
+                    id: base_name.to_str().unwrap().into(),
+                    name: base_name.to_str().unwrap().into(),
+                    original_parent: parent_path.to_path_buf(),
+                };
 
-        match entry {
-            btree_map::Entry::Vacant(_) => {
-                if !options.ignore_if_not_exists {
-                    anyhow::bail!("{path:?} does not exist");
-                }
-            }
-            btree_map::Entry::Occupied(mut entry) => {
-                {
-                    let children = entry.get_mut().dir_entries(&path)?;
-                    if !options.recursive && !children.is_empty() {
-                        anyhow::bail!("{path:?} is not empty");
-                    }
-                }
-                entry.remove();
+                let mut state = self.state.lock();
+                state.trash.push((trashed_entry.clone(), fake_entry));
+                Ok(trashed_entry)
             }
+            None => anyhow::bail!("{normalized_path:?} does not exist"),
         }
-        state.emit_event([(path, Some(PathEventKind::Removed))]);
-        Ok(())
     }
 
     async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()> {
-        self.simulate_random_delay().await;
+        self.remove_file_inner(path, options).await.map(|_| ())
+    }
 
-        let path = normalize_path(path);
-        let parent_path = path.parent().context("cannot remove the root")?;
-        let base_name = path.file_name().unwrap();
-        let mut state = self.state.lock();
-        let parent_entry = state.entry(parent_path)?;
-        let entry = parent_entry
-            .dir_entries(parent_path)?
-            .entry(base_name.to_str().unwrap().into());
-        match entry {
-            btree_map::Entry::Vacant(_) => {
-                if !options.ignore_if_not_exists {
-                    anyhow::bail!("{path:?} does not exist");
-                }
-            }
-            btree_map::Entry::Occupied(mut entry) => {
-                entry.get_mut().file_content(&path)?;
-                entry.remove();
+    async fn trash_file(&self, path: &Path) -> Result<TrashedEntry> {
+        let normalized_path = normalize_path(path);
+        let parent_path = normalized_path.parent().context("cannot remove the root")?;
+        let base_name = normalized_path.file_name().unwrap();
+
+        match self.remove_file_inner(path, Default::default()).await? {
+            Some(fake_entry) => {
+                let trashed_entry = TrashedEntry {
+                    id: base_name.to_str().unwrap().into(),
+                    name: base_name.to_str().unwrap().into(),
+                    original_parent: parent_path.to_path_buf(),
+                };
+
+                let mut state = self.state.lock();
+                state.trash.push((trashed_entry.clone(), fake_entry));
+                Ok(trashed_entry)
             }
+            None => anyhow::bail!("{normalized_path:?} does not exist"),
         }
-        state.emit_event([(path, Some(PathEventKind::Removed))]);
-        Ok(())
     }
 
     async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read + Send + Sync>> {

crates/fs/tests/integration/fs.rs 🔗

@@ -626,6 +626,67 @@ async fn test_realfs_symlink_loop_metadata(executor: BackgroundExecutor) {
     // don't care about len or mtime on symlinks?
 }
 
+#[gpui::test]
+async fn test_fake_fs_trash_file(executor: BackgroundExecutor) {
+    let fs = FakeFs::new(executor.clone());
+    fs.insert_tree(
+        path!("/root"),
+        json!({
+            "file_a.txt": "File A",
+            "file_b.txt": "File B",
+        }),
+    )
+    .await;
+
+    let root_path = PathBuf::from(path!("/root"));
+    let path = path!("/root/file_a.txt").as_ref();
+    let trashed_entry = fs
+        .trash_file(path)
+        .await
+        .expect("should be able to trash {path:?}");
+
+    assert_eq!(trashed_entry.name, "file_a.txt");
+    assert_eq!(trashed_entry.original_parent, root_path);
+    assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_b.txt"))]);
+
+    let trash_entries = fs.trash_entries();
+    assert_eq!(trash_entries.len(), 1);
+    assert_eq!(trash_entries[0].name, "file_a.txt");
+    assert_eq!(trash_entries[0].original_parent, root_path);
+}
+
+#[gpui::test]
+async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) {
+    let fs = FakeFs::new(executor.clone());
+    fs.insert_tree(
+        path!("/root"),
+        json!({
+            "src": {
+                "file_a.txt": "File A",
+                "file_b.txt": "File B",
+            },
+            "file_c.txt": "File C",
+        }),
+    )
+    .await;
+
+    let root_path = PathBuf::from(path!("/root"));
+    let path = path!("/root/src").as_ref();
+    let trashed_entry = fs
+        .trash_dir(path)
+        .await
+        .expect("should be able to trash {path:?}");
+
+    assert_eq!(trashed_entry.name, "src");
+    assert_eq!(trashed_entry.original_parent, root_path);
+    assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]);
+
+    let trash_entries = fs.trash_entries();
+    assert_eq!(trash_entries.len(), 1);
+    assert_eq!(trash_entries[0].name, "src");
+    assert_eq!(trash_entries[0].original_parent, root_path);
+}
+
 #[gpui::test]
 #[ignore = "stress test; run explicitly when needed"]
 async fn test_realfs_watch_stress_reports_missed_paths(

crates/worktree/src/worktree.rs 🔗

@@ -1693,19 +1693,12 @@ impl LocalWorktree {
         let delete = cx.background_spawn(async move {
             if entry.is_file() {
                 if trash {
-                    fs.trash_file(&abs_path, Default::default()).await?;
+                    fs.trash_file(&abs_path).await?;
                 } else {
                     fs.remove_file(&abs_path, Default::default()).await?;
                 }
             } else if trash {
-                fs.trash_dir(
-                    &abs_path,
-                    RemoveOptions {
-                        recursive: true,
-                        ignore_if_not_exists: false,
-                    },
-                )
-                .await?;
+                fs.trash_dir(&abs_path).await?;
             } else {
                 fs.remove_dir(
                     &abs_path,