fix: Improve diagnostic ranges for macro_calls!

We used to point to the entire macro call including its token tree if we couldn't upmap the diagnostic to the input
This generally makes things very noisy as the entire macro call will turn red on errors.
Instead, we now macro the path and `!` (bang) token as the error source range which is a lot nicer on the eyes.

Author: Veykril

crates/hir-expand/src/files.rs

@@ -315,11 +315,11 @@ impl<SN: Borrow<SyntaxNode>> InFile<SN> {
     }
 
     /// Falls back to the macro call range if the node cannot be mapped up fully.
-    pub fn original_file_range_with_macro_call_body(
+    pub fn original_file_range_with_macro_call_input(
         self,
         db: &dyn db::ExpandDatabase,
     ) -> FileRange {
-        self.borrow().map(SyntaxNode::text_range).original_node_file_range_with_macro_call_body(db)
+        self.borrow().map(SyntaxNode::text_range).original_node_file_range_with_macro_call_input(db)
     }
 
     pub fn original_syntax_node_rooted(
@@ -465,7 +465,7 @@ impl InFile<TextRange> {
         }
     }
 
-    pub fn original_node_file_range_with_macro_call_body(
+    pub fn original_node_file_range_with_macro_call_input(
         self,
         db: &dyn db::ExpandDatabase,
     ) -> FileRange {
@@ -476,7 +476,7 @@ impl InFile<TextRange> {
                     Some(it) => it,
                     _ => {
                         let loc = db.lookup_intern_macro_call(mac_file);
-                        loc.kind.original_call_range_with_body(db)
+                        loc.kind.original_call_range_with_input(db)
                     }
                 }
             }
@@ -497,6 +497,18 @@ impl InFile<TextRange> {
             }
         }
     }
+
+    pub fn original_node_file_range_rooted_opt(
+        self,
+        db: &dyn db::ExpandDatabase,
+    ) -> Option<FileRange> {
+        match self.file_id {
+            HirFileId::FileId(file_id) => Some(FileRange { file_id, range: self.value }),
+            HirFileId::MacroFile(mac_file) => {
+                map_node_range_up_rooted(db, &db.expansion_span_map(mac_file), self.value)
+            }
+        }
+    }
 }
 
 impl<N: AstNode> InFile<N> {

crates/hir-expand/src/lib.rs

@@ -688,8 +688,11 @@ impl MacroCallKind {
 
     /// Returns the original file range that best describes the location of this macro call.
     ///
-    /// Unlike `MacroCallKind::original_call_range`, this also spans the item of attributes and derives.
-    pub fn original_call_range_with_body(self, db: &dyn ExpandDatabase) -> FileRange {
+    /// This spans the entire macro call, including its input. That is for
+    /// - fn_like! {}, it spans the path and token tree
+    /// - #\[derive], it spans the `#[derive(...)]` attribute and the annotated item
+    /// - #\[attr], it spans the `#[attr(...)]` attribute and the annotated item
+    pub fn original_call_range_with_input(self, db: &dyn ExpandDatabase) -> FileRange {
         let mut kind = self;
         let file_id = loop {
             match kind.file_id() {
@@ -712,8 +715,8 @@ impl MacroCallKind {
     /// Returns the original file range that best describes the location of this macro call.
     ///
     /// Here we try to roughly match what rustc does to improve diagnostics: fn-like macros
-    /// get the whole `ast::MacroCall`, attribute macros get the attribute's range, and derives
-    /// get only the specific derive that is being referred to.
+    /// get the macro path (rustc shows the whole `ast::MacroCall`), attribute macros get the
+    /// attribute's range, and derives get only the specific derive that is being referred to.
     pub fn original_call_range(self, db: &dyn ExpandDatabase) -> FileRange {
         let mut kind = self;
         let file_id = loop {
@@ -726,7 +729,14 @@ impl MacroCallKind {
         };
 
         let range = match kind {
-            MacroCallKind::FnLike { ast_id, .. } => ast_id.to_ptr(db).text_range(),
+            MacroCallKind::FnLike { ast_id, .. } => {
+                let node = ast_id.to_node(db);
+                node.path()
+                    .unwrap()
+                    .syntax()
+                    .text_range()
+                    .cover(node.excl_token().unwrap().text_range())
+            }
             MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
                 // FIXME: should be the range of the macro name, not the whole derive
                 // FIXME: handle `cfg_attr`

crates/hir-ty/src/tests/simple.rs

@@ -3751,7 +3751,7 @@ fn foo() {
     }
     let v: bool = true;
     m!();
- // ^^^^ i32
+ // ^^ i32
 }
         "#,
     );
@@ -3765,39 +3765,39 @@ fn foo() {
     let v: bool;
     macro_rules! m { () => { v } }
     m!();
- // ^^^^ bool
+ // ^^ bool
 
     let v: char;
     macro_rules! m { () => { v } }
     m!();
- // ^^^^ char
+ // ^^ char
 
     {
         let v: u8;
         macro_rules! m { () => { v } }
         m!();
-     // ^^^^ u8
+     // ^^ u8
 
         let v: i8;
         macro_rules! m { () => { v } }
         m!();
-     // ^^^^ i8
+     // ^^ i8
 
         let v: i16;
         macro_rules! m { () => { v } }
         m!();
-     // ^^^^ i16
+     // ^^ i16
 
         {
             let v: u32;
             macro_rules! m { () => { v } }
             m!();
-         // ^^^^ u32
+         // ^^ u32
 
             let v: u64;
             macro_rules! m { () => { v } }
             m!();
-         // ^^^^ u64
+         // ^^ u64
         }
     }
 }

crates/ide-db/src/search.rs

@@ -317,7 +317,7 @@ impl Definition {
             };
             return match def {
                 Some(def) => SearchScope::file_range(
-                    def.as_ref().original_file_range_with_macro_call_body(db),
+                    def.as_ref().original_file_range_with_macro_call_input(db),
                 ),
                 None => SearchScope::single_file(file_id),
             };
@@ -332,7 +332,7 @@ impl Definition {
             };
             return match def {
                 Some(def) => SearchScope::file_range(
-                    def.as_ref().original_file_range_with_macro_call_body(db),
+                    def.as_ref().original_file_range_with_macro_call_input(db),
                 ),
                 None => SearchScope::single_file(file_id),
             };
@@ -341,7 +341,7 @@ impl Definition {
         if let Definition::SelfType(impl_) = self {
             return match impl_.source(db).map(|src| src.syntax().cloned()) {
                 Some(def) => SearchScope::file_range(
-                    def.as_ref().original_file_range_with_macro_call_body(db),
+                    def.as_ref().original_file_range_with_macro_call_input(db),
                 ),
                 None => SearchScope::single_file(file_id),
             };
@@ -360,7 +360,7 @@ impl Definition {
             };
             return match def {
                 Some(def) => SearchScope::file_range(
-                    def.as_ref().original_file_range_with_macro_call_body(db),
+                    def.as_ref().original_file_range_with_macro_call_input(db),
                 ),
                 None => SearchScope::single_file(file_id),
             };

crates/ide-diagnostics/src/handlers/macro_error.rs

@@ -242,8 +242,8 @@ macro_rules! outer {
 
 fn f() {
     outer!();
-} //^^^^^^^^ error: leftover tokens
-  //^^^^^^^^ error: Syntax Error in Expansion: expected expression
+} //^^^^^^ error: leftover tokens
+  //^^^^^^ error: Syntax Error in Expansion: expected expression
 "#,
         )
     }

crates/ide-diagnostics/src/handlers/missing_fields.rs

@@ -66,7 +66,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
     let current_module =
         ctx.sema.scope(d.field_list_parent.to_node(&root).syntax()).map(|it| it.module());
     let range = InFile::new(d.file, d.field_list_parent.text_range())
-        .original_node_file_range_rooted(ctx.sema.db);
+        .original_node_file_range_rooted_opt(ctx.sema.db)?;
 
     let build_text_edit = |new_syntax: &SyntaxNode, old_syntax| {
         let edit = {

crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs

@@ -77,6 +77,7 @@ fn quickfix_for_redundant_assoc_item(
     redundant_item_def: String,
     range: TextRange,
 ) -> Option<Vec<Assist>> {
+    let file_id = d.file_id.file_id()?;
     let add_assoc_item_def = |builder: &mut SourceChangeBuilder| -> Option<()> {
         let db = ctx.sema.db;
         let root = db.parse_or_expand(d.file_id);
@@ -90,12 +91,14 @@ fn quickfix_for_redundant_assoc_item(
         let trait_def = d.trait_.source(db)?.value;
         let l_curly = trait_def.assoc_item_list()?.l_curly_token()?.text_range();
         let where_to_insert =
-            hir::InFile::new(d.file_id, l_curly).original_node_file_range_rooted(db).range;
+            hir::InFile::new(d.file_id, l_curly).original_node_file_range_rooted_opt(db)?;
+        if where_to_insert.file_id != file_id {
+            return None;
+        }
 
-        builder.insert(where_to_insert.end(), redundant_item_def);
+        builder.insert(where_to_insert.range.end(), redundant_item_def);
         Some(())
     };
-    let file_id = d.file_id.file_id()?;
     let mut source_change_builder = SourceChangeBuilder::new(file_id.file_id(ctx.sema.db));
     add_assoc_item_def(&mut source_change_builder)?;
 

crates/ide-diagnostics/src/handlers/unresolved_method.rs

@@ -120,8 +120,7 @@ fn assoc_func_fix(
 
         let call = ast::MethodCallExpr::cast(expr.syntax().clone())?;
         let range = InFile::new(expr_ptr.file_id, call.syntax().text_range())
-            .original_node_file_range_rooted(db)
-            .range;
+            .original_node_file_range_rooted_opt(db)?;
 
         let receiver = call.receiver()?;
         let receiver_type = &ctx.sema.type_of_expr(&receiver)?.original;
@@ -174,18 +173,16 @@ fn assoc_func_fix(
 
         let assoc_func_call_expr_string = make::expr_call(assoc_func_path, args).to_string();
 
-        let file_id = ctx.sema.original_range_opt(call.receiver()?.syntax())?.file_id;
-
         Some(Assist {
             id: AssistId::quick_fix("method_call_to_assoc_func_call_fix"),
             label: Label::new(format!(
                 "Use associated func call instead: `{assoc_func_call_expr_string}`"
             )),
             group: None,
-            target: range,
+            target: range.range,
             source_change: Some(SourceChange::from_text_edit(
-                file_id.file_id(ctx.sema.db),
-                TextEdit::replace(range, assoc_func_call_expr_string),
+                range.file_id.file_id(ctx.sema.db),
+                TextEdit::replace(range.range, assoc_func_call_expr_string),
             )),
             command: None,
         })
@@ -300,7 +297,7 @@ macro_rules! m {
 }
 fn main() {
     m!(());
- // ^^^^^^ error: no method `foo` on type `()`
+ // ^^ error: no method `foo` on type `()`
 }
 "#,
         );

crates/ide/src/call_hierarchy.rs

@@ -592,7 +592,7 @@ macro_rules! call {
 "#,
             expect!["callee Function FileId(0) 22..37 30..36"],
             expect![[r#"
-                caller Function FileId(0) 38..52 : FileId(0):44..50
+                caller Function FileId(0) 38..43 : FileId(0):44..50
                 caller Function FileId(1) 130..136 130..136 : FileId(0):44..50
                 callee Function FileId(0) 38..52 44..50 : FileId(0):44..50"#]],
             expect![[]],

crates/ide/src/goto_definition.rs

@@ -1082,7 +1082,7 @@ macro_rules! define_fn {
 }
 
   define_fn!();
-//^^^^^^^^^^^^^
+//^^^^^^^^^^
 fn bar() {
    $0foo();
 }
@@ -3228,7 +3228,7 @@ mod bar {
     use crate::m;
 
     m!();
- // ^^^^^
+ // ^^
 
     fn qux() {
         Foo$0;