Skip to content

Commit

Permalink
Fix diagnostics panicking when resolving to different files due to ma…
Browse files Browse the repository at this point in the history
…cros
  • Loading branch information
Veykril committed Dec 6, 2023
1 parent 634d588 commit ba01ff4
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 77 deletions.
9 changes: 6 additions & 3 deletions crates/ide-diagnostics/src/handlers/field_shorthand.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
//! Suggests shortening `Foo { field: field }` to `Foo { field }` in both
//! expressions and patterns.

use ide_db::{base_db::FileId, source_change::SourceChange};
use ide_db::{
base_db::{FileId, FileRange},
source_change::SourceChange,
};
use syntax::{ast, match_ast, AstNode, SyntaxNode};
use text_edit::TextEdit;

Expand Down Expand Up @@ -49,7 +52,7 @@ fn check_expr_field_shorthand(
Diagnostic::new(
DiagnosticCode::Clippy("redundant_field_names"),
"Shorthand struct initialization",
field_range,
FileRange { file_id, range: field_range },
)
.with_fixes(Some(vec![fix(
"use_expr_field_shorthand",
Expand Down Expand Up @@ -93,7 +96,7 @@ fn check_pat_field_shorthand(
Diagnostic::new(
DiagnosticCode::Clippy("redundant_field_names"),
"Shorthand struct pattern",
field_range,
FileRange { file_id, range: field_range },
)
.with_fixes(Some(vec![fix(
"use_pat_field_shorthand",
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/inactive_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub(crate) fn inactive_code(
let res = Diagnostic::new(
DiagnosticCode::Ra("inactive-code", Severity::WeakWarning),
message,
ctx.sema.diagnostics_display_range(d.node.clone()).range,
ctx.sema.diagnostics_display_range(d.node.clone()),
)
.with_unused(true);
Some(res)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub(crate) fn invalid_derive_target(
ctx: &DiagnosticsContext<'_>,
d: &hir::InvalidDeriveTarget,
) -> Diagnostic {
let display_range = ctx.sema.diagnostics_display_range(d.node.clone()).range;
let display_range = ctx.sema.diagnostics_display_range(d.node.clone());

Diagnostic::new(
DiagnosticCode::RustcHardError("E0774"),
Expand Down
4 changes: 2 additions & 2 deletions crates/ide-diagnostics/src/handlers/json_is_not_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use hir::{PathResolution, Semantics};
use ide_db::{
base_db::FileId,
base_db::{FileId, FileRange},
helpers::mod_path_to_ast,
imports::insert_use::{insert_use, ImportScope},
source_change::SourceChangeBuilder,
Expand Down Expand Up @@ -119,7 +119,7 @@ pub(crate) fn json_in_items(
Diagnostic::new(
DiagnosticCode::Ra("json-is-not-rust", Severity::WeakWarning),
"JSON syntax is not valid as a Rust item",
range,
FileRange { file_id, range },
)
.with_fixes(Some(vec![{
let mut scb = SourceChangeBuilder::new(file_id);
Expand Down
20 changes: 20 additions & 0 deletions crates/ide-diagnostics/src/handlers/macro_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,4 +264,24 @@ fn f() {
"#,
)
}

#[test]
fn include_does_not_break_diagnostics() {
let mut config = DiagnosticsConfig::test_sample();
config.disabled.insert("inactive-code".to_string());
config.disabled.insert("unlinked-file".to_string());
check_diagnostics_with_config(
config,
r#"
//- minicore: include
//- /lib.rs crate:lib
include!("include-me.rs");
//- /include-me.rs
/// long doc that pushes the diagnostic range beyond the first file's text length
#[err]
//^^^^^^error: unresolved macro `err`
mod prim_never {}
"#,
);
}
}
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/malformed_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub(crate) fn malformed_derive(
ctx: &DiagnosticsContext<'_>,
d: &hir::MalformedDerive,
) -> Diagnostic {
let display_range = ctx.sema.diagnostics_display_range(d.node.clone()).range;
let display_range = ctx.sema.diagnostics_display_range(d.node.clone());

Diagnostic::new(
DiagnosticCode::RustcHardError("E0777"),
Expand Down
5 changes: 3 additions & 2 deletions crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use either::Either;
use hir::InFile;
use ide_db::base_db::FileRange;
use syntax::{
ast::{self, HasArgList},
AstNode, SyntaxNodePtr, TextRange,
AstNode, SyntaxNodePtr,
};

use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext};
Expand Down Expand Up @@ -48,7 +49,7 @@ fn invalid_args_range(
source: InFile<SyntaxNodePtr>,
expected: usize,
found: usize,
) -> TextRange {
) -> FileRange {
adjusted_display_range::<Either<ast::Expr, ast::TupleStructPat>>(ctx, source, &|expr| {
let (text_range, r_paren_token, expected_arg) = match expr {
Either::Left(ast::Expr::CallExpr(call)) => {
Expand Down
21 changes: 8 additions & 13 deletions crates/ide-diagnostics/src/handlers/type_mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,10 @@ pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch)
Some(salient_token_range)
},
),
pat => {
ctx.sema
.diagnostics_display_range(InFile {
file_id: d.expr_or_pat.file_id,
value: pat.syntax_node_ptr(),
})
.range
}
pat => ctx.sema.diagnostics_display_range(InFile {
file_id: d.expr_or_pat.file_id,
value: pat.syntax_node_ptr(),
}),
};
let mut diag = Diagnostic::new(
DiagnosticCode::RustcHardError("E0308"),
Expand Down Expand Up @@ -84,7 +80,7 @@ fn add_reference(
expr_ptr: &InFile<AstPtr<ast::Expr>>,
acc: &mut Vec<Assist>,
) -> Option<()> {
let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into())).range;
let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into()));

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

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

let edit = TextEdit::insert(range.start(), ampersands);
let source_change =
SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit);
acc.push(fix("add_reference_here", "Add reference here", source_change, range));
let edit = TextEdit::insert(range.range.start(), ampersands);
let source_change = SourceChange::from_text_edit(range.file_id, edit);
acc.push(fix("add_reference_here", "Add reference here", source_change, range.range));
Some(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/typed_hole.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub(crate) fn typed_hole(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Di
)
};

Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range.range)
Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range)
.with_fixes(fixes)
}

Expand Down
10 changes: 7 additions & 3 deletions crates/ide-diagnostics/src/handlers/unlinked_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::iter;

use hir::{db::DefDatabase, DefMap, InFile, ModuleSource};
use ide_db::{
base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt},
base_db::{FileId, FileLoader, FileRange, SourceDatabase, SourceDatabaseExt},
source_change::SourceChange,
RootDatabase,
};
Expand Down Expand Up @@ -46,8 +46,12 @@ pub(crate) fn unlinked_file(
.unwrap_or(range);

acc.push(
Diagnostic::new(DiagnosticCode::Ra("unlinked-file", Severity::WeakWarning), message, range)
.with_fixes(fixes),
Diagnostic::new(
DiagnosticCode::Ra("unlinked-file", Severity::WeakWarning),
message,
FileRange { file_id, range },
)
.with_fixes(fixes),
);
}

Expand Down
7 changes: 6 additions & 1 deletion crates/ide-diagnostics/src/handlers/unresolved_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ mod baz {}
"E0583",
),
message: "unresolved module, can't find module file: foo.rs, or foo/mod.rs",
range: 0..8,
range: FileRange {
file_id: FileId(
0,
),
range: 0..8,
},
severity: Error,
unused: false,
experimental: false,
Expand Down
7 changes: 5 additions & 2 deletions crates/ide-diagnostics/src/handlers/useless_braces.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use hir::InFile;
use ide_db::{base_db::FileId, source_change::SourceChange};
use ide_db::{
base_db::{FileId, FileRange},
source_change::SourceChange,
};
use itertools::Itertools;
use syntax::{ast, AstNode, SyntaxNode};
use text_edit::TextEdit;
Expand Down Expand Up @@ -38,7 +41,7 @@ pub(crate) fn useless_braces(
Diagnostic::new(
DiagnosticCode::RustcLint("unused_braces"),
"Unnecessary braces in use statement".to_string(),
use_range,
FileRange { file_id, range: use_range },
)
.with_main_node(InFile::new(file_id.into(), node.clone()))
.with_fixes(Some(vec![fix(
Expand Down
26 changes: 15 additions & 11 deletions crates/ide-diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl DiagnosticCode {
pub struct Diagnostic {
pub code: DiagnosticCode,
pub message: String,
pub range: TextRange,
pub range: FileRange,
pub severity: Severity,
pub unused: bool,
pub experimental: bool,
Expand All @@ -143,7 +143,7 @@ pub struct Diagnostic {
}

impl Diagnostic {
fn new(code: DiagnosticCode, message: impl Into<String>, range: TextRange) -> Diagnostic {
fn new(code: DiagnosticCode, message: impl Into<String>, range: FileRange) -> Diagnostic {
let message = message.into();
Diagnostic {
code,
Expand Down Expand Up @@ -172,7 +172,7 @@ impl Diagnostic {
node: InFile<SyntaxNodePtr>,
) -> Diagnostic {
let file_id = node.file_id;
Diagnostic::new(code, message, ctx.sema.diagnostics_display_range(node.clone()).range)
Diagnostic::new(code, message, ctx.sema.diagnostics_display_range(node.clone()))
.with_main_node(node.map(|x| x.to_node(&ctx.sema.parse_or_expand(file_id))))
}

Expand Down Expand Up @@ -267,7 +267,7 @@ impl DiagnosticsContext<'_> {
&self,
node: &InFile<SyntaxNodePtr>,
precise_location: Option<TextRange>,
) -> TextRange {
) -> FileRange {
let sema = &self.sema;
(|| {
let precise_location = precise_location?;
Expand All @@ -280,10 +280,11 @@ impl DiagnosticsContext<'_> {
}
})()
.unwrap_or_else(|| sema.diagnostics_display_range(node.clone()))
.range
}
}

/// Request diagnostics for the given [`FileId`]. The produced diagnostics may point to other files
/// due to macros.
pub fn diagnostics(
db: &RootDatabase,
config: &DiagnosticsConfig,
Expand All @@ -300,7 +301,7 @@ pub fn diagnostics(
Diagnostic::new(
DiagnosticCode::RustcHardError("syntax-error"),
format!("Syntax Error: {err}"),
err.range(),
FileRange { file_id, range: err.range() },
)
}));

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

let source_file = ctx.sema.db.parse(file_id);
find_node_at_range::<N>(&source_file.syntax_node(), range)
.filter(|it| it.syntax().text_range() == range)
.and_then(adj)
.unwrap_or(range)
FileRange {
file_id,
range: find_node_at_range::<N>(&source_file.syntax_node(), range)
.filter(|it| it.syntax().text_range() == range)
.and_then(adj)
.unwrap_or(range),
}
}
49 changes: 28 additions & 21 deletions crates/ide-diagnostics/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use ide_db::{
base_db::{fixture::WithFixture, SourceDatabaseExt},
LineIndexDatabase, RootDatabase,
};
use itertools::Itertools;
use stdx::trim_indent;
use test_utils::{assert_eq_text, extract_annotations, MiniCore};

Expand Down Expand Up @@ -103,33 +104,39 @@ pub(crate) fn check_diagnostics(ra_fixture: &str) {
#[track_caller]
pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixture: &str) {
let (db, files) = RootDatabase::with_many_files(ra_fixture);
let mut annotations = files
.iter()
.copied()
.flat_map(|file_id| {
super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id).into_iter().map(
|d| {
let mut annotation = String::new();
if let Some(fixes) = &d.fixes {
assert!(!fixes.is_empty());
annotation.push_str("💡 ")
}
annotation.push_str(match d.severity {
Severity::Error => "error",
Severity::WeakWarning => "weak",
Severity::Warning => "warn",
Severity::Allow => "allow",
});
annotation.push_str(": ");
annotation.push_str(&d.message);
(d.range, annotation)
},
)
})
.map(|(diagnostic, annotation)| (diagnostic.file_id, (diagnostic.range, annotation)))
.into_group_map();
for file_id in files {
let line_index = db.line_index(file_id);
let diagnostics = super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id);

let mut actual = annotations.remove(&file_id).unwrap_or_default();
let expected = extract_annotations(&db.file_text(file_id));
let mut actual = diagnostics
.into_iter()
.map(|d| {
let mut annotation = String::new();
if let Some(fixes) = &d.fixes {
assert!(!fixes.is_empty());
annotation.push_str("💡 ")
}
annotation.push_str(match d.severity {
Severity::Error => "error",
Severity::WeakWarning => "weak",
Severity::Warning => "warn",
Severity::Allow => "allow",
});
annotation.push_str(": ");
annotation.push_str(&d.message);
(d.range, annotation)
})
.collect::<Vec<_>>();
actual.sort_by_key(|(range, _)| range.start());
if expected.is_empty() {
// makes minicore smoke test debugable
// makes minicore smoke test debuggable
for (e, _) in &actual {
eprintln!(
"Code in range {e:?} = {}",
Expand Down

0 comments on commit ba01ff4

Please sign in to comment.