Skip to content

Commit 2899630

Browse files
committed
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.
1 parent f98e4d0 commit 2899630

File tree

12 files changed

+71
-49
lines changed

12 files changed

+71
-49
lines changed

src/tools/rust-analyzer/crates/hir-expand/src/files.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,11 +315,11 @@ impl<SN: Borrow<SyntaxNode>> InFile<SN> {
315315
}
316316

317317
/// Falls back to the macro call range if the node cannot be mapped up fully.
318-
pub fn original_file_range_with_macro_call_body(
318+
pub fn original_file_range_with_macro_call_input(
319319
self,
320320
db: &dyn db::ExpandDatabase,
321321
) -> FileRange {
322-
self.borrow().map(SyntaxNode::text_range).original_node_file_range_with_macro_call_body(db)
322+
self.borrow().map(SyntaxNode::text_range).original_node_file_range_with_macro_call_input(db)
323323
}
324324

325325
pub fn original_syntax_node_rooted(
@@ -465,7 +465,7 @@ impl InFile<TextRange> {
465465
}
466466
}
467467

468-
pub fn original_node_file_range_with_macro_call_body(
468+
pub fn original_node_file_range_with_macro_call_input(
469469
self,
470470
db: &dyn db::ExpandDatabase,
471471
) -> FileRange {
@@ -476,7 +476,7 @@ impl InFile<TextRange> {
476476
Some(it) => it,
477477
_ => {
478478
let loc = db.lookup_intern_macro_call(mac_file);
479-
loc.kind.original_call_range_with_body(db)
479+
loc.kind.original_call_range_with_input(db)
480480
}
481481
}
482482
}
@@ -497,6 +497,18 @@ impl InFile<TextRange> {
497497
}
498498
}
499499
}
500+
501+
pub fn original_node_file_range_rooted_opt(
502+
self,
503+
db: &dyn db::ExpandDatabase,
504+
) -> Option<FileRange> {
505+
match self.file_id {
506+
HirFileId::FileId(file_id) => Some(FileRange { file_id, range: self.value }),
507+
HirFileId::MacroFile(mac_file) => {
508+
map_node_range_up_rooted(db, &db.expansion_span_map(mac_file), self.value)
509+
}
510+
}
511+
}
500512
}
501513

502514
impl<N: AstNode> InFile<N> {

src/tools/rust-analyzer/crates/hir-expand/src/lib.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -688,8 +688,11 @@ impl MacroCallKind {
688688

689689
/// Returns the original file range that best describes the location of this macro call.
690690
///
691-
/// Unlike `MacroCallKind::original_call_range`, this also spans the item of attributes and derives.
692-
pub fn original_call_range_with_body(self, db: &dyn ExpandDatabase) -> FileRange {
691+
/// This spans the entire macro call, including its input. That is for
692+
/// - fn_like! {}, it spans the path and token tree
693+
/// - #\[derive], it spans the `#[derive(...)]` attribute and the annotated item
694+
/// - #\[attr], it spans the `#[attr(...)]` attribute and the annotated item
695+
pub fn original_call_range_with_input(self, db: &dyn ExpandDatabase) -> FileRange {
693696
let mut kind = self;
694697
let file_id = loop {
695698
match kind.file_id() {
@@ -712,8 +715,8 @@ impl MacroCallKind {
712715
/// Returns the original file range that best describes the location of this macro call.
713716
///
714717
/// Here we try to roughly match what rustc does to improve diagnostics: fn-like macros
715-
/// get the whole `ast::MacroCall`, attribute macros get the attribute's range, and derives
716-
/// get only the specific derive that is being referred to.
718+
/// get the macro path (rustc shows the whole `ast::MacroCall`), attribute macros get the
719+
/// attribute's range, and derives get only the specific derive that is being referred to.
717720
pub fn original_call_range(self, db: &dyn ExpandDatabase) -> FileRange {
718721
let mut kind = self;
719722
let file_id = loop {
@@ -726,7 +729,14 @@ impl MacroCallKind {
726729
};
727730

728731
let range = match kind {
729-
MacroCallKind::FnLike { ast_id, .. } => ast_id.to_ptr(db).text_range(),
732+
MacroCallKind::FnLike { ast_id, .. } => {
733+
let node = ast_id.to_node(db);
734+
node.path()
735+
.unwrap()
736+
.syntax()
737+
.text_range()
738+
.cover(node.excl_token().unwrap().text_range())
739+
}
730740
MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
731741
// FIXME: should be the range of the macro name, not the whole derive
732742
// FIXME: handle `cfg_attr`

src/tools/rust-analyzer/crates/hir-ty/src/tests/simple.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3751,7 +3751,7 @@ fn foo() {
37513751
}
37523752
let v: bool = true;
37533753
m!();
3754-
// ^^^^ i32
3754+
// ^^ i32
37553755
}
37563756
"#,
37573757
);
@@ -3765,39 +3765,39 @@ fn foo() {
37653765
let v: bool;
37663766
macro_rules! m { () => { v } }
37673767
m!();
3768-
// ^^^^ bool
3768+
// ^^ bool
37693769
37703770
let v: char;
37713771
macro_rules! m { () => { v } }
37723772
m!();
3773-
// ^^^^ char
3773+
// ^^ char
37743774
37753775
{
37763776
let v: u8;
37773777
macro_rules! m { () => { v } }
37783778
m!();
3779-
// ^^^^ u8
3779+
// ^^ u8
37803780
37813781
let v: i8;
37823782
macro_rules! m { () => { v } }
37833783
m!();
3784-
// ^^^^ i8
3784+
// ^^ i8
37853785
37863786
let v: i16;
37873787
macro_rules! m { () => { v } }
37883788
m!();
3789-
// ^^^^ i16
3789+
// ^^ i16
37903790
37913791
{
37923792
let v: u32;
37933793
macro_rules! m { () => { v } }
37943794
m!();
3795-
// ^^^^ u32
3795+
// ^^ u32
37963796
37973797
let v: u64;
37983798
macro_rules! m { () => { v } }
37993799
m!();
3800-
// ^^^^ u64
3800+
// ^^ u64
38013801
}
38023802
}
38033803
}

src/tools/rust-analyzer/crates/ide-db/src/search.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ impl Definition {
317317
};
318318
return match def {
319319
Some(def) => SearchScope::file_range(
320-
def.as_ref().original_file_range_with_macro_call_body(db),
320+
def.as_ref().original_file_range_with_macro_call_input(db),
321321
),
322322
None => SearchScope::single_file(file_id),
323323
};
@@ -332,7 +332,7 @@ impl Definition {
332332
};
333333
return match def {
334334
Some(def) => SearchScope::file_range(
335-
def.as_ref().original_file_range_with_macro_call_body(db),
335+
def.as_ref().original_file_range_with_macro_call_input(db),
336336
),
337337
None => SearchScope::single_file(file_id),
338338
};
@@ -341,7 +341,7 @@ impl Definition {
341341
if let Definition::SelfType(impl_) = self {
342342
return match impl_.source(db).map(|src| src.syntax().cloned()) {
343343
Some(def) => SearchScope::file_range(
344-
def.as_ref().original_file_range_with_macro_call_body(db),
344+
def.as_ref().original_file_range_with_macro_call_input(db),
345345
),
346346
None => SearchScope::single_file(file_id),
347347
};
@@ -360,7 +360,7 @@ impl Definition {
360360
};
361361
return match def {
362362
Some(def) => SearchScope::file_range(
363-
def.as_ref().original_file_range_with_macro_call_body(db),
363+
def.as_ref().original_file_range_with_macro_call_input(db),
364364
),
365365
None => SearchScope::single_file(file_id),
366366
};

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/macro_error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,8 @@ macro_rules! outer {
242242
243243
fn f() {
244244
outer!();
245-
} //^^^^^^^^ error: leftover tokens
246-
//^^^^^^^^ error: Syntax Error in Expansion: expected expression
245+
} //^^^^^^ error: leftover tokens
246+
//^^^^^^ error: Syntax Error in Expansion: expected expression
247247
"#,
248248
)
249249
}

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_fields.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
6666
let current_module =
6767
ctx.sema.scope(d.field_list_parent.to_node(&root).syntax()).map(|it| it.module());
6868
let range = InFile::new(d.file, d.field_list_parent.text_range())
69-
.original_node_file_range_rooted(ctx.sema.db);
69+
.original_node_file_range_rooted_opt(ctx.sema.db)?;
7070

7171
let build_text_edit = |new_syntax: &SyntaxNode, old_syntax| {
7272
let edit = {

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ fn quickfix_for_redundant_assoc_item(
7777
redundant_item_def: String,
7878
range: TextRange,
7979
) -> Option<Vec<Assist>> {
80+
let file_id = d.file_id.file_id()?;
8081
let add_assoc_item_def = |builder: &mut SourceChangeBuilder| -> Option<()> {
8182
let db = ctx.sema.db;
8283
let root = db.parse_or_expand(d.file_id);
@@ -90,12 +91,14 @@ fn quickfix_for_redundant_assoc_item(
9091
let trait_def = d.trait_.source(db)?.value;
9192
let l_curly = trait_def.assoc_item_list()?.l_curly_token()?.text_range();
9293
let where_to_insert =
93-
hir::InFile::new(d.file_id, l_curly).original_node_file_range_rooted(db).range;
94+
hir::InFile::new(d.file_id, l_curly).original_node_file_range_rooted_opt(db)?;
95+
if where_to_insert.file_id != file_id {
96+
return None;
97+
}
9498

95-
builder.insert(where_to_insert.end(), redundant_item_def);
99+
builder.insert(where_to_insert.range.end(), redundant_item_def);
96100
Some(())
97101
};
98-
let file_id = d.file_id.file_id()?;
99102
let mut source_change_builder = SourceChangeBuilder::new(file_id.file_id(ctx.sema.db));
100103
add_assoc_item_def(&mut source_change_builder)?;
101104

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,7 @@ fn assoc_func_fix(
120120

121121
let call = ast::MethodCallExpr::cast(expr.syntax().clone())?;
122122
let range = InFile::new(expr_ptr.file_id, call.syntax().text_range())
123-
.original_node_file_range_rooted(db)
124-
.range;
123+
.original_node_file_range_rooted_opt(db)?;
125124

126125
let receiver = call.receiver()?;
127126
let receiver_type = &ctx.sema.type_of_expr(&receiver)?.original;
@@ -174,18 +173,16 @@ fn assoc_func_fix(
174173

175174
let assoc_func_call_expr_string = make::expr_call(assoc_func_path, args).to_string();
176175

177-
let file_id = ctx.sema.original_range_opt(call.receiver()?.syntax())?.file_id;
178-
179176
Some(Assist {
180177
id: AssistId::quick_fix("method_call_to_assoc_func_call_fix"),
181178
label: Label::new(format!(
182179
"Use associated func call instead: `{assoc_func_call_expr_string}`"
183180
)),
184181
group: None,
185-
target: range,
182+
target: range.range,
186183
source_change: Some(SourceChange::from_text_edit(
187-
file_id.file_id(ctx.sema.db),
188-
TextEdit::replace(range, assoc_func_call_expr_string),
184+
range.file_id.file_id(ctx.sema.db),
185+
TextEdit::replace(range.range, assoc_func_call_expr_string),
189186
)),
190187
command: None,
191188
})
@@ -300,7 +297,7 @@ macro_rules! m {
300297
}
301298
fn main() {
302299
m!(());
303-
// ^^^^^^ error: no method `foo` on type `()`
300+
// ^^ error: no method `foo` on type `()`
304301
}
305302
"#,
306303
);

src/tools/rust-analyzer/crates/ide/src/call_hierarchy.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ macro_rules! call {
592592
"#,
593593
expect!["callee Function FileId(0) 22..37 30..36"],
594594
expect![[r#"
595-
caller Function FileId(0) 38..52 : FileId(0):44..50
595+
caller Function FileId(0) 38..43 : FileId(0):44..50
596596
caller Function FileId(1) 130..136 130..136 : FileId(0):44..50
597597
callee Function FileId(0) 38..52 44..50 : FileId(0):44..50"#]],
598598
expect![[]],

src/tools/rust-analyzer/crates/ide/src/goto_definition.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,7 @@ macro_rules! define_fn {
10821082
}
10831083
10841084
define_fn!();
1085-
//^^^^^^^^^^^^^
1085+
//^^^^^^^^^^
10861086
fn bar() {
10871087
$0foo();
10881088
}
@@ -3228,7 +3228,7 @@ mod bar {
32283228
use crate::m;
32293229
32303230
m!();
3231-
// ^^^^^
3231+
// ^^
32323232
32333233
fn qux() {
32343234
Foo$0;

0 commit comments

Comments
 (0)