chore: Wrap WorkspaceId in newtype (#9492)

Piotr Osiewicz created

Release Notes:

- N/A

Change summary

crates/collab/src/tests/editor_tests.rs       | 12 +++++-
crates/collab/src/tests/test_server.rs        | 16 ++++++++-
crates/editor/src/editor.rs                   |  6 ++-
crates/editor/src/scroll.rs                   |  4 +-
crates/recent_projects/src/recent_projects.rs |  5 ++
crates/workspace/src/item.rs                  |  2 
crates/workspace/src/persistence.rs           | 14 ++++----
crates/workspace/src/workspace.rs             | 32 +++++++++++++++++---
8 files changed, 67 insertions(+), 24 deletions(-)

Detailed changes

crates/collab/src/tests/editor_tests.rs 🔗

@@ -30,7 +30,7 @@ use std::{
     },
 };
 use text::Point;
-use workspace::Workspace;
+use workspace::{Workspace, WorkspaceId};
 
 #[gpui::test(iterations = 10)]
 async fn test_host_disconnect(
@@ -73,8 +73,14 @@ async fn test_host_disconnect(
 
     assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared()));
 
-    let workspace_b =
-        cx_b.add_window(|cx| Workspace::new(0, project_b.clone(), client_b.app_state.clone(), cx));
+    let workspace_b = cx_b.add_window(|cx| {
+        Workspace::new(
+            WorkspaceId::default(),
+            project_b.clone(),
+            client_b.app_state.clone(),
+            cx,
+        )
+    });
     let cx_b = &mut VisualTestContext::from_window(*workspace_b, cx_b);
     let workspace_b_view = workspace_b.root_view(cx_b).unwrap();
 

crates/collab/src/tests/test_server.rs 🔗

@@ -40,7 +40,7 @@ use std::{
     },
 };
 use util::{http::FakeHttpClient, SemanticVersion};
-use workspace::{Workspace, WorkspaceStore};
+use workspace::{Workspace, WorkspaceId, WorkspaceStore};
 
 pub struct TestServer {
     pub app_state: Arc<AppState>,
@@ -753,7 +753,12 @@ impl TestClient {
     ) -> (View<Workspace>, &'a mut VisualTestContext) {
         cx.add_window_view(|cx| {
             cx.activate_window();
-            Workspace::new(0, project.clone(), self.app_state.clone(), cx)
+            Workspace::new(
+                WorkspaceId::default(),
+                project.clone(),
+                self.app_state.clone(),
+                cx,
+            )
         })
     }
 
@@ -764,7 +769,12 @@ impl TestClient {
         let project = self.build_test_project(cx).await;
         cx.add_window_view(|cx| {
             cx.activate_window();
-            Workspace::new(0, project.clone(), self.app_state.clone(), cx)
+            Workspace::new(
+                WorkspaceId::default(),
+                project.clone(),
+                self.app_state.clone(),
+                cx,
+            )
         })
     }
 

crates/editor/src/editor.rs 🔗

@@ -125,7 +125,9 @@ use ui::{
 };
 use util::{maybe, post_inc, RangeExt, ResultExt, TryFutureExt};
 use workspace::Toast;
-use workspace::{searchable::SearchEvent, ItemNavHistory, SplitDirection, ViewId, Workspace};
+use workspace::{
+    searchable::SearchEvent, ItemNavHistory, SplitDirection, ViewId, Workspace, WorkspaceId,
+};
 
 use crate::hover_links::find_url;
 
@@ -407,7 +409,7 @@ pub struct Editor {
     cursor_shape: CursorShape,
     collapse_matches: bool,
     autoindent_mode: Option<AutoindentMode>,
-    workspace: Option<(WeakView<Workspace>, i64)>,
+    workspace: Option<(WeakView<Workspace>, WorkspaceId)>,
     keymap_context_layers: BTreeMap<TypeId, KeyContext>,
     input_enabled: bool,
     use_modal_editing: bool,

crates/editor/src/scroll.rs 🔗

@@ -182,7 +182,7 @@ impl ScrollManager {
         map: &DisplaySnapshot,
         local: bool,
         autoscroll: bool,
-        workspace_id: Option<i64>,
+        workspace_id: Option<WorkspaceId>,
         cx: &mut ViewContext<Editor>,
     ) {
         let (new_anchor, top_row) = if scroll_position.y <= 0. {
@@ -221,7 +221,7 @@ impl ScrollManager {
         top_row: u32,
         local: bool,
         autoscroll: bool,
-        workspace_id: Option<i64>,
+        workspace_id: Option<WorkspaceId>,
         cx: &mut ViewContext<Editor>,
     ) {
         self.anchor = anchor;

crates/recent_projects/src/recent_projects.rs 🔗

@@ -554,7 +554,10 @@ mod tests {
                         positions: Vec::new(),
                         string: "fake candidate".to_string(),
                     }];
-                    delegate.workspaces = vec![(0, WorkspaceLocation::new(vec!["/test/path/"]))];
+                    delegate.workspaces = vec![(
+                        WorkspaceId::default(),
+                        WorkspaceLocation::new(vec!["/test/path/"]),
+                    )];
                 });
             })
             .unwrap();

crates/workspace/src/item.rs 🔗

@@ -913,7 +913,7 @@ pub mod test {
                 nav_history: None,
                 tab_descriptions: None,
                 tab_detail: Default::default(),
-                workspace_id: 0,
+                workspace_id: Default::default(),
                 focus_handle: cx.focus_handle(),
             }
         }

crates/workspace/src/persistence.rs 🔗

@@ -753,7 +753,7 @@ mod tests {
         .unwrap();
 
         let mut workspace_1 = SerializedWorkspace {
-            id: 1,
+            id: WorkspaceId(1),
             location: (["/tmp", "/tmp2"]).into(),
             center_group: Default::default(),
             bounds: Default::default(),
@@ -763,7 +763,7 @@ mod tests {
         };
 
         let workspace_2 = SerializedWorkspace {
-            id: 2,
+            id: WorkspaceId(2),
             location: (["/tmp"]).into(),
             center_group: Default::default(),
             bounds: Default::default(),
@@ -862,7 +862,7 @@ mod tests {
         );
 
         let workspace = SerializedWorkspace {
-            id: 5,
+            id: WorkspaceId(5),
             location: (["/tmp", "/tmp2"]).into(),
             center_group,
             bounds: Default::default(),
@@ -891,7 +891,7 @@ mod tests {
         let db = WorkspaceDb(open_test_db("test_basic_functionality").await);
 
         let workspace_1 = SerializedWorkspace {
-            id: 1,
+            id: WorkspaceId(1),
             location: (["/tmp", "/tmp2"]).into(),
             center_group: Default::default(),
             bounds: Default::default(),
@@ -901,7 +901,7 @@ mod tests {
         };
 
         let mut workspace_2 = SerializedWorkspace {
-            id: 2,
+            id: WorkspaceId(2),
             location: (["/tmp"]).into(),
             center_group: Default::default(),
             bounds: Default::default(),
@@ -938,7 +938,7 @@ mod tests {
 
         // Test other mechanism for mutating
         let mut workspace_3 = SerializedWorkspace {
-            id: 3,
+            id: WorkspaceId(3),
             location: (&["/tmp", "/tmp2"]).into(),
             center_group: Default::default(),
             bounds: Default::default(),
@@ -972,7 +972,7 @@ mod tests {
         center_group: &SerializedPaneGroup,
     ) -> SerializedWorkspace {
         SerializedWorkspace {
-            id: 4,
+            id: WorkspaceId(4),
             location: workspace_id.into(),
             center_group: center_group.clone(),
             bounds: Default::default(),

crates/workspace/src/workspace.rs 🔗

@@ -51,6 +51,10 @@ use project::{Project, ProjectEntryId, ProjectPath, Worktree, WorktreeId};
 use serde::Deserialize;
 use settings::Settings;
 use shared_screen::SharedScreen;
+use sqlez::{
+    bindable::{Bind, Column, StaticColumnCount},
+    statement::Statement,
+};
 use status_bar::StatusBar;
 pub use status_bar::StatusItemView;
 use std::{
@@ -237,8 +241,22 @@ pub struct OpenTerminal {
     pub working_directory: PathBuf,
 }
 
-pub type WorkspaceId = i64;
+#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)]
+pub struct WorkspaceId(i64);
 
+impl StaticColumnCount for WorkspaceId {}
+impl Bind for WorkspaceId {
+    fn bind(&self, statement: &Statement, start_index: i32) -> Result<i32> {
+        self.0.bind(statement, start_index)
+    }
+}
+impl Column for WorkspaceId {
+    fn column(statement: &mut Statement, start_index: i32) -> Result<(Self, i32)> {
+        i64::column(statement, start_index)
+            .map(|(i, next_index)| (Self(i), next_index))
+            .with_context(|| format!("Failed to read WorkspaceId at index {start_index}"))
+    }
+}
 pub fn init_settings(cx: &mut AppContext) {
     WorkspaceSettings::register(cx);
     ItemSettings::register(cx);
@@ -867,7 +885,7 @@ impl Workspace {
             let workspace_id = if let Some(serialized_workspace) = serialized_workspace.as_ref() {
                 serialized_workspace.id
             } else {
-                DB.next_id().await.unwrap_or(0)
+                DB.next_id().await.unwrap_or_else(|_| Default::default())
             };
 
             let window = if let Some(window) = requesting_window {
@@ -3682,7 +3700,7 @@ impl Workspace {
             build_window_options: |_, _| Default::default(),
             node_runtime: FakeNodeRuntime::new(),
         });
-        let workspace = Self::new(0, project, app_state, cx);
+        let workspace = Self::new(Default::default(), project, app_state, cx);
         workspace.active_pane.update(cx, |pane, cx| pane.focus(cx));
         workspace
     }
@@ -4643,7 +4661,9 @@ pub fn join_hosted_project(
                 let mut options = (app_state.build_window_options)(None, cx);
                 options.bounds = window_bounds_override;
                 cx.open_window(options, |cx| {
-                    cx.new_view(|cx| Workspace::new(0, project, app_state.clone(), cx))
+                    cx.new_view(|cx| {
+                        Workspace::new(Default::default(), project, app_state.clone(), cx)
+                    })
                 })
             })?
         };
@@ -4702,7 +4722,9 @@ pub fn join_in_room_project(
                 let mut options = (app_state.build_window_options)(None, cx);
                 options.bounds = window_bounds_override;
                 cx.open_window(options, |cx| {
-                    cx.new_view(|cx| Workspace::new(0, project, app_state.clone(), cx))
+                    cx.new_view(|cx| {
+                        Workspace::new(Default::default(), project, app_state.clone(), cx)
+                    })
                 })
             })?
         };