Rollup merge of #112836 - GuillaumeGomez:rustdoc-invalid-file-creatio…

…n, r=notriddle

[rustdoc] partially fix invalid files creation

Part of #111249. It only removes generation for modules which shouldn't exist. For files, we need the compiler to keep re-export information alive for external items so we can actually have the right path to their location as it's currently not generating them correctly.

In case the item is inlined, it shouldn't (and neither should its children) get a file generated.

r? ```@notriddle```

src/librustdoc/clean/types.rs

@@ -358,15 +358,15 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
 
 impl Item {
     pub(crate) fn stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<Stability> {
-        self.item_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
+        self.def_id().and_then(|did| tcx.lookup_stability(did))
     }
 
     pub(crate) fn const_stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<ConstStability> {
-        self.item_id.as_def_id().and_then(|did| tcx.lookup_const_stability(did))
+        self.def_id().and_then(|did| tcx.lookup_const_stability(did))
     }
 
     pub(crate) fn deprecation(&self, tcx: TyCtxt<'_>) -> Option<Deprecation> {
-        self.item_id.as_def_id().and_then(|did| tcx.lookup_deprecation(did))
+        self.def_id().and_then(|did| tcx.lookup_deprecation(did))
     }
 
     pub(crate) fn inner_docs(&self, tcx: TyCtxt<'_>) -> bool {
@@ -391,7 +391,7 @@ impl Item {
                     panic!("blanket impl item has non-blanket ID")
                 }
             }
-            _ => self.item_id.as_def_id().map(|did| rustc_span(did, tcx)),
+            _ => self.def_id().map(|did| rustc_span(did, tcx)),
         }
     }
 
@@ -501,7 +501,7 @@ impl Item {
     }
 
     pub(crate) fn is_crate(&self) -> bool {
-        self.is_mod() && self.item_id.as_def_id().map_or(false, |did| did.is_crate_root())
+        self.is_mod() && self.def_id().map_or(false, |did| did.is_crate_root())
     }
     pub(crate) fn is_mod(&self) -> bool {
         self.type_() == ItemType::Module
@@ -638,11 +638,11 @@ impl Item {
         }
         let header = match *self.kind {
             ItemKind::ForeignFunctionItem(_) => {
-                let def_id = self.item_id.as_def_id().unwrap();
+                let def_id = self.def_id().unwrap();
                 let abi = tcx.fn_sig(def_id).skip_binder().abi();
                 hir::FnHeader {
                     unsafety: if abi == Abi::RustIntrinsic {
-                        intrinsic_operation_unsafety(tcx, self.item_id.as_def_id().unwrap())
+                        intrinsic_operation_unsafety(tcx, self.def_id().unwrap())
                     } else {
                         hir::Unsafety::Unsafe
                     },
@@ -659,7 +659,7 @@ impl Item {
                 }
             }
             ItemKind::FunctionItem(_) | ItemKind::MethodItem(_, _) | ItemKind::TyMethodItem(_) => {
-                let def_id = self.item_id.as_def_id().unwrap();
+                let def_id = self.def_id().unwrap();
                 build_fn_header(def_id, tcx, tcx.asyncness(def_id))
             }
             _ => return None,
@@ -738,7 +738,7 @@ impl Item {
                 }
             })
             .collect();
-        if let Some(def_id) = self.item_id.as_def_id() &&
+        if let Some(def_id) = self.def_id() &&
             !def_id.is_local() &&
             // This check is needed because `adt_def` will panic if not a compatible type otherwise...
             matches!(self.type_(), ItemType::Struct | ItemType::Enum | ItemType::Union)
@@ -787,6 +787,10 @@ impl Item {
     pub fn is_doc_hidden(&self) -> bool {
         self.attrs.is_doc_hidden()
     }
+
+    pub fn def_id(&self) -> Option<DefId> {
+        self.item_id.as_def_id()
+    }
 }
 
 #[derive(Clone, Debug)]

src/librustdoc/formats/cache.rs

@@ -121,6 +121,11 @@ pub(crate) struct Cache {
     pub(crate) intra_doc_links: FxHashMap<ItemId, FxIndexSet<clean::ItemLink>>,
     /// Cfg that have been hidden via #![doc(cfg_hide(...))]
     pub(crate) hidden_cfg: FxHashSet<clean::cfg::Cfg>,
+
+    /// Contains the list of `DefId`s which have been inlined. It is used when generating files
+    /// to check if a stripped item should get its file generated or not: if it's inside a
+    /// `#[doc(hidden)]` item or a private one and not inlined, it shouldn't get a file.
+    pub(crate) inlined_items: DefIdSet,
 }
 
 /// This struct is used to wrap the `cache` and `tcx` in order to run `DocFolder`.

src/librustdoc/html/render/context.rs

@@ -73,6 +73,8 @@ pub(crate) struct Context<'tcx> {
     pub(crate) include_sources: bool,
     /// Collection of all types with notable traits referenced in the current module.
     pub(crate) types_with_notable_traits: FxHashSet<clean::Type>,
+    /// Field used during rendering, to know if we're inside an inlined item.
+    pub(crate) is_inside_inlined_module: bool,
 }
 
 // `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
@@ -171,6 +173,19 @@ impl<'tcx> Context<'tcx> {
     }
 
     fn render_item(&mut self, it: &clean::Item, is_module: bool) -> String {
+        let mut render_redirect_pages = self.render_redirect_pages;
+        // If the item is stripped but inlined, links won't point to the item so no need to generate
+        // a file for it.
+        if it.is_stripped() &&
+            let Some(def_id) = it.def_id() &&
+            def_id.is_local()
+        {
+            if self.is_inside_inlined_module || self.shared.cache.inlined_items.contains(&def_id) {
+                // For now we're forced to generate a redirect page for stripped items until
+                // `record_extern_fqn` correctly points to external items.
+                render_redirect_pages = true;
+            }
+        }
         let mut title = String::new();
         if !is_module {
             title.push_str(it.name.unwrap().as_str());
@@ -205,7 +220,7 @@ impl<'tcx> Context<'tcx> {
             tyname.as_str()
         };
 
-        if !self.render_redirect_pages {
+        if !render_redirect_pages {
             let clone_shared = Rc::clone(&self.shared);
             let page = layout::Page {
                 css_class: tyname_s,
@@ -545,6 +560,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             shared: Rc::new(scx),
             include_sources,
             types_with_notable_traits: FxHashSet::default(),
+            is_inside_inlined_module: false,
         };
 
         if emit_crate {
@@ -574,6 +590,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             shared: Rc::clone(&self.shared),
             include_sources: self.include_sources,
             types_with_notable_traits: FxHashSet::default(),
+            is_inside_inlined_module: self.is_inside_inlined_module,
         }
     }
 
@@ -768,12 +785,22 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
 
         info!("Recursing into {}", self.dst.display());
 
-        let buf = self.render_item(item, true);
-        // buf will be empty if the module is stripped and there is no redirect for it
-        if !buf.is_empty() {
-            self.shared.ensure_dir(&self.dst)?;
-            let joint_dst = self.dst.join("index.html");
-            self.shared.fs.write(joint_dst, buf)?;
+        if !item.is_stripped() {
+            let buf = self.render_item(item, true);
+            // buf will be empty if the module is stripped and there is no redirect for it
+            if !buf.is_empty() {
+                self.shared.ensure_dir(&self.dst)?;
+                let joint_dst = self.dst.join("index.html");
+                self.shared.fs.write(joint_dst, buf)?;
+            }
+        }
+        if !self.is_inside_inlined_module {
+            if let Some(def_id) = item.def_id() && self.cache().inlined_items.contains(&def_id) {
+                self.is_inside_inlined_module = true;
+            }
+        } else if item.is_doc_hidden() {
+            // We're not inside an inlined module anymore since this one cannot be re-exported.
+            self.is_inside_inlined_module = false;
         }
 
         // Render sidebar-items.js used throughout this module.

src/librustdoc/visit_ast.rs

@@ -313,7 +313,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             return false;
         }
 
-        let ret = match tcx.hir().get_by_def_id(res_did) {
+        let inlined = match tcx.hir().get_by_def_id(res_did) {
             // Bang macros are handled a bit on their because of how they are handled by the
             // compiler. If they have `#[doc(hidden)]` and the re-export doesn't have
             // `#[doc(inline)]`, then we don't inline it.
@@ -344,7 +344,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             _ => false,
         };
         self.view_item_stack.remove(&res_did);
-        ret
+        if inlined {
+            self.cx.cache.inlined_items.insert(res_did.to_def_id());
+        }
+        inlined
     }
 
     /// Returns `true` if the item is visible, meaning it's not `#[doc(hidden)]` or private.

tests/rustdoc/files-creation-hidden.rs

@@ -0,0 +1,23 @@
+#![crate_name="foo"]
+
+// @!has "foo/struct.Foo.html"
+#[doc(hidden)]
+pub struct Foo;
+
+// @!has "foo/struct.Bar.html"
+pub use crate::Foo as Bar;
+
+// @!has "foo/struct.Baz.html"
+#[doc(hidden)]
+pub use crate::Foo as Baz;
+
+// @!has "foo/foo/index.html"
+#[doc(hidden)]
+pub mod foo {}
+
+// @!has "foo/bar/index.html"
+pub use crate::foo as bar;
+
+// @!has "foo/baz/index.html"
+#[doc(hidden)]
+pub use crate::foo as baz;

tests/rustdoc/files-creation-private.rs

@@ -0,0 +1,18 @@
+#![crate_name="foo"]
+
+// @!has "foo/priv/index.html"
+// @!has "foo/priv/struct.Foo.html"
+mod private {
+    pub struct Foo;
+}
+
+// @has "foo/struct.Bar.html"
+pub use crate::private::Foo as Bar;
+
+// @!has "foo/foo/index.html"
+mod foo {
+    pub mod subfoo {}
+}
+
+// @has "foo/bar/index.html"
+pub use crate::foo::subfoo as bar;

tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs

@@ -3,10 +3,10 @@
 #![crate_name = "foo"]
 
 // @!has 'foo/hidden/index.html'
-// FIXME: add missing `@` for the two next tests once issue is fixed!
-// To be done in <https://github.com/rust-lang/rust/issues/111249>.
-// !has 'foo/hidden/inner/index.html'
-// !has 'foo/hidden/inner/trait.Foo.html'
+// @!has 'foo/hidden/inner/index.html'
+// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
+// @has 'foo/hidden/inner/trait.Foo.html'
+// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
 #[doc(hidden)]
 pub mod hidden {
     pub mod inner {

tests/rustdoc/issue-111249-file-creation.rs

@@ -0,0 +1,34 @@
+#![crate_name = "foo"]
+#![feature(no_core)]
+#![no_core]
+
+// The following five should not fail!
+// @!has 'foo/hidden/index.html'
+// @!has 'foo/hidden/inner/index.html'
+// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
+// @has 'foo/hidden/inner/trait.Foo.html'
+// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
+// @!has 'foo/hidden/inner/inner_hidden/index.html'
+// @!has 'foo/hidden/inner/inner_hidden/trait.HiddenFoo.html'
+#[doc(hidden)]
+pub mod hidden {
+    pub mod inner {
+        pub trait Foo {}
+
+        #[doc(hidden)]
+        pub mod inner_hidden {
+            pub trait HiddenFoo {}
+        }
+    }
+}
+
+// @has 'foo/visible/index.html'
+// @has 'foo/visible/trait.Foo.html'
+#[doc(inline)]
+pub use hidden::inner as visible;
+
+// @has 'foo/struct.Bar.html'
+// @count - '//*[@id="impl-Foo-for-Bar"]' 1
+pub struct Bar;
+
+impl visible::Foo for Bar {}

tests/rustdoc/redirect.rs

@@ -10,7 +10,9 @@ pub trait Foo {}
 // @has - '//code' 'pub use reexp_stripped::Bar'
 // @has - '//code/a' 'Bar'
 // @has - '//a[@href="../reexp_stripped/hidden/struct.Bar.html"]' 'Bar'
+// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
 // @has reexp_stripped/hidden/struct.Bar.html
+// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../reexp_stripped/struct.Bar.html">'
 // @has 'reexp_stripped/struct.Bar.html'
 // @has - '//a[@href="struct.Bar.html"]' 'Bar'
 #[doc(no_inline)]