From 31dfd01fdaeaae638ea64d6494394f04282e182b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 22 Jan 2022 11:14:50 +0100 Subject: [PATCH] Make `add_local_worktree` private and use `find_or_create_local_worktree` The former always adds a worktree, even if we have one already in the project and that could be misused. The public API should always search for a local worktree containing the requested path first so that the project can uphold invariants about which worktrees it has. --- crates/diagnostics/src/diagnostics.rs | 2 +- crates/file_finder/src/file_finder.rs | 10 +-- crates/project/src/project.rs | 87 ++++++++++++----------- crates/project_panel/src/project_panel.rs | 4 +- crates/server/src/rpc.rs | 60 ++++++++++------ crates/workspace/src/workspace.rs | 2 +- crates/zed/src/zed.rs | 12 ++-- 7 files changed, 98 insertions(+), 79 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 88955255be..eedb77c9be 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -765,7 +765,7 @@ mod tests { project .update(&mut cx, |project, cx| { - project.add_local_worktree("/test", false, cx) + project.find_or_create_local_worktree("/test", false, cx) }) .await .unwrap(); diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 99a60cf28d..588a80593e 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -457,7 +457,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -518,7 +518,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/dir"), false, cx) + project.find_or_create_local_worktree("/dir", false, cx) }) .await .unwrap(); @@ -584,11 +584,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path( - Path::new("/root/the-parent-dir/the-file"), - false, - cx, - ) + project.find_or_create_local_worktree("/root/the-parent-dir/the-file", false, cx) }) .await .unwrap(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index bceb151116..d0b42eaaaf 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -568,7 +568,7 @@ impl Project { abs_path: PathBuf, cx: &mut ModelContext, ) -> Task> { - let worktree_task = self.find_or_create_worktree_for_abs_path(&abs_path, false, cx); + let worktree_task = self.find_or_create_local_worktree(&abs_path, false, cx); cx.spawn(|this, mut cx| async move { let (worktree, path) = worktree_task.await?; worktree @@ -890,7 +890,7 @@ impl Project { cx: &mut ModelContext, ) -> Result<(), anyhow::Error> { let (worktree, relative_path) = self - .find_worktree_for_abs_path(&abs_path, cx) + .find_local_worktree(&abs_path, cx) .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), @@ -995,15 +995,14 @@ impl Project { .to_file_path() .map_err(|_| anyhow!("invalid target path"))?; - let (worktree, relative_path) = if let Some(result) = this - .read_with(&cx, |this, cx| { - this.find_worktree_for_abs_path(&abs_path, cx) - }) { + let (worktree, relative_path) = if let Some(result) = + this.read_with(&cx, |this, cx| this.find_local_worktree(&abs_path, cx)) + { result } else { - let (worktree, relative_path) = this + let worktree = this .update(&mut cx, |this, cx| { - this.create_worktree_for_abs_path(&abs_path, true, cx) + this.create_local_worktree(&abs_path, true, cx) }) .await?; this.update(&mut cx, |this, cx| { @@ -1012,7 +1011,7 @@ impl Project { lang_server.clone(), ); }); - (worktree, relative_path) + (worktree, PathBuf::new()) }; let project_path = ProjectPath { @@ -1045,34 +1044,23 @@ impl Project { } } - pub fn find_or_create_worktree_for_abs_path( + pub fn find_or_create_local_worktree( &self, abs_path: impl AsRef, weak: bool, cx: &mut ModelContext, ) -> Task, PathBuf)>> { let abs_path = abs_path.as_ref(); - if let Some((tree, relative_path)) = self.find_worktree_for_abs_path(abs_path, cx) { + if let Some((tree, relative_path)) = self.find_local_worktree(abs_path, cx) { Task::ready(Ok((tree.clone(), relative_path.into()))) } else { - self.create_worktree_for_abs_path(abs_path, weak, cx) + let worktree = self.create_local_worktree(abs_path, weak, cx); + cx.foreground() + .spawn(async move { Ok((worktree.await?, PathBuf::new())) }) } } - fn create_worktree_for_abs_path( - &self, - abs_path: &Path, - weak: bool, - cx: &mut ModelContext, - ) -> Task, PathBuf)>> { - let worktree = self.add_local_worktree(abs_path, weak, cx); - cx.background().spawn(async move { - let worktree = worktree.await?; - Ok((worktree, PathBuf::new())) - }) - } - - fn find_worktree_for_abs_path( + fn find_local_worktree( &self, abs_path: &Path, cx: &AppContext, @@ -1096,7 +1084,7 @@ impl Project { } } - pub fn add_local_worktree( + fn create_local_worktree( &self, abs_path: impl AsRef, weak: bool, @@ -1909,7 +1897,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(&root_link_path, false, cx) + project.find_or_create_local_worktree(&root_link_path, false, cx) }) .await .unwrap(); @@ -1984,7 +1972,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(dir.path(), false, cx) + project.find_or_create_local_worktree(dir.path(), false, cx) }) .await .unwrap(); @@ -2090,7 +2078,7 @@ mod tests { let project = build_project(Arc::new(RealFs), &mut cx); let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(&dir.path(), false, cx) + project.find_or_create_local_worktree(&dir.path(), false, cx) }) .await .unwrap(); @@ -2144,7 +2132,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(dir.path().join("b.rs"), false, cx) + project.find_or_create_local_worktree(dir.path().join("b.rs"), false, cx) }) .await .unwrap(); @@ -2241,9 +2229,12 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project - .update(&mut cx, |p, cx| p.add_local_worktree("/dir", false, cx)) + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); let buffer = project @@ -2277,10 +2268,11 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project .update(&mut cx, |p, cx| { - p.add_local_worktree("/dir/file1", false, cx) + p.find_or_create_local_worktree("/dir/file1", false, cx) }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); let buffer = project @@ -2318,8 +2310,10 @@ mod tests { let project = build_project(Arc::new(RealFs), &mut cx); let rpc = project.read_with(&cx, |p, _| p.client.clone()); - let tree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (tree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = tree.read_with(&cx, |tree, _| tree.id()); @@ -2460,9 +2454,12 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project - .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/the-dir", false, cx) + }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); // Spawn multiple tasks to open paths, repeating some paths. @@ -2506,8 +2503,10 @@ mod tests { })); let project = build_project(Arc::new(RealFs), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |worktree, _| worktree.id()); @@ -2638,8 +2637,10 @@ mod tests { let dir = temp_tree(json!({ "the-file": initial_contents })); let project = build_project(Arc::new(RealFs), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); @@ -2747,8 +2748,10 @@ mod tests { .await; let project = build_project(fs.clone(), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/the-dir", false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 781c4cd586..643891f5d4 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -644,7 +644,7 @@ mod tests { }); let (root1, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path("/root1", false, cx) + project.find_or_create_local_worktree("/root1", false, cx) }) .await .unwrap(); @@ -653,7 +653,7 @@ mod tests { .await; let (root2, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path("/root2", false, cx) + project.find_or_create_local_worktree("/root2", false, cx) }) .await .unwrap(); diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 8032a85eef..4ad2382699 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1160,8 +1160,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); @@ -1296,8 +1298,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1396,8 +1400,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1546,8 +1552,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1639,8 +1647,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1717,8 +1727,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1791,8 +1803,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1878,8 +1892,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -2096,8 +2112,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -2601,8 +2619,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index ef91d4b015..8dc98c852b 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -723,7 +723,7 @@ impl Workspace { cx: &mut ViewContext, ) -> Task> { let entry = self.project().update(cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(abs_path, false, cx) + project.find_or_create_local_worktree(abs_path, false, cx) }); cx.spawn(|_, cx| async move { let (worktree, path) = entry.await?; diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 5f5422d256..fc7ae9a3c5 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -245,7 +245,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -358,7 +358,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/dir1"), false, cx) + project.find_or_create_local_worktree("/dir1", false, cx) }) .await .unwrap(); @@ -425,7 +425,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -474,7 +474,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -626,7 +626,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -689,7 +689,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap();