Merge #265

265: Refactor symbol resolve API r=matklad a=matklad

Introduce ReferenceResolution to avoid nesting to many non-nominal
types.

Co-authored-by: Aleksey Kladov <[email protected]>

4 files changed, 59 insertions, 24 deletions

diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 99bcf0666..03d17de0d 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -27,6 +27,7 @@ use crate::{
     symbol_index::{SymbolIndex, SymbolsDatabase},
     AnalysisChange, Cancelable, CrateId, Diagnostic, FileId,
     FileSystemEdit, FilePosition, Query, SourceChange, SourceFileNodeEdit,
+    ReferenceResolution,
 };
 
 #[derive(Debug, Default)]
@@ -206,10 +207,11 @@ impl AnalysisImpl {
     pub fn approximately_resolve_symbol(
         &self,
         position: FilePosition,
-    ) -> Cancelable<Option<(TextRange, Vec<(FileId, FileSymbol)>)>> {
+    ) -> Cancelable<Option<ReferenceResolution>> {
         let file = self.db.source_file(position.file_id);
         let syntax = file.syntax();
         if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(syntax, position.offset) {
+            let mut rr = ReferenceResolution::new(name_ref.syntax().range());
             if let Some(fn_descr) = source_binder::function_from_child_node(
                 &*self.db,
                 position.file_id,
@@ -218,24 +220,25 @@ impl AnalysisImpl {
                 let scope = fn_descr.scope(&*self.db);
                 // First try to resolve the symbol locally
                 if let Some(entry) = scope.resolve_local_name(name_ref) {
-                    let vec = vec![(
+                    rr.add_resolution(
                         position.file_id,
                         FileSymbol {
                             name: entry.name().clone(),
                             node_range: entry.ptr().range(),
                             kind: NAME,
                         },
-                    )];
-                    return Ok(Some((name_ref.syntax().range(), vec)));
+                    );
+                    return Ok(Some(rr));
                 };
             }
             // If that fails try the index based approach.
-            return Ok(Some((
-                name_ref.syntax().range(),
-                self.index_resolve(name_ref)?,
-            )));
+            for (file_id, symbol) in self.index_resolve(name_ref)? {
+                rr.add_resolution(file_id, symbol);
+            }
+            return Ok(Some(rr));
         }
         if let Some(name) = find_node_at_offset::<ast::Name>(syntax, position.offset) {
+            let mut rr = ReferenceResolution::new(name.syntax().range());
             if let Some(module) = name.syntax().parent().and_then(ast::Module::cast) {
                 if module.has_semi() {
                     let parent_module =
@@ -250,7 +253,8 @@ impl AnalysisImpl {
                                     node_range: TextRange::offset_len(0.into(), 0.into()),
                                     kind: MODULE,
                                 };
-                                return Ok(Some((name.syntax().range(), vec![(file_id, symbol)])));
+                                rr.add_resolution(file_id, symbol);
+                                return Ok(Some(rr));
                             }
                         }
                         _ => (),
@@ -258,10 +262,7 @@ impl AnalysisImpl {
                 }
             }
         }
-        let range =
-            ctry!(ra_syntax::algo::find_leaf_at_offset(syntax, position.offset).left_biased())
-                .range();
-        Ok(Some((range, vec![])))
+        Ok(None)
     }
 
     pub fn find_all_refs(&self, position: FilePosition) -> Cancelable<Vec<(FileId, TextRange)>> {
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index d33f3e4ca..eaf24cb36 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -178,6 +178,30 @@ impl Query {
     }
 }
 
+/// Result of "goto def" query.
+#[derive(Debug)]
+pub struct ReferenceResolution {
+    /// The range of the reference itself. Client does not know what constitutes
+    /// a reference, it handles us only the offset. It's helpful to tell the
+    /// client where the reference was.
+    pub reference_range: TextRange,
+    /// What this reference resolves to.
+    pub resolves_to: Vec<(FileId, FileSymbol)>,
+}
+
+impl ReferenceResolution {
+    fn new(reference_range: TextRange) -> ReferenceResolution {
+        ReferenceResolution {
+            reference_range,
+            resolves_to: Vec::new(),
+        }
+    }
+
+    fn add_resolution(&mut self, file_id: FileId, symbol: FileSymbol) {
+        self.resolves_to.push((file_id, symbol))
+    }
+}
+
 /// Analysis is a snapshot of a world state at a moment in time. It is the main
 /// entry point for asking semantic information about the world. When the world
 /// state is advanced using `AnalysisHost::apply_change` method, all existing
@@ -236,7 +260,7 @@ impl Analysis {
     pub fn approximately_resolve_symbol(
         &self,
         position: FilePosition,
-    ) -> Cancelable<Option<(TextRange, Vec<(FileId, FileSymbol)>)>> {
+    ) -> Cancelable<Option<ReferenceResolution>> {
         self.imp.approximately_resolve_symbol(position)
     }
     pub fn find_all_refs(&self, position: FilePosition) -> Cancelable<Vec<(FileId, TextRange)>> {
diff --git a/crates/ra_analysis/tests/tests.rs b/crates/ra_analysis/tests/tests.rs
index 05ad687ae..889b568b9 100644
--- a/crates/ra_analysis/tests/tests.rs
+++ b/crates/ra_analysis/tests/tests.rs
@@ -23,7 +23,10 @@ fn approximate_resolve_works_in_items() {
 
     let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
     assert_eq_dbg(
-        r#"([23; 26), [(FileId(1), FileSymbol { name: "Foo", node_range: [0; 11), kind: STRUCT_DEF })])"#,
+        r#"ReferenceResolution {
+            reference_range: [23; 26),
+            resolves_to: [(FileId(1), FileSymbol { name: "Foo", node_range: [0; 11), kind: STRUCT_DEF })]
+        }"#,
         &symbols,
     );
 }
@@ -41,7 +44,10 @@ fn test_resolve_module() {
 
     let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
     assert_eq_dbg(
-        r#"([4; 7), [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })])"#,
+        r#"ReferenceResolution {
+            reference_range: [4; 7),
+            resolves_to: [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })]
+        }"#,
         &symbols,
     );
 
@@ -56,7 +62,10 @@ fn test_resolve_module() {
 
     let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
     assert_eq_dbg(
-        r#"([4; 7), [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })])"#,
+        r#"ReferenceResolution {
+            reference_range: [4; 7),
+            resolves_to: [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })]
+        }"#,
         &symbols,
     );
 }
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index f18a1305d..92e92f836 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -203,11 +203,12 @@ pub fn handle_goto_definition(
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<req::GotoDefinitionResponse>> {
     let position = params.try_conv_with(&world)?;
-    let mut res = Vec::new();
-    for (file_id, symbol) in match world.analysis().approximately_resolve_symbol(position)? {
+    let rr = match world.analysis().approximately_resolve_symbol(position)? {
         None => return Ok(None),
-        Some(it) => it.1,
-    } {
+        Some(it) => it,
+    };
+    let mut res = Vec::new();
+    for (file_id, symbol) in rr.resolves_to {
         let line_index = world.analysis().file_line_index(file_id);
         let location = to_location(file_id, symbol.node_range, &world, &line_index)?;
         res.push(location)
@@ -510,17 +511,17 @@ pub fn handle_hover(
     // TODO: Cut down on number of allocations
     let position = params.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(position.file_id);
-    let (range, resolved) = match world.analysis().approximately_resolve_symbol(position)? {
+    let rr = match world.analysis().approximately_resolve_symbol(position)? {
         None => return Ok(None),
         Some(it) => it,
     };
     let mut result = Vec::new();
-    for (file_id, symbol) in resolved {
+    for (file_id, symbol) in rr.resolves_to {
         if let Some(docs) = world.analysis().doc_text_for(file_id, symbol)? {
             result.push(docs);
         }
     }
-    let range = range.conv_with(&line_index);
+    let range = rr.reference_range.conv_with(&line_index);
     if result.len() > 0 {
         return Ok(Some(Hover {
             contents: HoverContents::Scalar(MarkedString::String(result.join("\n\n---\n"))),