Fix diagnostics panicking when resolving to different files due to macros

Veykril · Veykril · commit ba01ff4f88ba · 2023-12-06T14:39:26.000+01:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ pub(crate) fn inactive_code(`
`31`	`31`	`let res = Diagnostic::new(`
`32`	`32`	`DiagnosticCode::Ra("inactive-code", Severity::WeakWarning),`
`33`	`33`	`message,`
`34`		`- ctx.sema.diagnostics_display_range(d.node.clone()).range,`
	`34`	`+ ctx.sema.diagnostics_display_range(d.node.clone()),`
`35`	`35`	`)`
`36`	`36`	`.with_unused(true);`
`37`	`37`	`Some(res)`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ pub(crate) fn typed_hole(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Di`
`26`	`26`	`)`
`27`	`27`	`};`
`28`	`28`
`29`		`- Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range.range)`
	`29`	`+ Diagnostic::new(DiagnosticCode::RustcHardError("typed-hole"), message, display_range)`
`30`	`30`	`.with_fixes(fixes)`
`31`	`31`	`}`
`32`	`32`