internal: refactor inactive code diagnostics

crates/hir/src/diagnostics.rs

@@ -5,11 +5,10 @@
 //! be expressed in terms of hir types themselves.
 use std::any::Any;
 
-use cfg::{CfgExpr, CfgOptions, DnfExpr};
+use cfg::{CfgExpr, CfgOptions};
 use either::Either;
 use hir_def::path::ModPath;
 use hir_expand::{name::Name, HirFileId, InFile};
-use stdx::format_to;
 use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
 
 pub use crate::diagnostics_sink::{
@@ -38,6 +37,7 @@ diagnostics![
     UnresolvedImport,
     UnresolvedMacroCall,
     MissingFields,
+    InactiveCode,
 ];
 
 #[derive(Debug)]
@@ -62,39 +62,13 @@ pub struct UnresolvedMacroCall {
     pub path: ModPath,
 }
 
-// Diagnostic: inactive-code
-//
-// This diagnostic is shown for code with inactive `#[cfg]` attributes.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct InactiveCode {
-    pub file: HirFileId,
+    pub node: InFile<SyntaxNodePtr>,
-    pub node: SyntaxNodePtr,
     pub cfg: CfgExpr,
     pub opts: CfgOptions,
 }
 
-impl Diagnostic for InactiveCode {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("inactive-code")
-    }
-    fn message(&self) -> String {
-        let inactive = DnfExpr::new(self.cfg.clone()).why_inactive(&self.opts);
-        let mut buf = "code is inactive due to #[cfg] directives".to_string();
-
-        if let Some(inactive) = inactive {
-            format_to!(buf, ": {}", inactive);
-        }
-
-        buf
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile::new(self.file, self.node.clone())
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
 // Diagnostic: unresolved-proc-macro
 //
 // This diagnostic is shown when a procedural macro can not be found. This usually means that

crates/hir/src/lib.rs

@@ -506,12 +506,14 @@ impl Module {
 
                 DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } => {
                     let item = ast.to_node(db.upcast());
-                    sink.push(InactiveCode {
+                    acc.push(
-                        file: ast.file_id,
+                        InactiveCode {
-                        node: AstPtr::new(&item).into(),
+                            node: ast.with_value(AstPtr::new(&item).into()),
-                        cfg: cfg.clone(),
+                            cfg: cfg.clone(),
-                        opts: opts.clone(),
+                            opts: opts.clone(),
-                    });
+                        }
+                        .into(),
+                    );
                 }
 
                 DefDiagnosticKind::UnresolvedProcMacro { ast } => {
@@ -1045,12 +1047,10 @@ impl Function {
         let source_map = db.body_with_source_map(self.id.into()).1;
         for diag in source_map.diagnostics() {
             match diag {
-                BodyDiagnostic::InactiveCode { node, cfg, opts } => sink.push(InactiveCode {
+                BodyDiagnostic::InactiveCode { node, cfg, opts } => acc.push(
-                    file: node.file_id,
+                    InactiveCode { node: node.clone(), cfg: cfg.clone(), opts: opts.clone() }
-                    node: node.value.clone(),
+                        .into(),
-                    cfg: cfg.clone(),
+                ),
-                    opts: opts.clone(),
-                }),
                 BodyDiagnostic::MacroError { node, message } => sink.push(MacroError {
                     file: node.file_id,
                     node: node.value.clone().into(),

crates/hir_def/src/body/tests.rs

@@ -88,39 +88,6 @@ mod m {
     );
 }
 
-#[test]
-fn cfg_diagnostics() {
-    check_diagnostics(
-        r"
-fn f() {
-    // The three g̶e̶n̶d̶e̶r̶s̶ statements:
-
-    #[cfg(a)] fn f() {}  // Item statement
-  //^^^^^^^^^^^^^^^^^^^ InactiveCode
-    #[cfg(a)] {}         // Expression statement
-  //^^^^^^^^^^^^ InactiveCode
-    #[cfg(a)] let x = 0; // let statement
-  //^^^^^^^^^^^^^^^^^^^^ InactiveCode
-
-    abc(#[cfg(a)] 0);
-      //^^^^^^^^^^^ InactiveCode
-    let x = Struct {
-        #[cfg(a)] f: 0,
-      //^^^^^^^^^^^^^^ InactiveCode
-    };
-    match () {
-        () => (),
-        #[cfg(a)] () => (),
-      //^^^^^^^^^^^^^^^^^^ InactiveCode
-    }
-
-    #[cfg(a)] 0          // Trailing expression of block
-  //^^^^^^^^^^^ InactiveCode
-}
-    ",
-    );
-}
-
 #[test]
 fn macro_diag_builtin() {
     check_diagnostics(

crates/hir_def/src/nameres/tests/diagnostics.rs

@@ -12,48 +12,6 @@ fn check_no_diagnostics(ra_fixture: &str) {
     db.check_no_diagnostics();
 }
 
-#[test]
-fn inactive_item() {
-    // Additional tests in `cfg` crate. This only tests disabled cfgs.
-
-    check_diagnostics(
-        r#"
-        //- /lib.rs
-          #[cfg(no)] pub fn f() {}
-        //^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-
-          #[cfg(no)] #[cfg(no2)] mod m;
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-
-          #[cfg(all(not(a), b))] enum E {}
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-
-          #[cfg(feature = "std")] use std;
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-        "#,
-    );
-}
-
-/// Tests that `cfg` attributes behind `cfg_attr` is handled properly.
-#[test]
-fn inactive_via_cfg_attr() {
-    cov_mark::check!(cfg_attr_active);
-    check_diagnostics(
-        r#"
-        //- /lib.rs
-          #[cfg_attr(not(never), cfg(no))] fn f() {}
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-
-          #[cfg_attr(not(never), cfg(not(no)))] fn f() {}
-
-          #[cfg_attr(never, cfg(no))] fn g() {}
-
-          #[cfg_attr(not(never), inline, cfg(no))] fn h() {}
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode
-        "#,
-    );
-}
-
 #[test]
 fn builtin_macro_fails_expansion() {
     check_diagnostics(

crates/ide/src/diagnostics.rs

@@ -8,6 +8,7 @@ mod unresolved_module;
 mod unresolved_extern_crate;
 mod unresolved_import;
 mod unresolved_macro_call;
+mod inactive_code;
 mod missing_fields;
 
 mod fixes;
@@ -164,22 +165,6 @@ pub(crate) fn diagnostics(
         .on::<hir::diagnostics::ReplaceFilterMapNextWithFindMap, _>(|d| {
             res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
         })
-        .on::<hir::diagnostics::InactiveCode, _>(|d| {
-            // If there's inactive code somewhere in a macro, don't propagate to the call-site.
-            if d.display_source().file_id.expansion_info(db).is_some() {
-                return;
-            }
-
-            // Override severity and mark as unused.
-            res.borrow_mut().push(
-                Diagnostic::hint(
-                    sema.diagnostics_display_range(d.display_source()).range,
-                    d.message(),
-                )
-                .with_unused(true)
-                .with_code(Some(d.code())),
-            );
-        })
         .on::<UnlinkedFile, _>(|d| {
             // Limit diagnostic to the first few characters in the file. This matches how VS Code
             // renders it with the full span, but on other editors, and is less invasive.
@@ -247,6 +232,11 @@ pub(crate) fn diagnostics(
             AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d),
             AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d),
             AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d),
+
+            AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) {
+                Some(it) => it,
+                None => continue,
+            }
         };
         if let Some(code) = d.code {
             if ctx.config.disabled.contains(code.as_str()) {
@@ -451,7 +441,13 @@ mod tests {
         expect.assert_debug_eq(&diagnostics)
     }
 
+    #[track_caller]
     pub(crate) fn check_diagnostics(ra_fixture: &str) {
+        check_diagnostics_with_inactive_code(ra_fixture, false)
+    }
+
+    #[track_caller]
+    pub(crate) fn check_diagnostics_with_inactive_code(ra_fixture: &str, with_inactive_code: bool) {
         let (analysis, file_id) = fixture::file(ra_fixture);
         let diagnostics = analysis
             .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id)
@@ -460,7 +456,7 @@ mod tests {
         let expected = extract_annotations(&*analysis.file_text(file_id).unwrap());
         let mut actual = diagnostics
             .into_iter()
-            .filter(|d| d.code != Some(DiagnosticCode("inactive-code")))
+            .filter(|d| d.code != Some(DiagnosticCode("inactive-code")) || with_inactive_code)
             .map(|d| (d.range, d.message))
             .collect::<Vec<_>>();
         actual.sort_by_key(|(range, _)| range.start());

crates/ide/src/diagnostics/inactive_code.rs

@@ -0,0 +1,113 @@
+use cfg::DnfExpr;
+use stdx::format_to;
+
+use crate::diagnostics::{Diagnostic, DiagnosticsContext};
+
+// Diagnostic: inactive-code
+//
+// This diagnostic is shown for code with inactive `#[cfg]` attributes.
+pub(super) fn inactive_code(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::InactiveCode,
+) -> Option<Diagnostic> {
+    // If there's inactive code somewhere in a macro, don't propagate to the call-site.
+    if d.node.file_id.expansion_info(ctx.sema.db).is_some() {
+        return None;
+    }
+
+    let inactive = DnfExpr::new(d.cfg.clone()).why_inactive(&d.opts);
+    let mut message = "code is inactive due to #[cfg] directives".to_string();
+
+    if let Some(inactive) = inactive {
+        format_to!(message, ": {}", inactive);
+    }
+
+    let res = Diagnostic::new(
+        "inactive-code",
+        message,
+        ctx.sema.diagnostics_display_range(d.node.clone()).range,
+    )
+    .with_unused(true);
+    Some(res)
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostics::tests::check_diagnostics_with_inactive_code;
+
+    #[test]
+    fn cfg_diagnostics() {
+        check_diagnostics_with_inactive_code(
+            r#"
+fn f() {
+    // The three g̶e̶n̶d̶e̶r̶s̶ statements:
+
+    #[cfg(a)] fn f() {}  // Item statement
+  //^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+    #[cfg(a)] {}         // Expression statement
+  //^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+    #[cfg(a)] let x = 0; // let statement
+  //^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+
+    abc(#[cfg(a)] 0);
+      //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+    let x = Struct {
+        #[cfg(a)] f: 0,
+      //^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+    };
+    match () {
+        () => (),
+        #[cfg(a)] () => (),
+      //^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+    }
+
+    #[cfg(a)] 0          // Trailing expression of block
+  //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled
+}
+        "#,
+            true,
+        );
+    }
+
+    #[test]
+    fn inactive_item() {
+        // Additional tests in `cfg` crate. This only tests disabled cfgs.
+
+        check_diagnostics_with_inactive_code(
+            r#"
+    #[cfg(no)] pub fn f() {}
+  //^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled
+
+    #[cfg(no)] #[cfg(no2)] mod m;
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no and no2 are disabled
+
+    #[cfg(all(not(a), b))] enum E {}
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: b is disabled
+
+    #[cfg(feature = "std")] use std;
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: feature = "std" is disabled
+"#,
+            true,
+        );
+    }
+
+    /// Tests that `cfg` attributes behind `cfg_attr` is handled properly.
+    #[test]
+    fn inactive_via_cfg_attr() {
+        cov_mark::check!(cfg_attr_active);
+        check_diagnostics_with_inactive_code(
+            r#"
+    #[cfg_attr(not(never), cfg(no))] fn f() {}
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled
+
+    #[cfg_attr(not(never), cfg(not(no)))] fn f() {}
+
+    #[cfg_attr(never, cfg(no))] fn g() {}
+
+    #[cfg_attr(not(never), inline, cfg(no))] fn h() {}
+  //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled
+"#,
+            true,
+        );
+    }
+}