Unnamed repository; edit this file 'description' to name the repository.
Fix diagnostics panicking when resolving to different files due to macros
15 files changed, 158 insertions, 77 deletions
diff --git a/crates/ide-diagnostics/src/handlers/field_shorthand.rs b/crates/ide-diagnostics/src/handlers/field_shorthand.rs index 9ed8199ae4..45fc6f8e68 100644 --- a/crates/ide-diagnostics/src/handlers/field_shorthand.rs +++ b/crates/ide-diagnostics/src/handlers/field_shorthand.rs @@ -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; @@ -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", @@ -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", diff --git a/crates/ide-diagnostics/src/handlers/inactive_code.rs b/crates/ide-diagnostics/src/handlers/inactive_code.rs index 9eb763d3e2..3b2e15a178 100644 --- a/crates/ide-diagnostics/src/handlers/inactive_code.rs +++ b/crates/ide-diagnostics/src/handlers/inactive_code.rs @@ -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) diff --git a/crates/ide-diagnostics/src/handlers/invalid_derive_target.rs b/crates/ide-diagnostics/src/handlers/invalid_derive_target.rs index 1ec17952b2..f68f5b44b1 100644 --- a/crates/ide-diagnostics/src/handlers/invalid_derive_target.rs +++ b/crates/ide-diagnostics/src/handlers/invalid_derive_target.rs @@ -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"), diff --git a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs index 659b74445f..d330973aaa 100644 --- a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs +++ b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs @@ -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, @@ -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); diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs index 2993950be0..099de4528d 100644 --- a/crates/ide-diagnostics/src/handlers/macro_error.rs +++ b/crates/ide-diagnostics/src/handlers/macro_error.rs @@ -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 {} +"#, + ); + } } diff --git a/crates/ide-diagnostics/src/handlers/malformed_derive.rs b/crates/ide-diagnostics/src/handlers/malformed_derive.rs index fc57dde69f..6202d15853 100644 --- a/crates/ide-diagnostics/src/handlers/malformed_derive.rs +++ b/crates/ide-diagnostics/src/handlers/malformed_derive.rs @@ -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"), diff --git a/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs b/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs index 06ba13bcc5..8296018022 100644 --- a/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs +++ b/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs @@ -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}; @@ -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)) => { diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index fd00535d0c..70beb94689 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -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"), @@ -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); @@ -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(()) } diff --git a/crates/ide-diagnostics/src/handlers/typed_hole.rs b/crates/ide-diagnostics/src/handlers/typed_hole.rs index ea5c7564d3..a740e332bb 100644 --- a/crates/ide-diagnostics/src/handlers/typed_hole.rs +++ b/crates/ide-diagnostics/src/handlers/typed_hole.rs @@ -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) } diff --git a/crates/ide-diagnostics/src/handlers/unlinked_file.rs b/crates/ide-diagnostics/src/handlers/unlinked_file.rs index e04f27c27f..becc24ab21 100644 --- a/crates/ide-diagnostics/src/handlers/unlinked_file.rs +++ b/crates/ide-diagnostics/src/handlers/unlinked_file.rs @@ -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, }; @@ -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), ); } diff --git a/crates/ide-diagnostics/src/handlers/unresolved_module.rs b/crates/ide-diagnostics/src/handlers/unresolved_module.rs index 4f81ec0512..e90d385bab 100644 --- a/crates/ide-diagnostics/src/handlers/unresolved_module.rs +++ b/crates/ide-diagnostics/src/handlers/unresolved_module.rs @@ -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, diff --git a/crates/ide-diagnostics/src/handlers/useless_braces.rs b/crates/ide-diagnostics/src/handlers/useless_braces.rs index c4ac59ec2a..8dce2af23e 100644 --- a/crates/ide-diagnostics/src/handlers/useless_braces.rs +++ b/crates/ide-diagnostics/src/handlers/useless_braces.rs @@ -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; @@ -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( diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index f9fb921f40..35272e8726 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -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, @@ -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, @@ -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)))) } @@ -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?; @@ -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, @@ -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() }, ) })); @@ -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), + } } diff --git a/crates/ide-diagnostics/src/tests.rs b/crates/ide-diagnostics/src/tests.rs index c766a018bf..48e0363c9c 100644 --- a/crates/ide-diagnostics/src/tests.rs +++ b/crates/ide-diagnostics/src/tests.rs @@ -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}; @@ -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:?} = {}", diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs index 71701ef161..f80beb9caa 100644 --- a/crates/rust-analyzer/src/diagnostics.rs +++ b/crates/rust-analyzer/src/diagnostics.rs @@ -5,6 +5,7 @@ use std::mem; use ide::FileId; use ide_db::FxHashMap; +use itertools::Itertools; use nohash_hasher::{IntMap, IntSet}; use rustc_hash::FxHashSet; use triomphe::Arc; @@ -129,8 +130,28 @@ pub(crate) fn fetch_native_diagnostics( ) -> Vec<(FileId, Vec<lsp_types::Diagnostic>)> { let _p = profile::span("fetch_native_diagnostics"); let _ctx = stdx::panic_context::enter("fetch_native_diagnostics".to_owned()); - subscriptions - .into_iter() + + let convert_diagnostic = + |line_index: &crate::line_index::LineIndex, d: ide::Diagnostic| lsp_types::Diagnostic { + range: lsp::to_proto::range(&line_index, d.range.range), + severity: Some(lsp::to_proto::diagnostic_severity(d.severity)), + code: Some(lsp_types::NumberOrString::String(d.code.as_str().to_string())), + code_description: Some(lsp_types::CodeDescription { + href: lsp_types::Url::parse(&d.code.url()).unwrap(), + }), + source: Some("rust-analyzer".to_string()), + message: d.message, + related_information: None, + tags: d.unused.then(|| vec![lsp_types::DiagnosticTag::UNNECESSARY]), + data: None, + }; + + // the diagnostics produced may point to different files not requested by the concrete request, + // put those into here and filter later + let mut odd_ones = Vec::new(); + let mut diagnostics = subscriptions + .iter() + .copied() .filter_map(|file_id| { let line_index = snapshot.file_line_index(file_id).ok()?; let diagnostics = snapshot @@ -142,21 +163,39 @@ pub(crate) fn fetch_native_diagnostics( ) .ok()? .into_iter() - .map(move |d| lsp_types::Diagnostic { - range: lsp::to_proto::range(&line_index, d.range), - severity: Some(lsp::to_proto::diagnostic_severity(d.severity)), - code: Some(lsp_types::NumberOrString::String(d.code.as_str().to_string())), - code_description: Some(lsp_types::CodeDescription { - href: lsp_types::Url::parse(&d.code.url()).unwrap(), - }), - source: Some("rust-analyzer".to_string()), - message: d.message, - related_information: None, - tags: d.unused.then(|| vec![lsp_types::DiagnosticTag::UNNECESSARY]), - data: None, + .filter_map(|d| { + if d.range.file_id == file_id { + Some(convert_diagnostic(&line_index, d)) + } else { + odd_ones.push(d); + None + } }) .collect::<Vec<_>>(); Some((file_id, diagnostics)) }) - .collect() + .collect::<Vec<_>>(); + + // Add back any diagnostics that point to files we are subscribed to + for (file_id, group) in odd_ones + .into_iter() + .sorted_by_key(|it| it.range.file_id) + .group_by(|it| it.range.file_id) + .into_iter() + { + if !subscriptions.contains(&file_id) { + continue; + } + let Some((_, diagnostics)) = diagnostics.iter_mut().find(|&&mut (id, _)| id == file_id) + else { + continue; + }; + let Some(line_index) = snapshot.file_line_index(file_id).ok() else { + break; + }; + for diagnostic in group { + diagnostics.push(convert_diagnostic(&line_index, diagnostic)); + } + } + diagnostics } |