Merge #1311

1311: Move NameRef classification logic out of reference_definition r=matklad a=lnicola



Co-authored-by: Laurențiu Nicola <[email protected]>

4 files changed, 143 insertions, 97 deletions

diff --git a/crates/ra_ide_api/src/display/navigation_target.rs b/crates/ra_ide_api/src/display/navigation_target.rs
index 7ea336c50..1c694cbc9 100644
--- a/crates/ra_ide_api/src/display/navigation_target.rs
+++ b/crates/ra_ide_api/src/display/navigation_target.rs
@@ -5,7 +5,7 @@ use ra_syntax::{
     ast::{self, NameOwner, VisibilityOwner, TypeAscriptionOwner},
     algo::visit::{visitor, Visitor},
 };
-use hir::{ModuleSource, FieldSource, ImplItem, Either};
+use hir::{ModuleSource, FieldSource, ImplItem};
 
 use crate::{FileSymbol, db::RootDatabase};
 
@@ -77,17 +77,12 @@ impl NavigationTarget {
     pub(crate) fn from_pat(
         db: &RootDatabase,
         file_id: FileId,
-        pat: Either<AstPtr<ast::Pat>, AstPtr<ast::SelfParam>>,
+        pat: AstPtr<ast::Pat>,
     ) -> NavigationTarget {
         let file = db.parse(file_id);
-        let (name, full_range) = match pat {
-            Either::A(pat) => match pat.to_node(file.syntax()).kind() {
-                ast::PatKind::BindPat(pat) => {
-                    return NavigationTarget::from_bind_pat(file_id, &pat)
-                }
-                _ => ("_".into(), pat.syntax_node_ptr().range()),
-            },
-            Either::B(slf) => ("self".into(), slf.syntax_node_ptr().range()),
+        let (name, full_range) = match pat.to_node(file.syntax()).kind() {
+            ast::PatKind::BindPat(pat) => return NavigationTarget::from_bind_pat(file_id, &pat),
+            _ => ("_".into(), pat.syntax_node_ptr().range()),
         };
         NavigationTarget {
             file_id,
@@ -99,6 +94,21 @@ impl NavigationTarget {
         }
     }
 
+    pub(crate) fn from_self_param(
+        file_id: FileId,
+        par: AstPtr<ast::SelfParam>,
+    ) -> NavigationTarget {
+        let (name, full_range) = ("self".into(), par.syntax_node_ptr().range());
+        NavigationTarget {
+            file_id,
+            name,
+            full_range,
+            focus_range: None,
+            kind: NAME,
+            container_name: None,
+        }
+    }
+
     pub(crate) fn from_module(db: &RootDatabase, module: hir::Module) -> NavigationTarget {
         let (file_id, source) = module.definition_source(db);
         let file_id = file_id.as_original_file();
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index adae29e9c..9c56f17f2 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -1,12 +1,19 @@
 use ra_db::{FileId, SourceDatabase};
 use ra_syntax::{
     AstNode, ast,
-    algo::{find_node_at_offset, visit::{visitor, Visitor}},
+    algo::{
+        find_node_at_offset,
+        visit::{visitor, Visitor},
+    },
     SyntaxNode,
 };
-use test_utils::tested_by;
 
-use crate::{FilePosition, NavigationTarget, db::RootDatabase, RangeInfo};
+use crate::{
+    FilePosition, NavigationTarget,
+    db::RootDatabase,
+    RangeInfo,
+    name_ref_kind::{NameRefKind::*, classify_name_ref},
+};
 
 pub(crate) fn goto_definition(
     db: &RootDatabase,
@@ -50,85 +57,24 @@ pub(crate) fn reference_definition(
 
     let analyzer = hir::SourceAnalyzer::new(db, file_id, name_ref.syntax(), None);
 
-    // Special cases:
-
-    // Check if it is a method
-    if let Some(method_call) = name_ref.syntax().parent().and_then(ast::MethodCallExpr::cast) {
-        tested_by!(goto_definition_works_for_methods);
-        if let Some(func) = analyzer.resolve_method_call(method_call) {
-            return Exact(NavigationTarget::from_function(db, func));
-        }
-    }
-
-    //it could be a macro call
-    if let Some(macro_call) = name_ref
-        .syntax()
-        .parent()
-        .and_then(|node| node.parent())
-        .and_then(|node| node.parent())
-        .and_then(ast::MacroCall::cast)
-    {
-        tested_by!(goto_definition_works_for_macros);
-        if let Some(macro_call) = analyzer.resolve_macro_call(macro_call) {
-            return Exact(NavigationTarget::from_macro_def(db, macro_call));
-        }
-    }
-
-    // It could also be a field access
-    if let Some(field_expr) = name_ref.syntax().parent().and_then(ast::FieldExpr::cast) {
-        tested_by!(goto_definition_works_for_fields);
-        if let Some(field) = analyzer.resolve_field(field_expr) {
-            return Exact(NavigationTarget::from_field(db, field));
-        };
-    }
-
-    // It could also be a named field
-    if let Some(field_expr) = name_ref.syntax().parent().and_then(ast::NamedField::cast) {
-        tested_by!(goto_definition_works_for_named_fields);
-
-        let struct_lit = field_expr.syntax().ancestors().find_map(ast::StructLit::cast);
-
-        if let Some(ty) = struct_lit.and_then(|lit| analyzer.type_of(db, lit.into())) {
-            if let Some((hir::AdtDef::Struct(s), _)) = ty.as_adt() {
-                let hir_path = hir::Path::from_name_ref(name_ref);
-                let hir_name = hir_path.as_ident().unwrap();
-
-                if let Some(field) = s.field(db, hir_name) {
-                    return Exact(NavigationTarget::from_field(db, field));
-                }
+    match classify_name_ref(db, &analyzer, name_ref) {
+        Some(Method(func)) => return Exact(NavigationTarget::from_function(db, func)),
+        Some(Macro(mac)) => return Exact(NavigationTarget::from_macro_def(db, mac)),
+        Some(FieldAccess(field)) => return Exact(NavigationTarget::from_field(db, field)),
+        Some(AssocItem(assoc)) => return Exact(NavigationTarget::from_impl_item(db, assoc)),
+        Some(Def(def)) => return Exact(NavigationTarget::from_def(db, def)),
+        Some(SelfType(ty)) => {
+            if let Some((def_id, _)) = ty.as_adt() {
+                return Exact(NavigationTarget::from_adt_def(db, def_id));
             }
         }
-    }
-
-    // General case, a path or a local:
-    if let Some(path) = name_ref.syntax().ancestors().find_map(ast::Path::cast) {
-        if let Some(resolved) = analyzer.resolve_path(db, path) {
-            match resolved {
-                hir::PathResolution::Def(def) => return Exact(NavigationTarget::from_def(db, def)),
-                hir::PathResolution::LocalBinding(pat) => {
-                    let nav = NavigationTarget::from_pat(db, file_id, pat);
-                    return Exact(nav);
-                }
-                hir::PathResolution::GenericParam(..) => {
-                    // FIXME: go to the generic param def
-                }
-                hir::PathResolution::Macro(def) => {
-                    let nav = NavigationTarget::from_macro_def(db, def);
-                    return Exact(nav);
-                }
-                hir::PathResolution::SelfType(impl_block) => {
-                    let ty = impl_block.target_ty(db);
-
-                    if let Some((def_id, _)) = ty.as_adt() {
-                        return Exact(NavigationTarget::from_adt_def(db, def_id));
-                    }
-                }
-                hir::PathResolution::AssocItem(assoc) => {
-                    return Exact(NavigationTarget::from_impl_item(db, assoc));
-                }
-            }
+        Some(Pat(pat)) => return Exact(NavigationTarget::from_pat(db, file_id, pat)),
+        Some(SelfParam(par)) => return Exact(NavigationTarget::from_self_param(file_id, par)),
+        Some(GenericParam(_)) => {
+            // FIXME: go to the generic param def
         }
-    }
+        None => {}
+    };
 
     // Fallback index based approach:
     let navs = crate::symbol_index::index_resolve(db, name_ref)
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index d4be8bd6c..f78348f74 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -18,6 +18,7 @@ mod change;
 mod status;
 mod completion;
 mod runnables;
+mod name_ref_kind;
 mod goto_definition;
 mod goto_type_definition;
 mod extend_selection;
@@ -53,10 +54,7 @@ use ra_db::{
 };
 use relative_path::RelativePathBuf;
 
-use crate::{
-    symbol_index::FileSymbol,
-    db::LineIndexDatabase,
-};
+use crate::{symbol_index::FileSymbol, db::LineIndexDatabase};
 
 pub use crate::{
     change::{AnalysisChange, LibraryData},
@@ -73,10 +71,7 @@ pub use crate::{
     display::{FunctionSignature, NavigationTarget, StructureNode, file_structure},
 };
 
-pub use ra_db::{
-    Canceled, CrateGraph, CrateId, FileId, FilePosition, FileRange, SourceRootId,
-    Edition
-};
+pub use ra_db::{Canceled, CrateGraph, CrateId, FileId, FilePosition, FileRange, SourceRootId, Edition};
 pub use hir::Documentation;
 
 // We use jemalloc mainly to get heap usage statistics, actual performance
diff --git a/crates/ra_ide_api/src/name_ref_kind.rs b/crates/ra_ide_api/src/name_ref_kind.rs
new file mode 100644
index 000000000..b498fe495
--- /dev/null
+++ b/crates/ra_ide_api/src/name_ref_kind.rs
@@ -0,0 +1,95 @@
+use ra_syntax::{AstNode, AstPtr, ast};
+use hir::Either;
+use crate::db::RootDatabase;
+use test_utils::tested_by;
+
+pub enum NameRefKind {
+    Method(hir::Function),
+    Macro(hir::MacroByExampleDef),
+    FieldAccess(hir::StructField),
+    AssocItem(hir::ImplItem),
+    Def(hir::ModuleDef),
+    SelfType(hir::Ty),
+    Pat(AstPtr<ast::Pat>),
+    SelfParam(AstPtr<ast::SelfParam>),
+    GenericParam(u32),
+}
+
+pub(crate) fn classify_name_ref(
+    db: &RootDatabase,
+    analyzer: &hir::SourceAnalyzer,
+    name_ref: &ast::NameRef,
+) -> Option<NameRefKind> {
+    use NameRefKind::*;
+
+    // Check if it is a method
+    if let Some(method_call) = name_ref.syntax().parent().and_then(ast::MethodCallExpr::cast) {
+        tested_by!(goto_definition_works_for_methods);
+        if let Some(func) = analyzer.resolve_method_call(method_call) {
+            return Some(Method(func));
+        }
+    }
+
+    // It could be a macro call
+    if let Some(macro_call) = name_ref
+        .syntax()
+        .parent()
+        .and_then(|node| node.parent())
+        .and_then(|node| node.parent())
+        .and_then(ast::MacroCall::cast)
+    {
+        tested_by!(goto_definition_works_for_macros);
+        if let Some(mac) = analyzer.resolve_macro_call(macro_call) {
+            return Some(Macro(mac));
+        }
+    }
+
+    // It could also be a field access
+    if let Some(field_expr) = name_ref.syntax().parent().and_then(ast::FieldExpr::cast) {
+        tested_by!(goto_definition_works_for_fields);
+        if let Some(field) = analyzer.resolve_field(field_expr) {
+            return Some(FieldAccess(field));
+        };
+    }
+
+    // It could also be a named field
+    if let Some(field_expr) = name_ref.syntax().parent().and_then(ast::NamedField::cast) {
+        tested_by!(goto_definition_works_for_named_fields);
+
+        let struct_lit = field_expr.syntax().ancestors().find_map(ast::StructLit::cast);
+
+        if let Some(ty) = struct_lit.and_then(|lit| analyzer.type_of(db, lit.into())) {
+            if let Some((hir::AdtDef::Struct(s), _)) = ty.as_adt() {
+                let hir_path = hir::Path::from_name_ref(name_ref);
+                let hir_name = hir_path.as_ident().unwrap();
+
+                if let Some(field) = s.field(db, hir_name) {
+                    return Some(FieldAccess(field));
+                }
+            }
+        }
+    }
+
+    // General case, a path or a local:
+    if let Some(path) = name_ref.syntax().ancestors().find_map(ast::Path::cast) {
+        if let Some(resolved) = analyzer.resolve_path(db, path) {
+            return match resolved {
+                hir::PathResolution::Def(def) => Some(Def(def)),
+                hir::PathResolution::LocalBinding(Either::A(pat)) => Some(Pat(pat)),
+                hir::PathResolution::LocalBinding(Either::B(par)) => Some(SelfParam(par)),
+                hir::PathResolution::GenericParam(par) => {
+                    // FIXME: get generic param def
+                    Some(GenericParam(par))
+                }
+                hir::PathResolution::Macro(def) => Some(Macro(def)),
+                hir::PathResolution::SelfType(impl_block) => {
+                    let ty = impl_block.target_ty(db);
+                    Some(SelfType(ty))
+                }
+                hir::PathResolution::AssocItem(assoc) => Some(AssocItem(assoc)),
+            };
+        }
+    }
+
+    None
+}