Merge #9420

9420: fix: Make hover on attribute paths work again r=Veykril a=Veykril

Unfortunately not (yet) unit-testable I believe

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/ide/src/hover.rs

@@ -11,7 +11,10 @@ use ide_db::{
 };
 use itertools::Itertools;
 use stdx::format_to;
-use syntax::{algo, ast, match_ast, AstNode, AstToken, Direction, SyntaxKind::*, SyntaxToken, T};
+use syntax::{
+    algo, ast, display::fn_as_proc_macro_label, match_ast, AstNode, AstToken, Direction,
+    SyntaxKind::*, SyntaxToken, T,
+};
 
 use crate::{
     display::{macro_label, TryToNav},
@@ -166,6 +169,7 @@ pub(crate) fn hover(
 
     let node = token
         .ancestors()
+        .take_while(|it| !ast::Item::can_cast(it.kind()))
         .find(|n| ast::Expr::can_cast(n.kind()) || ast::Pat::can_cast(n.kind()))?;
 
     let ty = match_ast! {
@@ -409,16 +413,13 @@ fn hover_for_definition(
 ) -> Option<Markup> {
     let mod_path = definition_mod_path(db, &def);
     let (label, docs) = match def {
-        Definition::Macro(it) => match &it.source(db)?.value {
-            Either::Left(mac) => {
-                let label = macro_label(mac);
-                (label, it.attrs(db).docs())
-            }
-            Either::Right(_) => {
-                // FIXME
-                return None;
-            }
+        Definition::Macro(it) => (
+            match &it.source(db)?.value {
+                Either::Left(mac) => macro_label(mac),
+                Either::Right(mac_fn) => fn_as_proc_macro_label(mac_fn),
             },
+            it.attrs(db).docs(),
+        ),
         Definition::Field(def) => label_and_docs(db, def),
         Definition::ModuleDef(it) => match it {
             hir::ModuleDef::Module(it) => label_and_docs(db, it),

crates/ide/src/references.rs

@@ -1308,24 +1308,6 @@ fn test$0() {
         );
     }
 
-    #[test]
-    fn test_attr_matches_proc_macro_fn() {
-        check(
-            r#"
-#[proc_macro_attribute]
-fn my_proc_macro() {}
-
-#[my_proc_macro$0]
-fn test() {}
-"#,
-            expect![[r#"
-                my_proc_macro Function FileId(0) 0..45 27..40
-
-                FileId(0) 49..62
-            "#]],
-        );
-    }
-
     #[test]
     fn test_const_in_pattern() {
         check(

crates/ide_db/src/defs.rs

@@ -6,8 +6,8 @@
 // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06).
 
 use hir::{
-    db::HirDatabase, Crate, Field, GenericParam, HasAttrs, HasVisibility, Impl, Label, Local,
-    MacroDef, Module, ModuleDef, Name, PathResolution, Semantics, Visibility,
+    db::HirDatabase, Crate, Field, GenericParam, HasVisibility, Impl, Label, Local, MacroDef,
+    Module, ModuleDef, Name, PathResolution, Semantics, Visibility,
 };
 use syntax::{
     ast::{self, AstNode, PathSegmentKind},
@@ -385,15 +385,22 @@ impl NameRefClass {
             }
 
             if let Some(resolved) = sema.resolve_path(&path) {
-                if path.syntax().ancestors().find_map(ast::Attr::cast).is_some() {
-                    if let PathResolution::Def(ModuleDef::Function(func)) = resolved {
-                        if func.attrs(sema.db).by_key("proc_macro_attribute").exists() {
-                            return Some(NameRefClass::Definition(resolved.into()));
+                return if path.syntax().ancestors().find_map(ast::Attr::cast).is_some() {
+                    match resolved {
+                        // Don't wanna collide with builtin attributes here like `test` hence guard
+                        PathResolution::Def(module @ ModuleDef::Module(_))
+                            if path.parent_path().is_some() =>
+                        {
+                            Some(NameRefClass::Definition(Definition::ModuleDef(module)))
                         }
+                        PathResolution::Macro(mac) if mac.kind() == hir::MacroKind::Attr => {
+                            Some(NameRefClass::Definition(Definition::Macro(mac)))
+                        }
+                        _ => None,
                     }
                 } else {
-                    return Some(NameRefClass::Definition(resolved.into()));
-                }
+                    Some(NameRefClass::Definition(resolved.into()))
+                };
             }
         }
 

crates/syntax/src/display.rs

@@ -77,19 +77,35 @@ pub fn type_label(node: &ast::TypeAlias) -> String {
 }
 
 pub fn macro_label(node: &ast::Macro) -> String {
-    let name = node.name().map(|name| name.syntax().text().to_string()).unwrap_or_default();
+    let name = node.name();
+    let mut s = String::new();
     match node {
         ast::Macro::MacroRules(node) => {
             let vis = if node.has_atom_attr("macro_export") { "#[macro_export] " } else { "" };
-            format!("{}macro_rules! {}", vis, name)
+            format_to!(s, "{}macro_rules!", vis);
         }
         ast::Macro::MacroDef(node) => {
+            if let Some(vis) = node.visibility() {
+                format_to!(s, "{} ", vis);
+            }
+            format_to!(s, "macro");
+        }
+    }
+    if let Some(name) = name {
+        format_to!(s, " {}", name);
+    }
+    s
+}
+
+pub fn fn_as_proc_macro_label(node: &ast::Fn) -> String {
+    let name = node.name();
     let mut s = String::new();
     if let Some(vis) = node.visibility() {
         format_to!(s, "{} ", vis);
     }
-            format_to!(s, "macro {}", name);
+    format_to!(s, "macro");
+    if let Some(name) = name {
+        format_to!(s, " {}", name);
+    }
     s
 }
-    }
-}