Merge #3274

3274: Simplify r=matklad a=matklad



bors r+
🤖

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

crates/ra_ide/src/display.rs

@@ -15,7 +15,7 @@
 pub use navigation_target::NavigationTarget;
 pub use structure::{file_structure, StructureNode};
 
-pub(crate) use navigation_target::ToNav;
+pub(crate) use navigation_target::{ToNav, TryToNav};
 pub(crate) use short_label::ShortLabel;
 
 pub(crate) fn function_label(node: &ast::FnDef) -> String {

crates/ra_ide/src/display/navigation_target.rs

@@ -11,7 +11,7 @@
     TextRange,
 };
 
-use crate::{expand::original_range, FileSymbol};
+use crate::{expand::original_range, references::NameDefinition, FileSymbol};
 
 use super::short_label::ShortLabel;
 
@@ -36,6 +36,10 @@ pub(crate) trait ToNav {
     fn to_nav(&self, db: &RootDatabase) -> NavigationTarget;
 }
 
+pub(crate) trait TryToNav {
+    fn try_to_nav(&self, db: &RootDatabase) -> Option<NavigationTarget>;
+}
+
 impl NavigationTarget {
     /// When `focus_range` is specified, returns it. otherwise
     /// returns `full_range`
@@ -96,26 +100,6 @@ pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> Nav
         module.to_nav(db)
     }
 
-    pub(crate) fn from_def(
-        db: &RootDatabase,
-        module_def: hir::ModuleDef,
-    ) -> Option<NavigationTarget> {
-        let nav = match module_def {
-            hir::ModuleDef::Module(module) => module.to_nav(db),
-            hir::ModuleDef::Function(it) => it.to_nav(db),
-            hir::ModuleDef::Adt(it) => it.to_nav(db),
-            hir::ModuleDef::Const(it) => it.to_nav(db),
-            hir::ModuleDef::Static(it) => it.to_nav(db),
-            hir::ModuleDef::EnumVariant(it) => it.to_nav(db),
-            hir::ModuleDef::Trait(it) => it.to_nav(db),
-            hir::ModuleDef::TypeAlias(it) => it.to_nav(db),
-            hir::ModuleDef::BuiltinType(..) => {
-                return None;
-            }
-        };
-        Some(nav)
-    }
-
     #[cfg(test)]
     pub(crate) fn assert_match(&self, expected: &str) {
         let actual = self.debug_render();
@@ -201,6 +185,36 @@ fn to_nav(&self, db: &RootDatabase) -> NavigationTarget {
     }
 }
 
+impl TryToNav for NameDefinition {
+    fn try_to_nav(&self, db: &RootDatabase) -> Option<NavigationTarget> {
+        match self {
+            NameDefinition::Macro(it) => Some(it.to_nav(db)),
+            NameDefinition::StructField(it) => Some(it.to_nav(db)),
+            NameDefinition::ModuleDef(it) => it.try_to_nav(db),
+            NameDefinition::SelfType(it) => Some(it.to_nav(db)),
+            NameDefinition::Local(it) => Some(it.to_nav(db)),
+            NameDefinition::TypeParam(it) => Some(it.to_nav(db)),
+        }
+    }
+}
+
+impl TryToNav for hir::ModuleDef {
+    fn try_to_nav(&self, db: &RootDatabase) -> Option<NavigationTarget> {
+        let res = match self {
+            hir::ModuleDef::Module(it) => it.to_nav(db),
+            hir::ModuleDef::Function(it) => it.to_nav(db),
+            hir::ModuleDef::Adt(it) => it.to_nav(db),
+            hir::ModuleDef::EnumVariant(it) => it.to_nav(db),
+            hir::ModuleDef::Const(it) => it.to_nav(db),
+            hir::ModuleDef::Static(it) => it.to_nav(db),
+            hir::ModuleDef::Trait(it) => it.to_nav(db),
+            hir::ModuleDef::TypeAlias(it) => it.to_nav(db),
+            hir::ModuleDef::BuiltinType(_) => return None,
+        };
+        Some(res)
+    }
+}
+
 pub(crate) trait ToNavFromAst {}
 impl ToNavFromAst for hir::Function {}
 impl ToNavFromAst for hir::Const {}
@@ -232,15 +246,17 @@ impl ToNav for hir::Module {
     fn to_nav(&self, db: &RootDatabase) -> NavigationTarget {
         let src = self.definition_source(db);
         let name = self.name(db).map(|it| it.to_string().into()).unwrap_or_default();
-        let syntax = match &src.value {
-            ModuleSource::SourceFile(node) => node.syntax(),
-            ModuleSource::Module(node) => node.syntax(),
+        let (syntax, focus) = match &src.value {
+            ModuleSource::SourceFile(node) => (node.syntax(), None),
+            ModuleSource::Module(node) => {
+                (node.syntax(), node.name().map(|it| it.syntax().text_range()))
+            }
         };
         let frange = original_range(db, src.with_value(syntax));
         NavigationTarget::from_syntax(
             frange.file_id,
             name,
-            None,
+            focus,
             frange.range,
             syntax.kind(),
             None,

crates/ra_ide/src/goto_definition.rs

@@ -1,18 +1,18 @@
 //! FIXME: write short doc here
 
 use hir::{db::AstDatabase, InFile, SourceBinder};
-use ra_ide_db::{defs::NameDefinition, symbol_index, RootDatabase};
+use ra_ide_db::{symbol_index, RootDatabase};
 use ra_syntax::{
-    ast::{self, DocCommentsOwner},
+    ast::{self},
     match_ast, AstNode,
     SyntaxKind::*,
-    SyntaxNode, SyntaxToken, TokenAtOffset,
+    SyntaxToken, TokenAtOffset,
 };
 
 use crate::{
-    display::{ShortLabel, ToNav},
+    display::{ToNav, TryToNav},
     expand::descend_into_macros,
-    references::classify_name_ref,
+    references::{classify_name, classify_name_ref},
     FilePosition, NavigationTarget, RangeInfo,
 };
 
@@ -74,23 +74,12 @@ pub(crate) fn reference_definition(
     use self::ReferenceResult::*;
 
     let name_kind = classify_name_ref(sb, name_ref);
-    match name_kind {
-        Some(NameDefinition::Macro(it)) => return Exact(it.to_nav(sb.db)),
-        Some(NameDefinition::StructField(it)) => return Exact(it.to_nav(sb.db)),
-        Some(NameDefinition::TypeParam(it)) => return Exact(it.to_nav(sb.db)),
-        Some(NameDefinition::Local(it)) => return Exact(it.to_nav(sb.db)),
-        Some(NameDefinition::ModuleDef(def)) => match NavigationTarget::from_def(sb.db, def) {
-            Some(nav) => return Exact(nav),
-            None => return Approximate(vec![]),
-        },
-        Some(NameDefinition::SelfType(imp)) => {
-            // FIXME: ideally, this should point to the type in the impl, and
-            // not at the whole impl. And goto **type** definition should bring
-            // us to the actual type
-            return Exact(imp.to_nav(sb.db));
-        }
-        None => {}
-    };
+    if let Some(def) = name_kind {
+        return match def.try_to_nav(sb.db) {
+            Some(nav) => ReferenceResult::Exact(nav),
+            None => ReferenceResult::Approximate(Vec::new()),
+        };
+    }
 
     // Fallback index based approach:
     let navs = symbol_index::index_resolve(sb.db, name_ref.value)
@@ -104,119 +93,9 @@ fn name_definition(
     sb: &mut SourceBinder<RootDatabase>,
     name: InFile<&ast::Name>,
 ) -> Option<Vec<NavigationTarget>> {
-    let parent = name.value.syntax().parent()?;
-
-    if let Some(module) = ast::Module::cast(parent.clone()) {
-        if module.has_semi() {
-            let src = name.with_value(module);
-            if let Some(child_module) = sb.to_def(src) {
-                let nav = child_module.to_nav(sb.db);
-                return Some(vec![nav]);
-            }
-        }
-    }
-
-    if let Some(nav) = named_target(sb.db, name.with_value(&parent)) {
-        return Some(vec![nav]);
-    }
-
-    None
-}
-
-fn named_target(db: &RootDatabase, node: InFile<&SyntaxNode>) -> Option<NavigationTarget> {
-    match_ast! {
-        match (node.value) {
-            ast::StructDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::EnumDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::EnumVariant(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::FnDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::TypeAliasDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::ConstDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::StaticDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::TraitDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::RecordFieldDef(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::Module(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    it.short_label(),
-                ))
-            },
-            ast::MacroCall(it) => {
-                Some(NavigationTarget::from_named(
-                    db,
-                    node.with_value(&it),
-                    it.doc_comment_text(),
-                    None,
-                ))
-            },
-            _ => None,
-        }
-    }
+    let def = classify_name(sb, name)?;
+    let nav = def.try_to_nav(sb.db)?;
+    Some(vec![nav])
 }
 
 #[cfg(test)]

crates/ra_ide/src/references.rs

@@ -25,7 +25,7 @@
     match_ast, AstNode, SourceFile, SyntaxKind, SyntaxNode, TextRange, TextUnit, TokenAtOffset,
 };
 
-use crate::{display::ToNav, FilePosition, FileRange, NavigationTarget, RangeInfo};
+use crate::{display::TryToNav, FilePosition, FileRange, NavigationTarget, RangeInfo};
 
 pub(crate) use self::{
     classify::{classify_name, classify_name_ref},
@@ -125,15 +125,7 @@ pub(crate) fn find_all_refs(
         };
 
     let RangeInfo { range, info: (name, def) } = find_name(db, &syntax, position, opt_name)?;
-
-    let declaration = match def {
-        NameDefinition::Macro(mac) => mac.to_nav(db),
-        NameDefinition::StructField(field) => field.to_nav(db),
-        NameDefinition::ModuleDef(def) => NavigationTarget::from_def(db, def)?,
-        NameDefinition::SelfType(imp) => imp.to_nav(db),
-        NameDefinition::Local(local) => local.to_nav(db),
-        NameDefinition::TypeParam(_) => return None,
-    };
+    let declaration = def.try_to_nav(db)?;
 
     let search_scope = {
         let base = SearchScope::for_def(&def, db);