Add File.disk_state enum to clarify filesystem states (#20776)

Motivation for this is to make things more understandable while figuring
out #20775.

This is intended to be a refactoring that does not affect behavior, but
there are a few tricky spots:

* Previously `File.mtime()` (now `File.disk_state().mtime()`) would
return last known modification time for deleted files. Looking at uses,
I believe this will not affect anything. If there are behavior changes
here I believe they would be improvements.

* `BufferEvent::DirtyChanged` is now only emitted if dirtiness actually
changed, rather than if it may have changed. This should only be an
efficiency improvement.

Release Notes:

- N/A

Co-authored-by: Mikayla Maki <mikayla@zed.dev>

Cargo.toml

@@ -388,7 +388,7 @@ pet-core = { git = "https://github.com/microsoft/python-environment-tools.git",
 pet-poetry = { git = "https://github.com/microsoft/python-environment-tools.git", rev = "ffcbf3f28c46633abd5448a52b1f396c322e0d6c"  }
 pet-reporter = { git = "https://github.com/microsoft/python-environment-tools.git", rev = "ffcbf3f28c46633abd5448a52b1f396c322e0d6c"  }
 postage = { version = "0.5", features = ["futures-traits"] }
-pretty_assertions = "1.3.0"
+pretty_assertions = { version = "1.3.0", features = ["unstable"] }
 profiling = "1"
 prost = "0.9"
 prost-build = "0.9"

crates/collab/src/tests/random_project_collaboration_tests.rs

@@ -1323,11 +1323,8 @@ impl RandomizedTest for ProjectCollaborationTest {
                     match (host_file, guest_file) {
                         (Some(host_file), Some(guest_file)) => {
                             assert_eq!(guest_file.path(), host_file.path());
-                            assert_eq!(guest_file.is_deleted(), host_file.is_deleted());
-                            assert_eq!(
-                                guest_file.mtime(),
-                                host_file.mtime(),
-                                "guest {} mtime does not match host {} for path {:?} in project {}",
+                            assert_eq!(guest_file.disk_state(), host_file.disk_state(),
+                                "guest {} disk_state does not match host {} for path {:?} in project {}",
                                 guest_user_id,
                                 host_user_id,
                                 guest_file.path(),

crates/copilot/src/copilot.rs

@@ -1229,8 +1229,10 @@ mod tests {
             Some(self)
         }
 
-        fn mtime(&self) -> Option<std::time::SystemTime> {
-            unimplemented!()
+        fn disk_state(&self) -> language::DiskState {
+            language::DiskState::Present {
+                mtime: std::time::UNIX_EPOCH,
+            }
         }
 
         fn path(&self) -> &Arc<Path> {
@@ -1245,10 +1247,6 @@ mod tests {
             unimplemented!()
         }
 
-        fn is_deleted(&self) -> bool {
-            unimplemented!()
-        }
-
         fn as_any(&self) -> &dyn std::any::Any {
             unimplemented!()
         }

crates/editor/src/items.rs

@@ -16,7 +16,8 @@ use gpui::{
     VisualContext, WeakView, WindowContext,
 };
 use language::{
-    proto::serialize_anchor as serialize_text_anchor, Bias, Buffer, CharKind, Point, SelectionGoal,
+    proto::serialize_anchor as serialize_text_anchor, Bias, Buffer, CharKind, DiskState, Point,
+    SelectionGoal,
 };
 use lsp::DiagnosticSeverity;
 use multi_buffer::AnchorRangeExt;
@@ -641,7 +642,7 @@ impl Item for Editor {
             .read(cx)
             .as_singleton()
             .and_then(|buffer| buffer.read(cx).file())
-            .map_or(false, |file| file.is_deleted() && file.is_created());
+            .map_or(false, |file| file.disk_state() == DiskState::Deleted);
 
         h_flex()
             .gap_2()

crates/language/src/buffer.rs

@@ -104,6 +104,7 @@ pub struct Buffer {
     text: TextBuffer,
     diff_base: Option<BufferDiffBase>,
     git_diff: git::diff::BufferDiff,
+    /// Filesystem state, `None` when there is no path.
     file: Option<Arc<dyn File>>,
     /// The mtime of the file when this buffer was last loaded from
     /// or saved to disk.
@@ -371,8 +372,9 @@ pub trait File: Send + Sync {
         self.as_local().is_some()
     }
 
-    /// Returns the file's mtime.
-    fn mtime(&self) -> Option<SystemTime>;
+    /// Returns whether the file is new, exists in storage, or has been deleted. Includes metadata
+    /// only available in some states, such as modification time.
+    fn disk_state(&self) -> DiskState;
 
     /// Returns the path of this file relative to the worktree's root directory.
     fn path(&self) -> &Arc<Path>;
@@ -390,14 +392,6 @@ pub trait File: Send + Sync {
     /// This is needed for looking up project-specific settings.
     fn worktree_id(&self, cx: &AppContext) -> WorktreeId;
 
-    /// Returns whether the file has been deleted.
-    fn is_deleted(&self) -> bool;
-
-    /// Returns whether the file existed on disk at one point
-    fn is_created(&self) -> bool {
-        self.mtime().is_some()
-    }
-
     /// Converts this file into an [`Any`] trait object.
     fn as_any(&self) -> &dyn Any;
 
@@ -408,6 +402,34 @@ pub trait File: Send + Sync {
     fn is_private(&self) -> bool;
 }
 
+/// The file's storage status - whether it's stored (`Present`), and if so when it was last
+/// modified. In the case where the file is not stored, it can be either `New` or `Deleted`. In the
+/// UI these two states are distinguished. For example, the buffer tab does not display a deletion
+/// indicator for new files.
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum DiskState {
+    /// File created in Zed that has not been saved.
+    New,
+    /// File present on the filesystem.
+    Present {
+        /// Last known mtime (modification time).
+        mtime: SystemTime,
+    },
+    /// Deleted file that was previously present.
+    Deleted,
+}
+
+impl DiskState {
+    /// Returns the file's last known modification time on disk.
+    pub fn mtime(self) -> Option<SystemTime> {
+        match self {
+            DiskState::New => None,
+            DiskState::Present { mtime } => Some(mtime),
+            DiskState::Deleted => None,
+        }
+    }
+}
+
 /// The file associated with a buffer, in the case where the file is on the local disk.
 pub trait LocalFile: File {
     /// Returns the absolute path of this file
@@ -750,7 +772,7 @@ impl Buffer {
         file: Option<Arc<dyn File>>,
         capability: Capability,
     ) -> Self {
-        let saved_mtime = file.as_ref().and_then(|file| file.mtime());
+        let saved_mtime = file.as_ref().and_then(|file| file.disk_state().mtime());
         let snapshot = buffer.snapshot();
         let git_diff = git::diff::BufferDiff::new(&snapshot);
         let syntax_map = Mutex::new(SyntaxMap::new(&snapshot));
@@ -1014,7 +1036,7 @@ impl Buffer {
         self.reload_task = Some(cx.spawn(|this, mut cx| async move {
             let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| {
                 let file = this.file.as_ref()?.as_local()?;
-                Some((file.mtime(), file.load(cx)))
+                Some((file.disk_state().mtime(), file.load(cx)))
             })?
             else {
                 return Ok(());
@@ -1070,28 +1092,20 @@ impl Buffer {
     /// Updates the [`File`] backing this buffer. This should be called when
     /// the file has changed or has been deleted.
     pub fn file_updated(&mut self, new_file: Arc<dyn File>, cx: &mut ModelContext<Self>) {
+        let was_dirty = self.is_dirty();
         let mut file_changed = false;
 
         if let Some(old_file) = self.file.as_ref() {
             if new_file.path() != old_file.path() {
                 file_changed = true;
             }
 
-            if new_file.is_deleted() {
-                if !old_file.is_deleted() {
-                    file_changed = true;
-                    if !self.is_dirty() {
-                        cx.emit(BufferEvent::DirtyChanged);
-                    }
-                }
-            } else {
-                let new_mtime = new_file.mtime();
-                if new_mtime != old_file.mtime() {
-                    file_changed = true;
-
-                    if !self.is_dirty() {
-                        cx.emit(BufferEvent::ReloadNeeded);
-                    }
+            let old_state = old_file.disk_state();
+            let new_state = new_file.disk_state();
+            if old_state != new_state {
+                file_changed = true;
+                if !was_dirty && matches!(new_state, DiskState::Present { .. }) {
+                    cx.emit(BufferEvent::ReloadNeeded)
                 }
             }
         } else {
@@ -1101,6 +1115,9 @@ impl Buffer {
         self.file = Some(new_file);
         if file_changed {
             self.non_text_state_update_count += 1;
+            if was_dirty != self.is_dirty() {
+                cx.emit(BufferEvent::DirtyChanged);
+            }
             cx.emit(BufferEvent::FileHandleChanged);
             cx.notify();
         }
@@ -1742,15 +1759,10 @@ impl Buffer {
     pub fn is_dirty(&self) -> bool {
         self.capability != Capability::ReadOnly
             && (self.has_conflict
-                || self.has_unsaved_edits()
-                || self
-                    .file
-                    .as_ref()
-                    .map_or(false, |file| file.is_deleted() || !file.is_created()))
-    }
-
-    pub fn is_deleted(&self) -> bool {
-        self.file.as_ref().map_or(false, |file| file.is_deleted())
+                || self.file.as_ref().map_or(false, |file| {
+                    matches!(file.disk_state(), DiskState::New | DiskState::Deleted)
+                })
+                || self.has_unsaved_edits())
     }
 
     /// Checks if the buffer and its file have both changed since the buffer
@@ -1762,7 +1774,13 @@ impl Buffer {
         let Some(file) = self.file.as_ref() else {
             return false;
         };
-        file.is_deleted() || (file.mtime() > self.saved_mtime && self.has_unsaved_edits())
+        match file.disk_state() {
+            DiskState::New | DiskState::Deleted => true,
+            DiskState::Present { mtime } => match self.saved_mtime {
+                Some(saved_mtime) => mtime > saved_mtime && self.has_unsaved_edits(),
+                None => true,
+            },
+        }
     }
 
     /// Gets a [`Subscription`] that tracks all of the changes to the buffer's text.
@@ -4403,7 +4421,7 @@ impl File for TestFile {
         None
     }
 
-    fn mtime(&self) -> Option<SystemTime> {
+    fn disk_state(&self) -> DiskState {
         unimplemented!()
     }
 
@@ -4415,10 +4433,6 @@ impl File for TestFile {
         WorktreeId::from_usize(0)
     }
 
-    fn is_deleted(&self) -> bool {
-        unimplemented!()
-    }
-
     fn as_any(&self) -> &dyn std::any::Any {
         unimplemented!()
     }

crates/multi_buffer/src/multi_buffer.rs

@@ -10,9 +10,9 @@ use itertools::Itertools;
 use language::{
     language_settings::{language_settings, LanguageSettings},
     AutoindentMode, Buffer, BufferChunks, BufferRow, BufferSnapshot, Capability, CharClassifier,
-    CharKind, Chunk, CursorShape, DiagnosticEntry, File, IndentGuide, IndentSize, Language,
-    LanguageScope, OffsetRangeExt, OffsetUtf16, Outline, OutlineItem, Point, PointUtf16, Selection,
-    TextDimension, ToOffset as _, ToOffsetUtf16 as _, ToPoint as _, ToPointUtf16 as _,
+    CharKind, Chunk, CursorShape, DiagnosticEntry, DiskState, File, IndentGuide, IndentSize,
+    Language, LanguageScope, OffsetRangeExt, OffsetUtf16, Outline, OutlineItem, Point, PointUtf16,
+    Selection, TextDimension, ToOffset as _, ToOffsetUtf16 as _, ToPoint as _, ToPointUtf16 as _,
     TransactionId, Unclipped,
 };
 use smallvec::SmallVec;
@@ -2035,7 +2035,9 @@ impl MultiBuffer {
             edited |= buffer_edited;
             non_text_state_updated |= buffer_non_text_state_updated;
             is_dirty |= buffer.is_dirty();
-            has_deleted_file |= buffer.file().map_or(false, |file| file.is_deleted());
+            has_deleted_file |= buffer
+                .file()
+                .map_or(false, |file| file.disk_state() == DiskState::Deleted);
             has_conflict |= buffer.has_conflict();
         }
         if edited {

crates/project/src/buffer_store.rs

@@ -20,7 +20,7 @@ use language::{
         deserialize_line_ending, deserialize_version, serialize_line_ending, serialize_version,
         split_operations,
     },
-    Buffer, BufferEvent, Capability, File as _, Language, Operation,
+    Buffer, BufferEvent, Capability, DiskState, File as _, Language, Operation,
 };
 use rpc::{proto, AnyProtoClient, ErrorExt as _, TypedEnvelope};
 use smol::channel::Receiver;
@@ -434,7 +434,10 @@ impl LocalBufferStore {
         let line_ending = buffer.line_ending();
         let version = buffer.version();
         let buffer_id = buffer.remote_id();
-        if buffer.file().is_some_and(|file| !file.is_created()) {
+        if buffer
+            .file()
+            .is_some_and(|file| file.disk_state() == DiskState::New)
+        {
             has_changed_file = true;
         }
 
@@ -444,7 +447,7 @@ impl LocalBufferStore {
 
         cx.spawn(move |this, mut cx| async move {
             let new_file = save.await?;
-            let mtime = new_file.mtime;
+            let mtime = new_file.disk_state().mtime();
             this.update(&mut cx, |this, cx| {
                 if let Some((downstream_client, project_id)) = this.downstream_client(cx) {
                     if has_changed_file {
@@ -658,37 +661,30 @@ impl LocalBufferStore {
                 return None;
             }
 
-            let new_file = if let Some(entry) = old_file
+            let snapshot_entry = old_file
                 .entry_id
                 .and_then(|entry_id| snapshot.entry_for_id(entry_id))
-            {
-                File {
-                    is_local: true,
-                    entry_id: Some(entry.id),
-                    mtime: entry.mtime,
-                    path: entry.path.clone(),
-                    worktree: worktree.clone(),
-                    is_deleted: false,
-                    is_private: entry.is_private,
-                }
-            } else if let Some(entry) = snapshot.entry_for_path(old_file.path.as_ref()) {
+                .or_else(|| snapshot.entry_for_path(old_file.path.as_ref()));
+
+            let new_file = if let Some(entry) = snapshot_entry {
                 File {
+                    disk_state: match entry.mtime {
+                        Some(mtime) => DiskState::Present { mtime },
+                        None => old_file.disk_state,
+                    },
                     is_local: true,
                     entry_id: Some(entry.id),
-                    mtime: entry.mtime,
                     path: entry.path.clone(),
                     worktree: worktree.clone(),
-                    is_deleted: false,
                     is_private: entry.is_private,
                 }
             } else {
                 File {
+                    disk_state: DiskState::Deleted,
                     is_local: true,
                     entry_id: old_file.entry_id,
                     path: old_file.path.clone(),
-                    mtime: old_file.mtime,
                     worktree: worktree.clone(),
-                    is_deleted: true,
                     is_private: old_file.is_private,
                 }
             };
@@ -867,10 +863,9 @@ impl BufferStoreImpl for Model<LocalBufferStore> {
                             Some(Arc::new(File {
                                 worktree,
                                 path,
-                                mtime: None,
+                                disk_state: DiskState::New,
                                 entry_id: None,
                                 is_local: true,
-                                is_deleted: false,
                                 is_private: false,
                             })),
                             Capability::ReadWrite,