Merge pull request #20160 from Veykril/push-pqvskktpnylu

fix: Improve diagnostic ranges for `macro_calls!`

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;