Fix invalid missing documentation lint reporting for re-exports

compiler/rustc_lint/messages.ftl

@@ -370,6 +370,7 @@ lint_builtin_decl_unsafe_method = declaration of an `unsafe` method
 lint_builtin_impl_unsafe_method = implementation of an `unsafe` method
 
 lint_builtin_missing_doc = missing documentation for {$article} {$desc}
+    .note = because it is re-exported here
 
 lint_builtin_missing_copy_impl = type could implement `Copy`; consider adding `impl Copy`
 

compiler/rustc_lint/src/builtin.rs

@@ -66,6 +66,7 @@ use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef};
 use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
 use rustc_span::edition::Edition;
+use rustc_span::hygiene::MacroKind;
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{BytePos, InnerSpan, Span};
@@ -450,6 +451,8 @@ declare_lint! {
 pub struct MissingDoc {
     /// Stack of whether `#[doc(hidden)]` is set at each level which has lint attributes.
     doc_hidden_stack: Vec<bool>,
+    /// Set of types being re-exported while not directly public.
+    reexported_items: FxHashSet<DefId>,
 }
 
 impl_lint_pass!(MissingDoc => [MISSING_DOCS]);
@@ -480,13 +483,18 @@ fn has_doc(attr: &ast::Attribute) -> bool {
 
 impl MissingDoc {
     pub fn new() -> MissingDoc {
-        MissingDoc { doc_hidden_stack: vec![false] }
+        MissingDoc { doc_hidden_stack: vec![false], reexported_items: FxHashSet::default() }
     }
 
     fn doc_hidden(&self) -> bool {
         *self.doc_hidden_stack.last().expect("empty doc_hidden_stack")
     }
 
+    fn has_docs(&self, cx: &LateContext<'_>, def_id: DefId) -> bool {
+        let attrs = cx.tcx.get_attrs_unchecked(def_id);
+        attrs.iter().any(has_doc)
+    }
+
     fn check_missing_docs_attrs(
         &self,
         cx: &LateContext<'_>,
@@ -509,18 +517,21 @@ impl MissingDoc {
         // It's an option so the crate root can also use this function (it doesn't
         // have a `NodeId`).
         if def_id != CRATE_DEF_ID {
-            if !cx.effective_visibilities.is_exported(def_id) {
+            // We will check re-exported items at a later stage.
+            if matches!(cx.tcx.def_kind(def_id), DefKind::Macro(MacroKind::Bang)) {
+                if !cx.effective_visibilities.is_exported(def_id) {
+                    return;
+                }
+            } else if !cx.effective_visibilities.is_directly_public(def_id) {
                 return;
             }
         }
 
-        let attrs = cx.tcx.hir().attrs(cx.tcx.hir().local_def_id_to_hir_id(def_id));
-        let has_doc = attrs.iter().any(has_doc);
-        if !has_doc {
+        if !self.has_docs(cx, def_id.to_def_id()) {
             cx.emit_spanned_lint(
                 MISSING_DOCS,
                 cx.tcx.def_span(def_id),
-                BuiltinMissingDoc { article, desc },
+                BuiltinMissingDoc { article, desc, reexport: None },
             );
         }
     }
@@ -570,6 +581,100 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
             | hir::ItemKind::Const(..)
             | hir::ItemKind::Static(..) => {}
 
+            hir::ItemKind::Use(ref use_path, hir::UseKind::Single) => {
+                let use_def_id = it.owner_id.def_id;
+                if !cx.tcx.is_doc_hidden(use_def_id) &&
+                    // If the re-export is `#[doc(hidden)]` or has an ancestor with this attribute,
+                    // we ignore it.
+                    !cx.tcx.inherits_doc_hidden(use_def_id) &&
+                    let Some(last_res) = use_path.res.last() &&
+                    let Some(res_def_id) = last_res.opt_def_id() &&
+                    // If the original item has `#[doc(hidden)]`, we don't consider it because it
+                    // won't appear into the documentation, unless it's a macro.
+                    (!cx.tcx.is_doc_hidden(res_def_id) || matches!(cx.tcx.def_kind(res_def_id), DefKind::Macro(_))) &&
+                    // We use this to differentiate betweeen `pub use` and `use` since we're only
+                    // interested about re-exports.
+                    match cx.tcx.local_visibility(use_def_id) {
+                        ty::Visibility::Public => true,
+                        ty::Visibility::Restricted(level) => {
+                            level != cx.tcx.parent_module_from_def_id(use_def_id)
+                        }
+                    } &&
+                    // If the re-exported item is not local, it'll be inlined in the documentation
+                    // so it needs to be checked. If it's local and the re-export is the "top one",
+                    // then we check it.
+                    (!res_def_id.is_local() || cx.effective_visibilities.is_directly_public(use_def_id)) &&
+                    !self.has_docs(cx, use_def_id.to_def_id()) &&
+                    !self.has_docs(cx, res_def_id) &&
+                    self.reexported_items.insert(res_def_id)
+                {
+                    let (article, desc) = cx.tcx.article_and_description(res_def_id);
+
+                    cx.emit_spanned_lint(
+                        MISSING_DOCS,
+                        cx.tcx.def_span(res_def_id),
+                        BuiltinMissingDoc { article, desc, reexport: Some(it.span) },
+                    );
+                }
+                return;
+            }
+            hir::ItemKind::Use(ref use_path, hir::UseKind::Glob) => {
+                let use_def_id = it.owner_id.def_id;
+                if cx.effective_visibilities.is_directly_public(use_def_id) &&
+                    !cx.tcx.is_doc_hidden(use_def_id) &&
+                    // If the re-export is `#[doc(hidden)]` or has an ancestor with this attribute,
+                    // we ignore it.
+                    !cx.tcx.inherits_doc_hidden(use_def_id) &&
+                    // We use this to differentiate betweeen `pub use` and `use` since we're only
+                    // interested about re-exports.
+                    match cx.tcx.local_visibility(use_def_id) {
+                        ty::Visibility::Public => true,
+                        ty::Visibility::Restricted(level) => {
+                            level != cx.tcx.parent_module_from_def_id(use_def_id)
+                        }
+                    } &&
+                    let Some(last_res) = use_path.res.last() &&
+                    let Some(res_def_id) = last_res.opt_def_id() &&
+                    let Some(res_def_id) = res_def_id.as_local() &&
+                    // We only check check glob re-exports of modules.
+                    cx.tcx.def_kind(res_def_id) == DefKind::Mod
+                {
+                    for child in cx.tcx.hir().module_items(res_def_id) {
+                        let child_def_id = child.owner_id.to_def_id();
+                        // With glob re-exports, only public items are re-exported.
+                        if cx.effective_visibilities.is_exported(child.owner_id.def_id) &&
+                            // If the original item has `#[doc(hidden)]`, we don't consider it because it
+                            // won't appear into the documentation.
+                            !cx.tcx.is_doc_hidden(child_def_id) &&
+                            !self.has_docs(cx, child_def_id) &&
+                            self.reexported_items.insert(child_def_id) &&
+                            matches!(
+                                cx.tcx.def_kind(child_def_id),
+                                DefKind::TyAlias
+                                | DefKind::Trait
+                                | DefKind::Fn
+                                | DefKind::Macro(..)
+                                | DefKind::Mod
+                                | DefKind::Enum
+                                | DefKind::Struct
+                                | DefKind::Union
+                                | DefKind::Const
+                                | DefKind::Static(..)
+                            )
+                        {
+                            let (article, desc) = cx.tcx.article_and_description(child_def_id);
+
+                            cx.emit_spanned_lint(
+                                MISSING_DOCS,
+                                cx.tcx.def_span(child_def_id),
+                                BuiltinMissingDoc { article, desc, reexport: Some(it.span) },
+                            );
+                        }
+                    }
+                }
+                return;
+            }
+
             _ => return,
         };
 

compiler/rustc_lint/src/lints.rs

@@ -120,6 +120,8 @@ pub enum BuiltinUnsafe {
 pub struct BuiltinMissingDoc<'a> {
     pub article: &'a str,
     pub desc: &'a str,
+    #[note]
+    pub reexport: Option<Span>,
 }
 
 #[derive(LintDiagnostic)]

compiler/rustc_middle/src/ty/context.rs

@@ -931,6 +931,31 @@ impl<'tcx> TyCtxt<'tcx> {
             self.def_path(def_id).to_string_no_crate_verbose()
         )
     }
+
+    /// Returns `true` if the provided `def_id` has any ancestor with the `#[doc(hidden)]`
+    /// attribute.
+    ///
+    /// It doesn't check if `def_id` itself has this attribute though. You will need to use
+    /// [`TyCtxt::is_doc_hidden`] to get this information.
+    pub fn inherits_doc_hidden(self, mut def_id: LocalDefId) -> bool {
+        let hir = self.hir();
+        while let Some(id) = self.opt_local_parent(def_id) {
+            def_id = id;
+            if self.is_doc_hidden(def_id.to_def_id()) {
+                return true;
+            } else if let Some(node) = hir.find_by_def_id(def_id) &&
+                matches!(
+                    node,
+                    hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }),
+                )
+            {
+                // `impl` blocks stand a bit on their own: unless they have `#[doc(hidden)]` directly
+                // on them, they don't inherit it from the parent context.
+                return false;
+            }
+        }
+        false
+    }
 }
 
 impl<'tcx> TyCtxtAt<'tcx> {

library/alloc/src/slice.rs

@@ -77,11 +77,13 @@ pub use core::slice::{SplitInclusive, SplitInclusiveMut};
 // HACK(japaric) needed for the implementation of `vec!` macro during testing
 // N.B., see the `hack` module in this file for more details.
 #[cfg(test)]
+#[allow(missing_docs)]
 pub use hack::into_vec;
 
 // HACK(japaric) needed for the implementation of `Vec::clone` during testing
 // N.B., see the `hack` module in this file for more details.
 #[cfg(test)]
+#[allow(missing_docs)]
 pub use hack::to_vec;
 
 // HACK(japaric): With cfg(test) `impl [T]` is not available, these three

src/librustdoc/passes/check_doc_test_visibility.rs

@@ -11,7 +11,6 @@ use crate::clean::*;
 use crate::core::DocContext;
 use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString};
 use crate::visit::DocVisitor;
-use crate::visit_ast::inherits_doc_hidden;
 use rustc_hir as hir;
 use rustc_middle::lint::LintLevelSource;
 use rustc_session::lint;
@@ -95,7 +94,7 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -
     }
 
     if cx.tcx.is_doc_hidden(def_id.to_def_id())
-        || inherits_doc_hidden(cx.tcx, def_id)
+        || cx.tcx.inherits_doc_hidden(def_id)
         || cx.tcx.def_span(def_id.to_def_id()).in_derive_expansion()
     {
         return false;

src/librustdoc/passes/strip_hidden.rs

@@ -9,7 +9,6 @@ use crate::clean::{Item, ItemIdSet, NestedAttributesExt};
 use crate::core::DocContext;
 use crate::fold::{strip_item, DocFolder};
 use crate::passes::{ImplStripper, Pass};
-use crate::visit_ast::inherits_doc_hidden;
 
 pub(crate) const STRIP_HIDDEN: Pass = Pass {
     name: "strip-hidden",
@@ -92,7 +91,7 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> {
                     .item_id
                     .as_def_id()
                     .and_then(|def_id| def_id.as_local())
-                    .map(|def_id| inherits_doc_hidden(self.tcx, def_id))
+                    .map(|def_id| self.tcx.inherits_doc_hidden(def_id))
                     .unwrap_or(false);
             }
         }

src/librustdoc/visit_ast.rs

@@ -59,26 +59,6 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec<Symbol> {
     std::iter::once(crate_name).chain(relative).collect()
 }
 
-pub(crate) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut def_id: LocalDefId) -> bool {
-    let hir = tcx.hir();
-    while let Some(id) = tcx.opt_local_parent(def_id) {
-        def_id = id;
-        if tcx.is_doc_hidden(def_id.to_def_id()) {
-            return true;
-        } else if let Some(node) = hir.find_by_def_id(def_id) &&
-            matches!(
-                node,
-                hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }),
-            )
-        {
-            // `impl` blocks stand a bit on their own: unless they have `#[doc(hidden)]` directly
-            // on them, they don't inherit it from the parent context.
-            return false;
-        }
-    }
-    false
-}
-
 pub(crate) struct RustdocVisitor<'a, 'tcx> {
     cx: &'a mut core::DocContext<'tcx>,
     view_item_stack: LocalDefIdSet,
@@ -258,7 +238,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
 
         let is_private =
             !self.cx.cache.effective_visibilities.is_directly_public(self.cx.tcx, ori_res_did);
-        let is_hidden = inherits_doc_hidden(self.cx.tcx, res_did);
+        let is_hidden = self.cx.tcx.inherits_doc_hidden(res_did);
 
         // Only inline if requested or if the item would otherwise be stripped.
         if (!please_inline && !is_private && !is_hidden) || is_no_inline {
@@ -279,7 +259,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 .cache
                 .effective_visibilities
                 .is_directly_public(self.cx.tcx, item_def_id.to_def_id()) &&
-            !inherits_doc_hidden(self.cx.tcx, item_def_id)
+            !self.cx.tcx.inherits_doc_hidden(item_def_id)
         {
             // The imported item is public and not `doc(hidden)` so no need to inline it.
             return false;

tests/ui/lint/issue-108570-missing-docs-on-hidden.rs

@@ -0,0 +1,17 @@
+// Regression test for <https://github.com/rust-lang/rust/issues/108570>.
+// If a `#[doc(hidden)]` item is re-exported or if a private item is re-exported
+// with a `#[doc(hidden)]` re-export, it shouldn't complain.
+
+// check-pass
+
+#![crate_type = "lib"]
+
+mod priv_mod {
+    pub struct Foo;
+    #[doc(hidden)]
+    pub struct Bar;
+}
+
+#[doc(hidden)]
+pub use priv_mod::Foo;
+pub use priv_mod::Bar;

tests/ui/lint/lint-missing-doc.stderr

@@ -117,18 +117,36 @@ error: missing documentation for a function
    |
 LL |     pub fn undocumented1() {}
    |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because it is re-exported here
+  --> $DIR/lint-missing-doc.rs:183:5
+   |
+LL |     pub use internal_impl::undocumented1 as bar;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: missing documentation for a function
   --> $DIR/lint-missing-doc.rs:170:5
    |
 LL |     pub fn undocumented2() {}
    |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because it is re-exported here
+  --> $DIR/lint-missing-doc.rs:184:41
+   |
+LL |     pub use internal_impl::{documented, undocumented2};
+   |                                         ^^^^^^^^^^^^^
 
 error: missing documentation for a function
   --> $DIR/lint-missing-doc.rs:176:9
    |
 LL |         pub fn also_undocumented1() {}
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because it is re-exported here
+  --> $DIR/lint-missing-doc.rs:185:5
+   |
+LL |     pub use internal_impl::globbed::*;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: missing documentation for a function
   --> $DIR/lint-missing-doc.rs:191:5

tests/ui/privacy/auxiliary/ctor_aux.rs

@@ -3,7 +3,7 @@
 //! Use the lint to additionally verify that items are reachable
 //! but not exported.
 #![allow(non_camel_case_types)]
-#![deny(missing_docs)]
+// #![deny(missing_docs)]
 
 mod hidden {
     pub struct s;