project: Fix issue where Cmd+Click on an import opens the wrong file (#26120)

smit created

Closes #21974

`resolve_path_in_worktrees` function looks for provided path in each
worktree until valid file is found.

In this PR we priortize current buffer worktree before other worktrees,
because of edge case where, file with same name might exists in other
worktrees.

Updated tests to handle this case.

Release Notes:

- Fixed an issue where the wrong file from a different worktree would
open when using `Cmd + Click` on a file import.

Change summary

crates/project/src/project.rs                    | 81 ++++++++++++-----
crates/remote_server/src/remote_editing_tests.rs | 36 +++++--
2 files changed, 81 insertions(+), 36 deletions(-)

Detailed changes

crates/project/src/project.rs 🔗

@@ -3462,34 +3462,39 @@ impl Project {
             }
         }
 
-        let worktrees = self.worktrees(cx).collect::<Vec<_>>();
+        let buffer_worktree_id = buffer.read(cx).file().map(|file| file.worktree_id(cx));
+        let worktrees_with_ids: Vec<_> = self
+            .worktrees(cx)
+            .map(|worktree| {
+                let id = worktree.read(cx).id();
+                (worktree, id)
+            })
+            .collect();
+
         cx.spawn(|_, mut cx| async move {
-            for worktree in worktrees {
+            if let Some(buffer_worktree_id) = buffer_worktree_id {
+                if let Some((worktree, _)) = worktrees_with_ids
+                    .iter()
+                    .find(|(_, id)| *id == buffer_worktree_id)
+                {
+                    for candidate in candidates.iter() {
+                        if let Some(path) =
+                            Self::resolve_path_in_worktree(&worktree, candidate, &mut cx)
+                        {
+                            return Some(path);
+                        }
+                    }
+                }
+            }
+            for (worktree, id) in worktrees_with_ids {
+                if Some(id) == buffer_worktree_id {
+                    continue;
+                }
                 for candidate in candidates.iter() {
-                    let path = worktree
-                        .update(&mut cx, |worktree, _| {
-                            let root_entry_path = &worktree.root_entry()?.path;
-
-                            let resolved = resolve_path(root_entry_path, candidate);
-
-                            let stripped =
-                                resolved.strip_prefix(root_entry_path).unwrap_or(&resolved);
-
-                            worktree.entry_for_path(stripped).map(|entry| {
-                                let project_path = ProjectPath {
-                                    worktree_id: worktree.id(),
-                                    path: entry.path.clone(),
-                                };
-                                ResolvedPath::ProjectPath {
-                                    project_path,
-                                    is_dir: entry.is_dir(),
-                                }
-                            })
-                        })
-                        .ok()?;
-
-                    if path.is_some() {
-                        return path;
+                    if let Some(path) =
+                        Self::resolve_path_in_worktree(&worktree, candidate, &mut cx)
+                    {
+                        return Some(path);
                     }
                 }
             }
@@ -3497,6 +3502,30 @@ impl Project {
         })
     }
 
+    fn resolve_path_in_worktree(
+        worktree: &Entity<Worktree>,
+        path: &PathBuf,
+        cx: &mut AsyncApp,
+    ) -> Option<ResolvedPath> {
+        worktree
+            .update(cx, |worktree, _| {
+                let root_entry_path = &worktree.root_entry()?.path;
+                let resolved = resolve_path(root_entry_path, path);
+                let stripped = resolved.strip_prefix(root_entry_path).unwrap_or(&resolved);
+                worktree.entry_for_path(stripped).map(|entry| {
+                    let project_path = ProjectPath {
+                        worktree_id: worktree.id(),
+                        path: entry.path.clone(),
+                    };
+                    ResolvedPath::ProjectPath {
+                        project_path,
+                        is_dir: entry.is_dir(),
+                    }
+                })
+            })
+            .ok()?
+    }
+
     pub fn list_directory(
         &self,
         query: String,

crates/remote_server/src/remote_editing_tests.rs 🔗

@@ -787,6 +787,7 @@ async fn test_remote_resolve_path_in_buffer(
     server_cx: &mut TestAppContext,
 ) {
     let fs = FakeFs::new(server_cx.executor());
+    // Even though we are not testing anything from project1, it is necessary to test if project2 is picking up correct worktree
     fs.insert_tree(
         path!("/code"),
         json!({
@@ -797,60 +798,75 @@ async fn test_remote_resolve_path_in_buffer(
                     "lib.rs": "fn one() -> usize { 1 }"
                 }
             },
+            "project2": {
+                ".git": {},
+                "README.md": "# project 2",
+                "src": {
+                    "lib.rs": "fn two() -> usize { 2 }"
+                }
+            }
         }),
     )
     .await;
 
     let (project, _headless) = init_test(&fs, cx, server_cx).await;
-    let (worktree, _) = project
+
+    let _ = project
         .update(cx, |project, cx| {
             project.find_or_create_worktree(path!("/code/project1"), true, cx)
         })
         .await
         .unwrap();
 
-    let worktree_id = cx.update(|cx| worktree.read(cx).id());
+    let (worktree2, _) = project
+        .update(cx, |project, cx| {
+            project.find_or_create_worktree(path!("/code/project2"), true, cx)
+        })
+        .await
+        .unwrap();
 
-    let buffer = project
+    let worktree2_id = cx.update(|cx| worktree2.read(cx).id());
+
+    let buffer2 = project
         .update(cx, |project, cx| {
-            project.open_buffer((worktree_id, Path::new("src/lib.rs")), cx)
+            project.open_buffer((worktree2_id, Path::new("src/lib.rs")), cx)
         })
         .await
         .unwrap();
 
     let path = project
         .update(cx, |project, cx| {
-            project.resolve_path_in_buffer(path!("/code/project1/README.md"), &buffer, cx)
+            project.resolve_path_in_buffer(path!("/code/project2/README.md"), &buffer2, cx)
         })
         .await
         .unwrap();
     assert!(path.is_file());
     assert_eq!(
         path.abs_path().unwrap().to_string_lossy(),
-        path!("/code/project1/README.md")
+        path!("/code/project2/README.md")
     );
 
     let path = project
         .update(cx, |project, cx| {
-            project.resolve_path_in_buffer("../README.md", &buffer, cx)
+            project.resolve_path_in_buffer("../README.md", &buffer2, cx)
         })
         .await
         .unwrap();
     assert!(path.is_file());
     assert_eq!(
         path.project_path().unwrap().clone(),
-        ProjectPath::from((worktree_id, "README.md"))
+        ProjectPath::from((worktree2_id, "README.md"))
     );
 
     let path = project
         .update(cx, |project, cx| {
-            project.resolve_path_in_buffer("../src", &buffer, cx)
+            project.resolve_path_in_buffer("../src", &buffer2, cx)
         })
         .await
         .unwrap();
     assert_eq!(
         path.project_path().unwrap().clone(),
-        ProjectPath::from((worktree_id, "src"))
+        ProjectPath::from((worktree2_id, "src"))
     );
     assert!(path.is_dir());
 }