Rework #[doc(cfg(..))] checks as distinct pass in rustdoc

Author: Urgau

src/librustdoc/clean/inline.rs

@@ -409,12 +409,12 @@ pub(crate) fn merge_attrs(
             } else {
                 Attributes::from_hir(&both)
             },
-            extract_cfg_from_attrs(both.iter(), cx.tcx, None, &cx.cache.hidden_cfg),
+            extract_cfg_from_attrs(both.iter(), cx.tcx, &cx.cache.hidden_cfg),
         )
     } else {
         (
             Attributes::from_hir(old_attrs),
-            extract_cfg_from_attrs(old_attrs.iter(), cx.tcx, None, &cx.cache.hidden_cfg),
+            extract_cfg_from_attrs(old_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg),
         )
     }
 }

src/librustdoc/clean/mod.rs

@@ -210,7 +210,6 @@ fn generate_item_with_correct_attrs(
             Cow::Owned(attr) => attr,
         }),
         cx.tcx,
-        def_id.as_local().map(|did| cx.tcx.local_def_id_to_hir_id(did)),
         &cx.cache.hidden_cfg,
     );
     let attrs = Attributes::from_hir_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false);

src/librustdoc/clean/types.rs

@@ -12,9 +12,8 @@ use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId};
 use rustc_hir::lang_items::LangItem;
-use rustc_hir::{BodyId, HirId, Mutability};
+use rustc_hir::{BodyId, Mutability};
 use rustc_index::IndexVec;
-use rustc_lint_defs::{BuiltinLintDiag, Lint};
 use rustc_metadata::rendered_const;
 use rustc_middle::span_bug;
 use rustc_middle::ty::fast_reject::SimplifiedType;
@@ -478,12 +477,7 @@ impl Item {
             name,
             kind,
             Attributes::from_hir(hir_attrs),
-            extract_cfg_from_attrs(
-                hir_attrs.iter(),
-                cx.tcx,
-                def_id.as_local().map(|did| cx.tcx.local_def_id_to_hir_id(did)),
-                &cx.cache.hidden_cfg,
-            ),
+            extract_cfg_from_attrs(hir_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg),
         )
     }
 
@@ -1039,7 +1033,6 @@ pub(crate) fn hir_attr_lists<'a, I: IntoIterator<Item = &'a hir::Attribute>>(
 pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute> + Clone>(
     attrs: I,
     tcx: TyCtxt<'_>,
-    hir_id: Option<HirId>,
     hidden_cfg: &FxHashSet<Cfg>,
 ) -> Option<Arc<Cfg>> {
     let doc_cfg_active = tcx.features().doc_cfg();
@@ -1064,42 +1057,10 @@ pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute>
         if doc_cfg.peek().is_some() && doc_cfg_active {
             let sess = tcx.sess;
 
-            struct RustdocCfgMatchesLintEmitter<'a>(TyCtxt<'a>, Option<HirId>);
-
-            impl<'a> rustc_attr_parsing::CfgMatchesLintEmitter for RustdocCfgMatchesLintEmitter<'a> {
-                fn emit_span_lint(
-                    &self,
-                    sess: &Session,
-                    lint: &'static Lint,
-                    sp: rustc_span::Span,
-                    builtin_diag: BuiltinLintDiag,
-                ) {
-                    if let Some(hir_id) = self.1 {
-                        self.0.node_span_lint(lint, hir_id, sp, |diag| {
-                            rustc_lint::decorate_builtin_lint(
-                                sess,
-                                Some(self.0),
-                                builtin_diag,
-                                diag,
-                            )
-                        });
-                    } else {
-                        // No HIR id. Probably in another crate. Don't lint.
-                    }
-                }
-            }
-
             doc_cfg.fold(Cfg::True, |mut cfg, item| {
                 if let Some(cfg_mi) =
                     item.meta_item().and_then(|item| rustc_expand::config::parse_cfg(item, sess))
                 {
-                    // The result is unused here but we can gate unstable predicates
-                    rustc_attr_parsing::cfg_matches(
-                        cfg_mi,
-                        tcx.sess,
-                        RustdocCfgMatchesLintEmitter(tcx, hir_id),
-                        Some(tcx.features()),
-                    );
                     match Cfg::parse(cfg_mi) {
                         Ok(new_cfg) => cfg &= new_cfg,
                         Err(e) => {

src/librustdoc/doctest/rust.rs

@@ -116,12 +116,9 @@ impl HirCollector<'_> {
         nested: F,
     ) {
         let ast_attrs = self.tcx.hir_attrs(self.tcx.local_def_id_to_hir_id(def_id));
-        if let Some(ref cfg) = extract_cfg_from_attrs(
-            ast_attrs.iter(),
-            self.tcx,
-            Some(self.tcx.local_def_id_to_hir_id(def_id)),
-            &FxHashSet::default(),
-        ) && !cfg.matches(&self.tcx.sess.psess)
+        if let Some(ref cfg) =
+            extract_cfg_from_attrs(ast_attrs.iter(), self.tcx, &FxHashSet::default())
+            && !cfg.matches(&self.tcx.sess.psess)
         {
             return;
         }

src/librustdoc/passes/check_doc_cfg.rs

@@ -0,0 +1,76 @@
+use rustc_hir::HirId;
+use rustc_hir::def_id::LocalDefId;
+use rustc_middle::ty::TyCtxt;
+use rustc_span::sym;
+
+use super::Pass;
+use crate::clean::{Attributes, Crate, Item};
+use crate::core::DocContext;
+use crate::visit::DocVisitor;
+
+pub(crate) const CHECK_DOC_CFG: Pass = Pass {
+    name: "check-doc-cfg",
+    run: Some(check_doc_cfg),
+    description: "checks `#[doc(cfg(...))]` for stability feature and unexpected cfgs",
+};
+
+pub(crate) fn check_doc_cfg(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
+    let mut checker = DocCfgChecker { cx };
+    checker.visit_crate(&krate);
+    krate
+}
+
+struct RustdocCfgMatchesLintEmitter<'a>(TyCtxt<'a>, HirId);
+
+impl<'a> rustc_attr_parsing::CfgMatchesLintEmitter for RustdocCfgMatchesLintEmitter<'a> {
+    fn emit_span_lint(
+        &self,
+        sess: &rustc_session::Session,
+        lint: &'static rustc_lint::Lint,
+        sp: rustc_span::Span,
+        builtin_diag: rustc_lint_defs::BuiltinLintDiag,
+    ) {
+        self.0.node_span_lint(lint, self.1, sp, |diag| {
+            rustc_lint::decorate_builtin_lint(sess, Some(self.0), builtin_diag, diag)
+        });
+    }
+}
+
+struct DocCfgChecker<'a, 'tcx> {
+    cx: &'a mut DocContext<'tcx>,
+}
+
+impl DocCfgChecker<'_, '_> {
+    fn check_attrs(&mut self, attrs: &Attributes, did: LocalDefId) {
+        let doc_cfgs = attrs
+            .other_attrs
+            .iter()
+            .filter(|attr| attr.has_name(sym::doc))
+            .flat_map(|attr| attr.meta_item_list().unwrap_or_default())
+            .filter(|attr| attr.has_name(sym::cfg));
+
+        for doc_cfg in doc_cfgs {
+            if let Some([cfg_mi]) = doc_cfg.meta_item_list() {
+                let _ = rustc_attr_parsing::cfg_matches(
+                    cfg_mi,
+                    &self.cx.tcx.sess,
+                    RustdocCfgMatchesLintEmitter(
+                        self.cx.tcx,
+                        self.cx.tcx.local_def_id_to_hir_id(did),
+                    ),
+                    Some(self.cx.tcx.features()),
+                );
+            }
+        }
+    }
+}
+
+impl DocVisitor<'_> for DocCfgChecker<'_, '_> {
+    fn visit_item(&mut self, item: &'_ Item) {
+        if let Some(Some(local_did)) = item.def_id().map(|did| did.as_local()) {
+            self.check_attrs(&item.attrs, local_did);
+        }
+
+        self.visit_item_recur(item);
+    }
+}

src/librustdoc/passes/mod.rs

@@ -32,6 +32,9 @@ pub(crate) use self::collect_intra_doc_links::COLLECT_INTRA_DOC_LINKS;
 mod check_doc_test_visibility;
 pub(crate) use self::check_doc_test_visibility::CHECK_DOC_TEST_VISIBILITY;
 
+mod check_doc_cfg;
+pub(crate) use self::check_doc_cfg::CHECK_DOC_CFG;
+
 mod collect_trait_impls;
 pub(crate) use self::collect_trait_impls::COLLECT_TRAIT_IMPLS;
 
@@ -72,6 +75,7 @@ pub(crate) enum Condition {
 
 /// The full list of passes.
 pub(crate) const PASSES: &[Pass] = &[
+    CHECK_DOC_CFG,
     CHECK_DOC_TEST_VISIBILITY,
     STRIP_ALIASED_NON_LOCAL,
     STRIP_HIDDEN,
@@ -89,6 +93,7 @@ pub(crate) const PASSES: &[Pass] = &[
 pub(crate) const DEFAULT_PASSES: &[ConditionalPass] = &[
     ConditionalPass::always(COLLECT_TRAIT_IMPLS),
     ConditionalPass::always(CHECK_DOC_TEST_VISIBILITY),
+    ConditionalPass::always(CHECK_DOC_CFG),
     ConditionalPass::always(STRIP_ALIASED_NON_LOCAL),
     ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
     ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),

tests/rustdoc-ui/doc-cfg-check-cfg.cfg_empty.stderr

@@ -1,12 +1,30 @@
 warning: unexpected `cfg` condition name: `foo`
-  --> $DIR/doc-cfg-check-cfg.rs:13:11
+  --> $DIR/doc-cfg-check-cfg.rs:12:12
+   |
+LL | #![doc(cfg(foo))]
+   |            ^^^
+   |
+   = help: to expect this configuration use `--check-cfg=cfg(foo)`
+   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
+   = note: `#[warn(unexpected_cfgs)]` on by default
+
+warning: unexpected `cfg` condition name: `foo`
+  --> $DIR/doc-cfg-check-cfg.rs:19:11
+   |
+LL | #[doc(cfg(foo))]
+   |           ^^^
+   |
+   = help: to expect this configuration use `--check-cfg=cfg(foo)`
+   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
+
+warning: unexpected `cfg` condition name: `foo`
+  --> $DIR/doc-cfg-check-cfg.rs:15:11
    |
 LL | #[doc(cfg(foo))]
    |           ^^^
    |
    = help: to expect this configuration use `--check-cfg=cfg(foo)`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
-   = note: `#[warn(unexpected_cfgs)]` on by default
 
-warning: 1 warning emitted
+warning: 3 warnings emitted
 

tests/rustdoc-ui/doc-cfg-check-cfg.rs

@@ -9,11 +9,15 @@
 //@[cfg_foo] compile-flags: --check-cfg cfg(foo)
 
 #![feature(doc_cfg)]
+#![doc(cfg(foo))]
+//[cfg_empty]~^ WARN unexpected `cfg` condition name: `foo`
 
 #[doc(cfg(foo))]
 //[cfg_empty]~^ WARN unexpected `cfg` condition name: `foo`
 pub fn foo() {}
 
+#[doc(cfg(foo))]
+//[cfg_empty]~^ WARN unexpected `cfg` condition name: `foo`
 pub mod module {
     #[allow(unexpected_cfgs)]
     #[doc(cfg(bar))]

tests/rustdoc-ui/doc-cfg.stderr

@@ -10,6 +10,18 @@ error: multiple `cfg` predicates are specified
 LL | #[doc(cfg(), cfg(foo, bar))]
    |                       ^^^
 
+error: `cfg` predicate is not specified
+  --> $DIR/doc-cfg.rs:9:7
+   |
+LL | #[doc(cfg())]
+   |       ^^^^^ help: expected syntax is: `cfg(/* predicate */)`
+
+error: multiple `cfg` predicates are specified
+  --> $DIR/doc-cfg.rs:10:16
+   |
+LL | #[doc(cfg(foo, bar))]
+   |                ^^^
+
 warning: unexpected `cfg` condition name: `foo`
   --> $DIR/doc-cfg.rs:6:11
    |
@@ -30,17 +42,5 @@ LL | #[doc(cfg(foo), cfg(bar))]
    = help: to expect this configuration use `--check-cfg=cfg(bar)`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
 
-error: `cfg` predicate is not specified
-  --> $DIR/doc-cfg.rs:9:7
-   |
-LL | #[doc(cfg())]
-   |       ^^^^^ help: expected syntax is: `cfg(/* predicate */)`
-
-error: multiple `cfg` predicates are specified
-  --> $DIR/doc-cfg.rs:10:16
-   |
-LL | #[doc(cfg(foo, bar))]
-   |                ^^^
-
 error: aborting due to 4 previous errors; 2 warnings emitted
 

tests/rustdoc-ui/issues/issue-91713.stdout

@@ -1,4 +1,5 @@
 Available passes for running rustdoc:
+       check-doc-cfg - checks `#[doc(cfg(...))]` for stability feature and unexpected cfgs
 check_doc_test_visibility - run various visibility-related lints on doctests
 strip-aliased-non-local - strips all non-local private aliased items from the output
         strip-hidden - strips all `#[doc(hidden)]` items from the output
@@ -14,6 +15,7 @@ calculate-doc-coverage - counts the number of items with and without documentati
 Default passes for rustdoc:
  collect-trait-impls
 check_doc_test_visibility
+       check-doc-cfg
 strip-aliased-non-local
         strip-hidden  (when not --document-hidden-items)
        strip-private  (when not --document-private-items)