Merge pull request #20160 from Veykril/push-pqvskktpnylu

fix: Improve diagnostic ranges for `macro_calls!`
This commit is contained in:
Lukas Wirth 2025-07-03 12:23:23 +00:00 committed by GitHub
commit f14bf95931
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 71 additions and 49 deletions

View file

@ -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. /// 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, self,
db: &dyn db::ExpandDatabase, db: &dyn db::ExpandDatabase,
) -> FileRange { ) -> 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( 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, self,
db: &dyn db::ExpandDatabase, db: &dyn db::ExpandDatabase,
) -> FileRange { ) -> FileRange {
@ -476,7 +476,7 @@ impl InFile<TextRange> {
Some(it) => it, Some(it) => it,
_ => { _ => {
let loc = db.lookup_intern_macro_call(mac_file); 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> { impl<N: AstNode> InFile<N> {

View file

@ -688,8 +688,11 @@ impl MacroCallKind {
/// Returns the original file range that best describes the location of this macro call. /// 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. /// This spans the entire macro call, including its input. That is for
pub fn original_call_range_with_body(self, db: &dyn ExpandDatabase) -> FileRange { /// - 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 mut kind = self;
let file_id = loop { let file_id = loop {
match kind.file_id() { match kind.file_id() {
@ -712,8 +715,8 @@ impl MacroCallKind {
/// Returns the original file range that best describes the location of this macro call. /// 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 /// 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 the macro path (rustc shows the whole `ast::MacroCall`), attribute macros get the
/// get only the specific derive that is being referred to. /// 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 { pub fn original_call_range(self, db: &dyn ExpandDatabase) -> FileRange {
let mut kind = self; let mut kind = self;
let file_id = loop { let file_id = loop {
@ -726,7 +729,14 @@ impl MacroCallKind {
}; };
let range = match kind { 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, .. } => { MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
// FIXME: should be the range of the macro name, not the whole derive // FIXME: should be the range of the macro name, not the whole derive
// FIXME: handle `cfg_attr` // FIXME: handle `cfg_attr`

View file

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

View file

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

View file

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

View file

@ -66,7 +66,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
let current_module = let current_module =
ctx.sema.scope(d.field_list_parent.to_node(&root).syntax()).map(|it| it.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()) 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 build_text_edit = |new_syntax: &SyntaxNode, old_syntax| {
let edit = { let edit = {

View file

@ -77,6 +77,7 @@ fn quickfix_for_redundant_assoc_item(
redundant_item_def: String, redundant_item_def: String,
range: TextRange, range: TextRange,
) -> Option<Vec<Assist>> { ) -> Option<Vec<Assist>> {
let file_id = d.file_id.file_id()?;
let add_assoc_item_def = |builder: &mut SourceChangeBuilder| -> Option<()> { let add_assoc_item_def = |builder: &mut SourceChangeBuilder| -> Option<()> {
let db = ctx.sema.db; let db = ctx.sema.db;
let root = db.parse_or_expand(d.file_id); 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 trait_def = d.trait_.source(db)?.value;
let l_curly = trait_def.assoc_item_list()?.l_curly_token()?.text_range(); let l_curly = trait_def.assoc_item_list()?.l_curly_token()?.text_range();
let where_to_insert = 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(()) Some(())
}; };
let file_id = d.file_id.file_id()?;
let mut source_change_builder = SourceChangeBuilder::new(file_id.file_id(ctx.sema.db)); let mut source_change_builder = SourceChangeBuilder::new(file_id.file_id(ctx.sema.db));
add_assoc_item_def(&mut source_change_builder)?; add_assoc_item_def(&mut source_change_builder)?;

View file

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

View file

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

View file

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

View file

@ -844,7 +844,7 @@ pub(crate) fn orig_range_with_focus_r(
// *should* contain the name // *should* contain the name
_ => { _ => {
let kind = call_kind(); 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 //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 // need to point the call site to the entire body, if not, fall back
// to the name range of the attribute/derive call // to the name range of the attribute/derive call

View file

@ -351,7 +351,7 @@ pub(crate) fn runnable_fn(
) )
.call_site(); .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 = let update_test =
UpdateTest::find_snapshot_macro(sema, &fn_source.file_syntax(sema.db), file_range); UpdateTest::find_snapshot_macro(sema, &fn_source.file_syntax(sema.db), file_range);
@ -425,7 +425,7 @@ pub(crate) fn runnable_impl(
let impl_source = sema.source(*def)?; let impl_source = sema.source(*def)?;
let impl_syntax = impl_source.syntax(); 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 = let update_test =
UpdateTest::find_snapshot_macro(sema, &impl_syntax.file_syntax(sema.db), file_range); UpdateTest::find_snapshot_macro(sema, &impl_syntax.file_syntax(sema.db), file_range);
@ -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: 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\" })", "(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 })", "(Test, NavigationTarget { file_id: FileId(0), full_range: 298..307, name: \"foo_test\", kind: Function })",
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 313..326, name: \"tests2\", kind: Module, description: \"mod tests2\" }, true)", "(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..326, name: \"foo_test2\", kind: Function }, 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..344, name: \"main\", kind: Function })", "(Bin, NavigationTarget { file_id: FileId(0), full_range: 327..341, name: \"main\", kind: Function })",
] ]
"#]], "#]],
); );
@ -1272,10 +1272,10 @@ foo!();
"#, "#,
expect![[r#" expect![[r#"
[ [
"(TestMod, NavigationTarget { file_id: FileId(0), full_range: 210..217, name: \"foo_tests\", kind: Module, description: \"mod foo_tests\" }, 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..217, name: \"foo0\", kind: Function }, 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..217, name: \"foo1\", 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..217, name: \"foo2\", kind: Function }, true)", "(Test, NavigationTarget { file_id: FileId(0), full_range: 210..214, name: \"foo2\", kind: Function }, true)",
] ]
"#]], "#]],
); );