Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit ba01ff4

Browse files
committed
Fix diagnostics panicking when resolving to different files due to macros
1 parent 634d588 commit ba01ff4

15 files changed

+158
-77
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
//! Suggests shortening `Foo { field: field }` to `Foo { field }` in both
22
//! expressions and patterns.
33
4-
use ide_db::{base_db::FileId, source_change::SourceChange};
4+
use ide_db::{
5+
base_db::{FileId, FileRange},
6+
source_change::SourceChange,
7+
};
58
use syntax::{ast, match_ast, AstNode, SyntaxNode};
69
use text_edit::TextEdit;
710

@@ -49,7 +52,7 @@ fn check_expr_field_shorthand(
4952
Diagnostic::new(
5053
DiagnosticCode::Clippy("redundant_field_names"),
5154
"Shorthand struct initialization",
52-
field_range,
55+
FileRange { file_id, range: field_range },
5356
)
5457
.with_fixes(Some(vec![fix(
5558
"use_expr_field_shorthand",
@@ -93,7 +96,7 @@ fn check_pat_field_shorthand(
9396
Diagnostic::new(
9497
DiagnosticCode::Clippy("redundant_field_names"),
9598
"Shorthand struct pattern",
96-
field_range,
99+
FileRange { file_id, range: field_range },
97100
)
98101
.with_fixes(Some(vec![fix(
99102
"use_pat_field_shorthand",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub(crate) fn inactive_code(
3131
let res = Diagnostic::new(
3232
DiagnosticCode::Ra("inactive-code", Severity::WeakWarning),
3333
message,
34-
ctx.sema.diagnostics_display_range(d.node.clone()).range,
34+
ctx.sema.diagnostics_display_range(d.node.clone()),
3535
)
3636
.with_unused(true);
3737
Some(res)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ pub(crate) fn invalid_derive_target(
88
ctx: &DiagnosticsContext<'_>,
99
d: &hir::InvalidDeriveTarget,
1010
) -> Diagnostic {
11-
let display_range = ctx.sema.diagnostics_display_range(d.node.clone()).range;
11+
let display_range = ctx.sema.diagnostics_display_range(d.node.clone());
1212

1313
Diagnostic::new(
1414
DiagnosticCode::RustcHardError("E0774"),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
44
use hir::{PathResolution, Semantics};
55
use ide_db::{
6-
base_db::FileId,
6+
base_db::{FileId, FileRange},
77
helpers::mod_path_to_ast,
88
imports::insert_use::{insert_use, ImportScope},
99
source_change::SourceChangeBuilder,
@@ -119,7 +119,7 @@ pub(crate) fn json_in_items(
119119
Diagnostic::new(
120120
DiagnosticCode::Ra("json-is-not-rust", Severity::WeakWarning),
121121
"JSON syntax is not valid as a Rust item",
122-
range,
122+
FileRange { file_id, range },
123123
)
124124
.with_fixes(Some(vec![{
125125
let mut scb = SourceChangeBuilder::new(file_id);

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,4 +264,24 @@ fn f() {
264264
"#,
265265
)
266266
}
267+
268+
#[test]
269+
fn include_does_not_break_diagnostics() {
270+
let mut config = DiagnosticsConfig::test_sample();
271+
config.disabled.insert("inactive-code".to_string());
272+
config.disabled.insert("unlinked-file".to_string());
273+
check_diagnostics_with_config(
274+
config,
275+
r#"
276+
//- minicore: include
277+
//- /lib.rs crate:lib
278+
include!("include-me.rs");
279+
//- /include-me.rs
280+
/// long doc that pushes the diagnostic range beyond the first file's text length
281+
#[err]
282+
//^^^^^^error: unresolved macro `err`
283+
mod prim_never {}
284+
"#,
285+
);
286+
}
267287
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub(crate) fn malformed_derive(
77
ctx: &DiagnosticsContext<'_>,
88
d: &hir::MalformedDerive,
99
) -> Diagnostic {
10-
let display_range = ctx.sema.diagnostics_display_range(d.node.clone()).range;
10+
let display_range = ctx.sema.diagnostics_display_range(d.node.clone());
1111

1212
Diagnostic::new(
1313
DiagnosticCode::RustcHardError("E0777"),

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use either::Either;
22
use hir::InFile;
3+
use ide_db::base_db::FileRange;
34
use syntax::{
45
ast::{self, HasArgList},
5-
AstNode, SyntaxNodePtr, TextRange,
6+
AstNode, SyntaxNodePtr,
67
};
78

89
use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext};
@@ -48,7 +49,7 @@ fn invalid_args_range(
4849
source: InFile<SyntaxNodePtr>,
4950
expected: usize,
5051
found: usize,
51-
) -> TextRange {
52+
) -> FileRange {
5253
adjusted_display_range::<Either<ast::Expr, ast::TupleStructPat>>(ctx, source, &|expr| {
5354
let (text_range, r_paren_token, expected_arg) = match expr {
5455
Either::Left(ast::Expr::CallExpr(call)) => {

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,10 @@ pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch)
3535
Some(salient_token_range)
3636
},
3737
),
38-
pat => {
39-
ctx.sema
40-
.diagnostics_display_range(InFile {
41-
file_id: d.expr_or_pat.file_id,
42-
value: pat.syntax_node_ptr(),
43-
})
44-
.range
45-
}
38+
pat => ctx.sema.diagnostics_display_range(InFile {
39+
file_id: d.expr_or_pat.file_id,
40+
value: pat.syntax_node_ptr(),
41+
}),
4642
};
4743
let mut diag = Diagnostic::new(
4844
DiagnosticCode::RustcHardError("E0308"),
@@ -84,7 +80,7 @@ fn add_reference(
8480
expr_ptr: &InFile<AstPtr<ast::Expr>>,
8581
acc: &mut Vec<Assist>,
8682
) -> Option<()> {
87-
let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into())).range;
83+
let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into()));
8884

8985
let (_, mutability) = d.expected.as_reference()?;
9086
let actual_with_ref = Type::reference(&d.actual, mutability);
@@ -94,10 +90,9 @@ fn add_reference(
9490

9591
let ampersands = format!("&{}", mutability.as_keyword_for_ref());
9692

97-
let edit = TextEdit::insert(range.start(), ampersands);
98-
let source_change =
99-
SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit);
100-
acc.push(fix("add_reference_here", "Add reference here", source_change, range));
93+
let edit = TextEdit::insert(range.range.start(), ampersands);
94+
let source_change = SourceChange::from_text_edit(range.file_id, edit);
95+
acc.push(fix("add_reference_here", "Add reference here", source_change, range.range));
10196
Some(())
10297
}
10398

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub(crate) fn typed_hole(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Di
2626
)
2727
};
2828

29-
Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range.range)
29+
Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range)
3030
.with_fixes(fixes)
3131
}
3232

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::iter;
44

55
use hir::{db::DefDatabase, DefMap, InFile, ModuleSource};
66
use ide_db::{
7-
base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt},
7+
base_db::{FileId, FileLoader, FileRange, SourceDatabase, SourceDatabaseExt},
88
source_change::SourceChange,
99
RootDatabase,
1010
};
@@ -46,8 +46,12 @@ pub(crate) fn unlinked_file(
4646
.unwrap_or(range);
4747

4848
acc.push(
49-
Diagnostic::new(DiagnosticCode::Ra("unlinked-file", Severity::WeakWarning), message, range)
50-
.with_fixes(fixes),
49+
Diagnostic::new(
50+
DiagnosticCode::Ra("unlinked-file", Severity::WeakWarning),
51+
message,
52+
FileRange { file_id, range },
53+
)
54+
.with_fixes(fixes),
5155
);
5256
}
5357

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,12 @@ mod baz {}
8787
"E0583",
8888
),
8989
message: "unresolved module, can't find module file: foo.rs, or foo/mod.rs",
90-
range: 0..8,
90+
range: FileRange {
91+
file_id: FileId(
92+
0,
93+
),
94+
range: 0..8,
95+
},
9196
severity: Error,
9297
unused: false,
9398
experimental: false,

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
use hir::InFile;
2-
use ide_db::{base_db::FileId, source_change::SourceChange};
2+
use ide_db::{
3+
base_db::{FileId, FileRange},
4+
source_change::SourceChange,
5+
};
36
use itertools::Itertools;
47
use syntax::{ast, AstNode, SyntaxNode};
58
use text_edit::TextEdit;
@@ -38,7 +41,7 @@ pub(crate) fn useless_braces(
3841
Diagnostic::new(
3942
DiagnosticCode::RustcLint("unused_braces"),
4043
"Unnecessary braces in use statement".to_string(),
41-
use_range,
44+
FileRange { file_id, range: use_range },
4245
)
4346
.with_main_node(InFile::new(file_id.into(), node.clone()))
4447
.with_fixes(Some(vec![fix(

crates/ide-diagnostics/src/lib.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl DiagnosticCode {
133133
pub struct Diagnostic {
134134
pub code: DiagnosticCode,
135135
pub message: String,
136-
pub range: TextRange,
136+
pub range: FileRange,
137137
pub severity: Severity,
138138
pub unused: bool,
139139
pub experimental: bool,
@@ -143,7 +143,7 @@ pub struct Diagnostic {
143143
}
144144

145145
impl Diagnostic {
146-
fn new(code: DiagnosticCode, message: impl Into<String>, range: TextRange) -> Diagnostic {
146+
fn new(code: DiagnosticCode, message: impl Into<String>, range: FileRange) -> Diagnostic {
147147
let message = message.into();
148148
Diagnostic {
149149
code,
@@ -172,7 +172,7 @@ impl Diagnostic {
172172
node: InFile<SyntaxNodePtr>,
173173
) -> Diagnostic {
174174
let file_id = node.file_id;
175-
Diagnostic::new(code, message, ctx.sema.diagnostics_display_range(node.clone()).range)
175+
Diagnostic::new(code, message, ctx.sema.diagnostics_display_range(node.clone()))
176176
.with_main_node(node.map(|x| x.to_node(&ctx.sema.parse_or_expand(file_id))))
177177
}
178178

@@ -267,7 +267,7 @@ impl DiagnosticsContext<'_> {
267267
&self,
268268
node: &InFile<SyntaxNodePtr>,
269269
precise_location: Option<TextRange>,
270-
) -> TextRange {
270+
) -> FileRange {
271271
let sema = &self.sema;
272272
(|| {
273273
let precise_location = precise_location?;
@@ -280,10 +280,11 @@ impl DiagnosticsContext<'_> {
280280
}
281281
})()
282282
.unwrap_or_else(|| sema.diagnostics_display_range(node.clone()))
283-
.range
284283
}
285284
}
286285

286+
/// Request diagnostics for the given [`FileId`]. The produced diagnostics may point to other files
287+
/// due to macros.
287288
pub fn diagnostics(
288289
db: &RootDatabase,
289290
config: &DiagnosticsConfig,
@@ -300,7 +301,7 @@ pub fn diagnostics(
300301
Diagnostic::new(
301302
DiagnosticCode::RustcHardError("syntax-error"),
302303
format!("Syntax Error: {err}"),
303-
err.range(),
304+
FileRange { file_id, range: err.range() },
304305
)
305306
}));
306307

@@ -569,12 +570,15 @@ fn adjusted_display_range<N: AstNode>(
569570
ctx: &DiagnosticsContext<'_>,
570571
diag_ptr: InFile<SyntaxNodePtr>,
571572
adj: &dyn Fn(N) -> Option<TextRange>,
572-
) -> TextRange {
573+
) -> FileRange {
573574
let FileRange { file_id, range } = ctx.sema.diagnostics_display_range(diag_ptr);
574575

575576
let source_file = ctx.sema.db.parse(file_id);
576-
find_node_at_range::<N>(&source_file.syntax_node(), range)
577-
.filter(|it| it.syntax().text_range() == range)
578-
.and_then(adj)
579-
.unwrap_or(range)
577+
FileRange {
578+
file_id,
579+
range: find_node_at_range::<N>(&source_file.syntax_node(), range)
580+
.filter(|it| it.syntax().text_range() == range)
581+
.and_then(adj)
582+
.unwrap_or(range),
583+
}
580584
}

crates/ide-diagnostics/src/tests.rs

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use ide_db::{
77
base_db::{fixture::WithFixture, SourceDatabaseExt},
88
LineIndexDatabase, RootDatabase,
99
};
10+
use itertools::Itertools;
1011
use stdx::trim_indent;
1112
use test_utils::{assert_eq_text, extract_annotations, MiniCore};
1213

@@ -103,33 +104,39 @@ pub(crate) fn check_diagnostics(ra_fixture: &str) {
103104
#[track_caller]
104105
pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixture: &str) {
105106
let (db, files) = RootDatabase::with_many_files(ra_fixture);
107+
let mut annotations = files
108+
.iter()
109+
.copied()
110+
.flat_map(|file_id| {
111+
super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id).into_iter().map(
112+
|d| {
113+
let mut annotation = String::new();
114+
if let Some(fixes) = &d.fixes {
115+
assert!(!fixes.is_empty());
116+
annotation.push_str("💡 ")
117+
}
118+
annotation.push_str(match d.severity {
119+
Severity::Error => "error",
120+
Severity::WeakWarning => "weak",
121+
Severity::Warning => "warn",
122+
Severity::Allow => "allow",
123+
});
124+
annotation.push_str(": ");
125+
annotation.push_str(&d.message);
126+
(d.range, annotation)
127+
},
128+
)
129+
})
130+
.map(|(diagnostic, annotation)| (diagnostic.file_id, (diagnostic.range, annotation)))
131+
.into_group_map();
106132
for file_id in files {
107133
let line_index = db.line_index(file_id);
108-
let diagnostics = super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id);
109134

135+
let mut actual = annotations.remove(&file_id).unwrap_or_default();
110136
let expected = extract_annotations(&db.file_text(file_id));
111-
let mut actual = diagnostics
112-
.into_iter()
113-
.map(|d| {
114-
let mut annotation = String::new();
115-
if let Some(fixes) = &d.fixes {
116-
assert!(!fixes.is_empty());
117-
annotation.push_str("💡 ")
118-
}
119-
annotation.push_str(match d.severity {
120-
Severity::Error => "error",
121-
Severity::WeakWarning => "weak",
122-
Severity::Warning => "warn",
123-
Severity::Allow => "allow",
124-
});
125-
annotation.push_str(": ");
126-
annotation.push_str(&d.message);
127-
(d.range, annotation)
128-
})
129-
.collect::<Vec<_>>();
130137
actual.sort_by_key(|(range, _)| range.start());
131138
if expected.is_empty() {
132-
// makes minicore smoke test debugable
139+
// makes minicore smoke test debuggable
133140
for (e, _) in &actual {
134141
eprintln!(
135142
"Code in range {e:?} = {}",

0 commit comments

Comments
 (0)