Merge #1360

1360: Improve goto definition for MBE r=matklad a=edwin0cheng

This PR improve the macro resolution for goto definition and expression macro invocation by using proper path resolution for external macros. 



Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>

crates/ra_hir/src/expr.rs

@@ -828,7 +828,7 @@ where
                     .ast_id(e)
                     .with_file_id(self.current_file_id);
 
-                if let Some(def) = self.resolver.resolve_macro_call(path) {
+                if let Some(def) = self.resolver.resolve_macro_call(self.db, path) {
                     let call_id = MacroCallLoc { def, ast_id }.id(self.db);
                     let file_id = call_id.as_file(MacroFileKind::Expr);
                     if let Some(node) = self.db.parse_or_expand(file_id) {

crates/ra_hir/src/nameres.rs

@@ -320,8 +320,22 @@ impl CrateDefMap {
         (res.resolved_def, res.segment_index)
     }
 
-    pub(crate) fn find_macro(&self, name: &Name) -> Option<MacroDefId> {
+    pub(crate) fn find_macro(
-        self.public_macros.get(name).or(self.local_macros.get(name)).map(|it| *it)
+        &self,
+        db: &impl DefDatabase,
+        original_module: CrateModuleId,
+        path: &Path,
+    ) -> Option<MacroDefId> {
+        let name = path.expand_macro_expr()?;
+        // search local first
+        // FIXME: Remove public_macros check when we have a correct local_macors implementation
+        let local = self.public_macros.get(&name).or(self.local_macros.get(&name)).map(|it| *it);
+        if local.is_some() {
+            return local;
+        }
+
+        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
+        res.resolved_def.right().map(|m| m.id)
     }
 
     // Returns Yes if we are sure that additions to `ItemMap` wouldn't change

crates/ra_hir/src/resolve.rs

@@ -130,9 +130,13 @@ impl Resolver {
         resolution
     }
 
-    pub(crate) fn resolve_macro_call(&self, path: Option<Path>) -> Option<MacroDefId> {
+    pub(crate) fn resolve_macro_call(
-        let name = path.and_then(|path| path.expand_macro_expr()).unwrap_or_else(Name::missing);
+        &self,
-        self.module()?.0.find_macro(&name)
+        db: &impl HirDatabase,
+        path: Option<Path>,
+    ) -> Option<MacroDefId> {
+        let m = self.module()?;
+        m.0.find_macro(db, m.1, &path?)
     }
 
     /// Returns the resolved path segments

crates/ra_hir/src/source_binder.rs

@@ -283,8 +283,13 @@ impl SourceAnalyzer {
         self.infer.as_ref()?.field_resolution(expr_id)
     }
 
-    pub fn resolve_macro_call(&self, macro_call: &ast::MacroCall) -> Option<MacroByExampleDef> {
+    pub fn resolve_macro_call(
-        let id = self.resolver.resolve_macro_call(macro_call.path().and_then(Path::from_ast))?;
+        &self,
+        db: &impl HirDatabase,
+        macro_call: &ast::MacroCall,
+    ) -> Option<MacroByExampleDef> {
+        let id =
+            self.resolver.resolve_macro_call(db, macro_call.path().and_then(Path::from_ast))?;
         Some(MacroByExampleDef { id })
     }
 

crates/ra_ide_api/src/goto_definition.rs

@@ -216,6 +216,29 @@ mod tests {
         );
     }
 
+    #[test]
+    fn goto_definition_works_for_macros_from_other_crates() {
+        covers!(goto_definition_works_for_macros);
+        check_goto(
+            "
+            //- /lib.rs
+            use foo::foo;
+            fn bar() {
+                <|>foo!();
+            }
+
+            //- /foo/lib.rs
+            #[macro_export]
+            macro_rules! foo {
+                () => {             
+                    {}
+                };
+            }            
+            ",
+            "foo MACRO_CALL FileId(2) [0; 79) [29; 32)",
+        );
+    }
+
     #[test]
     fn goto_definition_works_for_methods() {
         covers!(goto_definition_works_for_methods);

crates/ra_ide_api/src/name_ref_kind.rs

@@ -39,7 +39,7 @@ pub(crate) fn classify_name_ref(
         .and_then(ast::MacroCall::cast)
     {
         tested_by!(goto_definition_works_for_macros);
-        if let Some(mac) = analyzer.resolve_macro_call(macro_call) {
+        if let Some(mac) = analyzer.resolve_macro_call(db, macro_call) {
             return Some(Macro(mac));
         }
     }