Fall back to FindAllReferences if GoToDefinition have not navigated (#16512)

Follow-up of https://github.com/zed-industries/zed/pull/9243 

Release Notes:

- N/A

---------

Co-authored-by: Alex Kladov <aleksey.kladov@gmail.com>

crates/editor/src/editor.rs

@@ -266,6 +266,22 @@ pub enum Direction {
     Next,
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum Navigated {
+    Yes,
+    No,
+}
+
+impl Navigated {
+    pub fn from_bool(yes: bool) -> Navigated {
+        if yes {
+            Navigated::Yes
+        } else {
+            Navigated::No
+        }
+    }
+}
+
 pub fn init_settings(cx: &mut AppContext) {
     EditorSettings::register(cx);
 }
@@ -1561,6 +1577,7 @@ pub(crate) struct NavigationData {
     scroll_top_row: u32,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum GotoDefinitionKind {
     Symbol,
     Declaration,
@@ -4843,7 +4860,7 @@ impl Editor {
 
                             let range = Anchor {
                                 buffer_id,
-                                excerpt_id: excerpt_id,
+                                excerpt_id,
                                 text_anchor: start,
                             }..Anchor {
                                 buffer_id,
@@ -9020,15 +9037,28 @@ impl Editor {
         &mut self,
         _: &GoToDefinition,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
-        self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, false, cx)
+        let definition = self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, false, cx);
+        let references = self.find_all_references(&FindAllReferences, cx);
+        cx.background_executor().spawn(async move {
+            if definition.await? == Navigated::Yes {
+                return Ok(Navigated::Yes);
+            }
+            if let Some(references) = references {
+                if references.await? == Navigated::Yes {
+                    return Ok(Navigated::Yes);
+                }
+            }
+
+            Ok(Navigated::No)
+        })
     }
 
     pub fn go_to_declaration(
         &mut self,
         _: &GoToDeclaration,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Declaration, false, cx)
     }
 
@@ -9036,7 +9066,7 @@ impl Editor {
         &mut self,
         _: &GoToDeclaration,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Declaration, true, cx)
     }
 
@@ -9044,7 +9074,7 @@ impl Editor {
         &mut self,
         _: &GoToImplementation,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, false, cx)
     }
 
@@ -9052,7 +9082,7 @@ impl Editor {
         &mut self,
         _: &GoToImplementationSplit,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, true, cx)
     }
 
@@ -9060,7 +9090,7 @@ impl Editor {
         &mut self,
         _: &GoToTypeDefinition,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Type, false, cx)
     }
 
@@ -9068,7 +9098,7 @@ impl Editor {
         &mut self,
         _: &GoToDefinitionSplit,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, true, cx)
     }
 
@@ -9076,7 +9106,7 @@ impl Editor {
         &mut self,
         _: &GoToTypeDefinitionSplit,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         self.go_to_definition_of_kind(GotoDefinitionKind::Type, true, cx)
     }
 
@@ -9085,16 +9115,16 @@ impl Editor {
         kind: GotoDefinitionKind,
         split: bool,
         cx: &mut ViewContext<Self>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         let Some(workspace) = self.workspace() else {
-            return Task::ready(Ok(false));
+            return Task::ready(Ok(Navigated::No));
         };
         let buffer = self.buffer.read(cx);
         let head = self.selections.newest::<usize>(cx).head();
         let (buffer, head) = if let Some(text_anchor) = buffer.text_anchor_for_position(head, cx) {
             text_anchor
         } else {
-            return Task::ready(Ok(false));
+            return Task::ready(Ok(Navigated::No));
         };
 
         let project = workspace.read(cx).project().clone();
@@ -9153,7 +9183,7 @@ impl Editor {
         mut definitions: Vec<HoverLink>,
         split: bool,
         cx: &mut ViewContext<Editor>,
-    ) -> Task<Result<bool>> {
+    ) -> Task<Result<Navigated>> {
         // If there is one definition, just open it directly
         if definitions.len() == 1 {
             let definition = definitions.pop().unwrap();
@@ -9169,77 +9199,61 @@ impl Editor {
             };
             cx.spawn(|editor, mut cx| async move {
                 let target = target_task.await.context("target resolution task")?;
-                if let Some(target) = target {
+                let Some(target) = target else {
-                    editor.update(&mut cx, |editor, cx| {
+                    return Ok(Navigated::No);
-                        let Some(workspace) = editor.workspace() else {
+                };
-                            return false;
+                editor.update(&mut cx, |editor, cx| {
-                        };
+                    let Some(workspace) = editor.workspace() else {
-                        let pane = workspace.read(cx).active_pane().clone();
+                        return Navigated::No;
+                    };
+                    let pane = workspace.read(cx).active_pane().clone();
 
-                        let range = target.range.to_offset(target.buffer.read(cx));
+                    let range = target.range.to_offset(target.buffer.read(cx));
-                        let range = editor.range_for_match(&range);
+                    let range = editor.range_for_match(&range);
 
-                        /// If select range has more than one line, we
+                    if Some(&target.buffer) == editor.buffer.read(cx).as_singleton().as_ref() {
-                        /// just point the cursor to range.start.
+                        let buffer = target.buffer.read(cx);
-                        fn check_multiline_range(
+                        let range = check_multiline_range(buffer, range);
-                            buffer: &Buffer,
+                        editor.change_selections(Some(Autoscroll::focused()), cx, |s| {
-                            range: Range<usize>,
+                            s.select_ranges([range]);
-                        ) -> Range<usize> {
+                        });
-                            if buffer.offset_to_point(range.start).row
+                    } else {
-                                == buffer.offset_to_point(range.end).row
+                        cx.window_context().defer(move |cx| {
-                            {
+                            let target_editor: View<Self> =
-                                range
+                                workspace.update(cx, |workspace, cx| {
-                            } else {
+                                    let pane = if split {
-                                range.start..range.start
+                                        workspace.adjacent_pane(cx)
-                            }
+                                    } else {
-                        }
+                                        workspace.active_pane().clone()
+                                    };
 
-                        if Some(&target.buffer) == editor.buffer.read(cx).as_singleton().as_ref() {
+                                    workspace.open_project_item(
-                            let buffer = target.buffer.read(cx);
+                                        pane,
-                            let range = check_multiline_range(buffer, range);
+                                        target.buffer.clone(),
-                            editor.change_selections(Some(Autoscroll::focused()), cx, |s| {
+                                        true,
-                                s.select_ranges([range]);
+                                        true,
-                            });
-                        } else {
-                            cx.window_context().defer(move |cx| {
-                                let target_editor: View<Self> =
-                                    workspace.update(cx, |workspace, cx| {
-                                        let pane = if split {
-                                            workspace.adjacent_pane(cx)
-                                        } else {
-                                            workspace.active_pane().clone()
-                                        };
-
-                                        workspace.open_project_item(
-                                            pane,
-                                            target.buffer.clone(),
-                                            true,
-                                            true,
-                                            cx,
-                                        )
-                                    });
-                                target_editor.update(cx, |target_editor, cx| {
-                                    // When selecting a definition in a different buffer, disable the nav history
-                                    // to avoid creating a history entry at the previous cursor location.
-                                    pane.update(cx, |pane, _| pane.disable_history());
-                                    let buffer = target.buffer.read(cx);
-                                    let range = check_multiline_range(buffer, range);
-                                    target_editor.change_selections(
-                                        Some(Autoscroll::focused()),
                                         cx,
-                                        |s| {
+                                    )
-                                            s.select_ranges([range]);
-                                        },
-                                    );
-                                    pane.update(cx, |pane, _| pane.enable_history());
                                 });
+                            target_editor.update(cx, |target_editor, cx| {
+                                // When selecting a definition in a different buffer, disable the nav history
+                                // to avoid creating a history entry at the previous cursor location.
+                                pane.update(cx, |pane, _| pane.disable_history());
+                                let buffer = target.buffer.read(cx);
+                                let range = check_multiline_range(buffer, range);
+                                target_editor.change_selections(
+                                    Some(Autoscroll::focused()),
+                                    cx,
+                                    |s| {
+                                        s.select_ranges([range]);
+                                    },
+                                );
+                                pane.update(cx, |pane, _| pane.enable_history());
                             });
-                        }
+                        });
-                        true
+                    }
-                    })
+                    Navigated::Yes
-                } else {
+                })
-                    Ok(false)
-                }
             })
         } else if !definitions.is_empty() {
             let replica_id = self.replica_id(cx);
@@ -9289,7 +9303,7 @@ impl Editor {
                     .context("location tasks")?;
 
                 let Some(workspace) = workspace else {
-                    return Ok(false);
+                    return Ok(Navigated::No);
                 };
                 let opened = workspace
                     .update(&mut cx, |workspace, cx| {
@@ -9299,10 +9313,10 @@ impl Editor {
                     })
                     .ok();
 
-                anyhow::Ok(opened.is_some())
+                anyhow::Ok(Navigated::from_bool(opened.is_some()))
             })
         } else {
-            Task::ready(Ok(false))
+            Task::ready(Ok(Navigated::No))
         }
     }
 
@@ -9361,7 +9375,7 @@ impl Editor {
         &mut self,
         _: &FindAllReferences,
         cx: &mut ViewContext<Self>,
-    ) -> Option<Task<Result<()>>> {
+    ) -> Option<Task<Result<Navigated>>> {
         let multi_buffer = self.buffer.read(cx);
         let selection = self.selections.newest::<usize>(cx);
         let head = selection.head();
@@ -9416,7 +9430,7 @@ impl Editor {
 
             let locations = references.await?;
             if locations.is_empty() {
-                return anyhow::Ok(());
+                return anyhow::Ok(Navigated::No);
             }
 
             workspace.update(&mut cx, |workspace, cx| {
@@ -9436,6 +9450,7 @@ impl Editor {
                 Self::open_locations_in_multibuffer(
                     workspace, locations, replica_id, title, false, cx,
                 );
+                Navigated::Yes
             })
         }))
     }
@@ -13277,3 +13292,13 @@ fn hunk_status(hunk: &DiffHunk<MultiBufferRow>) -> DiffHunkStatus {
         DiffHunkStatus::Modified
     }
 }
+
+/// If select range has more than one line, we
+/// just point the cursor to range.start.
+fn check_multiline_range(buffer: &Buffer, range: Range<usize>) -> Range<usize> {
+    if buffer.offset_to_point(range.start).row == buffer.offset_to_point(range.end).row {
+        range
+    } else {
+        range.start..range.start
+    }
+}

crates/editor/src/editor_tests.rs

@@ -13221,6 +13221,127 @@ let foo = 15;"#,
     });
 }
 
+#[gpui::test]
+async fn test_goto_definition_with_find_all_references_fallback(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            definition_provider: Some(lsp::OneOf::Left(true)),
+            references_provider: Some(lsp::OneOf::Left(true)),
+            ..lsp::ServerCapabilities::default()
+        },
+        cx,
+    )
+    .await;
+
+    let set_up_lsp_handlers = |empty_go_to_definition: bool, cx: &mut EditorLspTestContext| {
+        let go_to_definition = cx.lsp.handle_request::<lsp::request::GotoDefinition, _, _>(
+            move |params, _| async move {
+                if empty_go_to_definition {
+                    Ok(None)
+                } else {
+                    Ok(Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location {
+                        uri: params.text_document_position_params.text_document.uri,
+                        range: lsp::Range::new(lsp::Position::new(4, 3), lsp::Position::new(4, 6)),
+                    })))
+                }
+            },
+        );
+        let references =
+            cx.lsp
+                .handle_request::<lsp::request::References, _, _>(move |params, _| async move {
+                    Ok(Some(vec![lsp::Location {
+                        uri: params.text_document_position.text_document.uri,
+                        range: lsp::Range::new(lsp::Position::new(0, 8), lsp::Position::new(0, 11)),
+                    }]))
+                });
+        (go_to_definition, references)
+    };
+
+    cx.set_state(
+        &r#"fn one() {
+            let mut a = ˇtwo();
+        }
+
+        fn two() {}"#
+            .unindent(),
+    );
+    set_up_lsp_handlers(false, &mut cx);
+    let navigated = cx
+        .update_editor(|editor, cx| editor.go_to_definition(&GoToDefinition, cx))
+        .await
+        .expect("Failed to navigate to definition");
+    assert_eq!(
+        navigated,
+        Navigated::Yes,
+        "Should have navigated to definition from the GetDefinition response"
+    );
+    cx.assert_editor_state(
+        &r#"fn one() {
+            let mut a = two();
+        }
+
+        fn «twoˇ»() {}"#
+            .unindent(),
+    );
+
+    let editors = cx.update_workspace(|workspace, cx| {
+        workspace.items_of_type::<Editor>(cx).collect::<Vec<_>>()
+    });
+    cx.update_editor(|_, test_editor_cx| {
+        assert_eq!(
+            editors.len(),
+            1,
+            "Initially, only one, test, editor should be open in the workspace"
+        );
+        assert_eq!(
+            test_editor_cx.view(),
+            editors.last().expect("Asserted len is 1")
+        );
+    });
+
+    set_up_lsp_handlers(true, &mut cx);
+    let navigated = cx
+        .update_editor(|editor, cx| editor.go_to_definition(&GoToDefinition, cx))
+        .await
+        .expect("Failed to navigate to lookup references");
+    assert_eq!(
+        navigated,
+        Navigated::Yes,
+        "Should have navigated to references as a fallback after empty GoToDefinition response"
+    );
+    // We should not change the selections in the existing file,
+    // if opening another milti buffer with the references
+    cx.assert_editor_state(
+        &r#"fn one() {
+            let mut a = two();
+        }
+
+        fn «twoˇ»() {}"#
+            .unindent(),
+    );
+    let editors = cx.update_workspace(|workspace, cx| {
+        workspace.items_of_type::<Editor>(cx).collect::<Vec<_>>()
+    });
+    cx.update_editor(|_, test_editor_cx| {
+        assert_eq!(
+            editors.len(),
+            2,
+            "After falling back to references search, we open a new editor with the results"
+        );
+        let references_fallback_text = editors
+            .into_iter()
+            .find(|new_editor| new_editor != test_editor_cx.view())
+            .expect("Should have one non-test editor now")
+            .read(test_editor_cx)
+            .text(test_editor_cx);
+        assert_eq!(
+            references_fallback_text, "fn one() {\n    let mut a = two();\n}",
+            "Should use the range from the references response and not the GoToDefinition one"
+        );
+    });
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(DisplayRow(row as u32), column as u32);
     point..point

crates/editor/src/hover_links.rs

@@ -2,7 +2,7 @@ use crate::{
     hover_popover::{self, InlayHover},
     scroll::ScrollAmount,
     Anchor, Editor, EditorSnapshot, FindAllReferences, GoToDefinition, GoToTypeDefinition, InlayId,
-    PointForPosition, SelectPhase,
+    Navigated, PointForPosition, SelectPhase,
 };
 use gpui::{px, AppContext, AsyncWindowContext, Model, Modifiers, Task, ViewContext};
 use language::{Bias, ToOffset};
@@ -157,10 +157,10 @@ impl Editor {
     ) {
         let reveal_task = self.cmd_click_reveal_task(point, modifiers, cx);
         cx.spawn(|editor, mut cx| async move {
-            let definition_revealed = reveal_task.await.log_err().unwrap_or(false);
+            let definition_revealed = reveal_task.await.log_err().unwrap_or(Navigated::No);
             let find_references = editor
                 .update(&mut cx, |editor, cx| {
-                    if definition_revealed {
+                    if definition_revealed == Navigated::Yes {
                         return None;
                     }
                     editor.find_all_references(&FindAllReferences, cx)
@@ -194,7 +194,7 @@ impl Editor {
         point: PointForPosition,
         modifiers: Modifiers,
         cx: &mut ViewContext<Editor>,
-    ) -> Task<anyhow::Result<bool>> {
+    ) -> Task<anyhow::Result<Navigated>> {
         if let Some(hovered_link_state) = self.hovered_link_state.take() {
             self.hide_hovered_link(cx);
             if !hovered_link_state.links.is_empty() {
@@ -211,7 +211,7 @@ impl Editor {
                     .read(cx)
                     .text_anchor_for_position(current_position, cx)
                 else {
-                    return Task::ready(Ok(false));
+                    return Task::ready(Ok(Navigated::No));
                 };
                 let links = hovered_link_state
                     .links
@@ -247,7 +247,7 @@ impl Editor {
                 self.go_to_definition(&GoToDefinition, cx)
             }
         } else {
-            Task::ready(Ok(false))
+            Task::ready(Ok(Navigated::No))
         }
     }
 }

crates/project/src/project.rs

@@ -232,6 +232,7 @@ pub struct Project {
     cached_shell_environments: HashMap<WorktreeId, HashMap<String, String>>,
 }
 
+#[derive(Debug)]
 pub enum LanguageServerToQuery {
     Primary,
     Other(LanguageServerId),