Skip to content

fix: Improve diagnostic ranges for macro_calls! #20160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 3, 2025
Merged

fix: Improve diagnostic ranges for macro_calls! #20160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions crates/hir-expand/src/files.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
}
}
Expand All @@ -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> {
Expand Down
20 changes: 15 additions & 5 deletions crates/hir-expand/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand All @@ -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`
Expand Down
16 changes: 8 additions & 8 deletions crates/hir-ty/src/tests/simple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3751,7 +3751,7 @@ fn foo() {
}
let v: bool = true;
m!();
// ^^^^ i32
// ^^ i32
}
"#,
);
Expand All @@ -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
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ide-db/src/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
Expand All @@ -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),
};
Expand All @@ -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),
};
Expand All @@ -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),
};
Expand Down
4 changes: 2 additions & 2 deletions crates/ide-diagnostics/src/handlers/macro_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
"#,
)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/missing_fields.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
9 changes: 6 additions & 3 deletions crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)?;

Expand Down
13 changes: 5 additions & 8 deletions crates/ide-diagnostics/src/handlers/unresolved_method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -300,7 +297,7 @@ macro_rules! m {
}
fn main() {
m!(());
// ^^^^^^ error: no method `foo` on type `()`
// ^^ error: no method `foo` on type `()`
}
"#,
);
Expand Down
2 changes: 1 addition & 1 deletion crates/ide/src/call_hierarchy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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![[]],
Expand Down
4 changes: 2 additions & 2 deletions crates/ide/src/goto_definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ macro_rules! define_fn {
}

define_fn!();
//^^^^^^^^^^^^^
//^^^^^^^^^^
fn bar() {
$0foo();
}
Expand Down Expand Up @@ -3228,7 +3228,7 @@ mod bar {
use crate::m;

m!();
// ^^^^^
// ^^

fn qux() {
Foo$0;
Expand Down
2 changes: 1 addition & 1 deletion crates/ide/src/navigation_target.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ pub(crate) fn orig_range_with_focus_r(
// *should* contain the name
_ => {
let kind = call_kind();
let range = kind.clone().original_call_range_with_body(db);
let range = kind.clone().original_call_range_with_input(db);
//If the focus range is in the attribute/derive body, we
// need to point the call site to the entire body, if not, fall back
// to the name range of the attribute/derive call
Expand Down
20 changes: 10 additions & 10 deletions crates/ide/src/runnables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pub(crate) fn runnable_fn(
)
.call_site();

let file_range = fn_source.syntax().original_file_range_with_macro_call_body(sema.db);
let file_range = fn_source.syntax().original_file_range_with_macro_call_input(sema.db);
let update_test =
UpdateTest::find_snapshot_macro(sema, &fn_source.file_syntax(sema.db), file_range);

Expand Down Expand Up @@ -425,7 +425,7 @@ pub(crate) fn runnable_impl(

let impl_source = sema.source(*def)?;
let impl_syntax = impl_source.syntax();
let file_range = impl_syntax.original_file_range_with_macro_call_body(sema.db);
let file_range = impl_syntax.original_file_range_with_macro_call_input(sema.db);
let update_test =
UpdateTest::find_snapshot_macro(sema, &impl_syntax.file_syntax(sema.db), file_range);

Expand Down Expand Up @@ -1241,10 +1241,10 @@ generate_main!();
[
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 0..345, name: \"\", kind: Module })",
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 282..312, focus_range: 286..291, name: \"tests\", kind: Module, description: \"mod tests\" })",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 298..310, name: \"foo_test\", kind: Function })",
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 313..326, name: \"tests2\", kind: Module, description: \"mod tests2\" }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 313..326, name: \"foo_test2\", kind: Function }, true)",
"(Bin, NavigationTarget { file_id: FileId(0), full_range: 327..344, name: \"main\", kind: Function })",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 298..307, name: \"foo_test\", kind: Function })",
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 313..323, name: \"tests2\", kind: Module, description: \"mod tests2\" }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 313..323, name: \"foo_test2\", kind: Function }, true)",
"(Bin, NavigationTarget { file_id: FileId(0), full_range: 327..341, name: \"main\", kind: Function })",
]
"#]],
);
Expand Down Expand Up @@ -1272,10 +1272,10 @@ foo!();
"#,
expect![[r#"
[
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 210..217, name: \"foo_tests\", kind: Module, description: \"mod foo_tests\" }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..217, name: \"foo0\", kind: Function }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..217, name: \"foo1\", kind: Function }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..217, name: \"foo2\", kind: Function }, true)",
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 210..214, name: \"foo_tests\", kind: Module, description: \"mod foo_tests\" }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..214, name: \"foo0\", kind: Function }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..214, name: \"foo1\", kind: Function }, true)",
"(Test, NavigationTarget { file_id: FileId(0), full_range: 210..214, name: \"foo2\", kind: Function }, true)",
]
"#]],
);
Expand Down