Merge #6324

6324: Improve #[cfg] diagnostics r=jonas-schievink a=jonas-schievink

Unfortunately I ran into https://github.com/rust-analyzer/rust-analyzer/issues/4058 while testing this on https://github.com/nrf-rs/nrf-hal/, so I didn't see much of it in action yet, but it does seem to work.

Co-authored-by: Jonas Schievink <[email protected]>
Co-authored-by: Jonas Schievink <[email protected]>

5 files changed, 75 insertions, 23 deletions

diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs
index dea552a60..b2ce7ca3c 100644
--- a/crates/hir_def/src/attr.rs
+++ b/crates/hir_def/src/attr.rs
@@ -125,12 +125,20 @@ impl Attrs {
         AttrQuery { attrs: self, key }
     }
 
-    pub fn cfg(&self) -> impl Iterator<Item = CfgExpr> + '_ {
+    pub fn cfg(&self) -> Option<CfgExpr> {
         // FIXME: handle cfg_attr :-)
-        self.by_key("cfg").tt_values().map(CfgExpr::parse)
+        let mut cfgs = self.by_key("cfg").tt_values().map(CfgExpr::parse).collect::<Vec<_>>();
+        match cfgs.len() {
+            0 => None,
+            1 => Some(cfgs.pop().unwrap()),
+            _ => Some(CfgExpr::All(cfgs)),
+        }
     }
     pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool {
-        self.cfg().all(|cfg| cfg_options.check(&cfg) != Some(false))
+        match self.cfg() {
+            None => true,
+            Some(cfg) => cfg_options.check(&cfg) != Some(false),
+        }
     }
 }
 
diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs
index c9c08b01f..532496b62 100644
--- a/crates/hir_def/src/diagnostics.rs
+++ b/crates/hir_def/src/diagnostics.rs
@@ -1,11 +1,12 @@
 //! Diagnostics produced by `hir_def`.
 
 use std::any::Any;
+use stdx::format_to;
 
+use cfg::{CfgExpr, CfgOptions, DnfExpr};
 use hir_expand::diagnostics::{Diagnostic, DiagnosticCode};
-use syntax::{ast, AstPtr, SyntaxNodePtr};
-
 use hir_expand::{HirFileId, InFile};
+use syntax::{ast, AstPtr, SyntaxNodePtr};
 
 // Diagnostic: unresolved-module
 //
@@ -94,6 +95,8 @@ impl Diagnostic for UnresolvedImport {
 pub struct InactiveCode {
     pub file: HirFileId,
     pub node: SyntaxNodePtr,
+    pub cfg: CfgExpr,
+    pub opts: CfgOptions,
 }
 
 impl Diagnostic for InactiveCode {
@@ -101,8 +104,14 @@ impl Diagnostic for InactiveCode {
         DiagnosticCode("inactive-code")
     }
     fn message(&self) -> String {
-        // FIXME: say *why* it is configured out
-        "code is inactive due to #[cfg] directives".to_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())
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index 01a28aeeb..eb41d324e 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -283,6 +283,7 @@ pub enum ModuleSource {
 }
 
 mod diagnostics {
+    use cfg::{CfgExpr, CfgOptions};
     use hir_expand::diagnostics::DiagnosticSink;
     use hir_expand::hygiene::Hygiene;
     use hir_expand::InFile;
@@ -299,7 +300,7 @@ mod diagnostics {
 
         UnresolvedImport { ast: AstId<ast::Use>, index: usize },
 
-        UnconfiguredCode { ast: InFile<SyntaxNodePtr> },
+        UnconfiguredCode { ast: InFile<SyntaxNodePtr>, cfg: CfgExpr, opts: CfgOptions },
     }
 
     #[derive(Debug, PartialEq, Eq)]
@@ -341,8 +342,10 @@ mod diagnostics {
         pub(super) fn unconfigured_code(
             container: LocalModuleId,
             ast: InFile<SyntaxNodePtr>,
+            cfg: CfgExpr,
+            opts: CfgOptions,
         ) -> Self {
-            Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast } }
+            Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast, cfg, opts } }
         }
 
         pub(super) fn add_to(
@@ -395,8 +398,13 @@ mod diagnostics {
                     }
                 }
 
-                DiagnosticKind::UnconfiguredCode { ast } => {
-                    sink.push(InactiveCode { file: ast.file_id, node: ast.value.clone() });
+                DiagnosticKind::UnconfiguredCode { ast, cfg, opts } => {
+                    sink.push(InactiveCode {
+                        file: ast.file_id,
+                        node: ast.value.clone(),
+                        cfg: cfg.clone(),
+                        opts: opts.clone(),
+                    });
                 }
             }
         }
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index bff8edb62..f30172d90 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -6,7 +6,7 @@
 use std::iter;
 
 use base_db::{CrateId, FileId, ProcMacroId};
-use cfg::CfgOptions;
+use cfg::{CfgExpr, CfgOptions};
 use hir_expand::InFile;
 use hir_expand::{
     ast_id_map::FileAstId,
@@ -900,7 +900,8 @@ impl ModCollector<'_, '_> {
         // `#[macro_use] extern crate` is hoisted to imports macros before collecting
         // any other items.
         for item in items {
-            if self.is_cfg_enabled(self.item_tree.attrs((*item).into())) {
+            let attrs = self.item_tree.attrs((*item).into());
+            if attrs.cfg().map_or(true, |cfg| self.is_cfg_enabled(&cfg)) {
                 if let ModItem::ExternCrate(id) = item {
                     let import = self.item_tree[*id].clone();
                     if import.is_macro_use {
@@ -912,9 +913,11 @@ impl ModCollector<'_, '_> {
 
         for &item in items {
             let attrs = self.item_tree.attrs(item.into());
-            if !self.is_cfg_enabled(attrs) {
-                self.emit_unconfigured_diagnostic(item);
-                continue;
+            if let Some(cfg) = attrs.cfg() {
+                if !self.is_cfg_enabled(&cfg) {
+                    self.emit_unconfigured_diagnostic(item, &cfg);
+                    continue;
+                }
             }
             let module =
                 ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id };
@@ -1321,20 +1324,22 @@ impl ModCollector<'_, '_> {
         }
     }
 
-    fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        attrs.is_cfg_enabled(self.def_collector.cfg_options)
+    fn is_cfg_enabled(&self, cfg: &CfgExpr) -> bool {
+        self.def_collector.cfg_options.check(cfg) != Some(false)
     }
 
-    fn emit_unconfigured_diagnostic(&mut self, item: ModItem) {
+    fn emit_unconfigured_diagnostic(&mut self, item: ModItem, cfg: &CfgExpr) {
         let ast_id = item.ast_id(self.item_tree);
         let id_map = self.def_collector.db.ast_id_map(self.file_id);
         let syntax_ptr = id_map.get(ast_id).syntax_node_ptr();
 
         let ast_node = InFile::new(self.file_id, syntax_ptr);
-        self.def_collector
-            .def_map
-            .diagnostics
-            .push(DefDiagnostic::unconfigured_code(self.module_id, ast_node));
+        self.def_collector.def_map.diagnostics.push(DefDiagnostic::unconfigured_code(
+            self.module_id,
+            ast_node,
+            cfg.clone(),
+            self.def_collector.cfg_options.clone(),
+        ));
     }
 }
 
diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs
index 576b813d2..5972248de 100644
--- a/crates/hir_def/src/nameres/tests/diagnostics.rs
+++ b/crates/hir_def/src/nameres/tests/diagnostics.rs
@@ -129,3 +129,25 @@ fn unresolved_module() {
         ",
     );
 }
+
+#[test]
+fn inactive_item() {
+    // Additional tests in `cfg` crate. This only tests disabled cfgs.
+
+    check_diagnostics(
+        r#"
+        //- /lib.rs
+          #[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
+        "#,
+    );
+}