Rollup merge of #99287 - GuillaumeGomez:rustdoc-json-double-export, r…

…=notriddle

[rustdoc-json] JSON no longer inlines

Fixes #98007.
Fixes #96161.
Fixes #83057.
Fixes #83720.

I took over #93518 and applied the comments and added more tests.

There was one thing missing (which is in the second commit): if a non-exported item was used in a public API but not reexported, it was still missing.

cc `@CraftSpider` `@Urgau` `@Enselic`

r? `@notriddle`

src/librustdoc/clean/mod.rs

@@ -2120,8 +2120,9 @@ fn clean_use_statement<'tcx>(
     // forcefully don't inline if this is not public or if the
     // #[doc(no_inline)] attribute is present.
     // Don't inline doc(hidden) imports so they can be stripped at a later stage.
-    let mut denied = !(visibility.is_public()
-        || (cx.render_options.document_private && is_visible_from_parent_mod))
+    let mut denied = cx.output_format.is_json()
+        || !(visibility.is_public()
+            || (cx.render_options.document_private && is_visible_from_parent_mod))
         || pub_underscore
         || attrs.iter().any(|a| {
             a.has_name(sym::doc)

src/librustdoc/core.rs

@@ -81,6 +81,8 @@ pub(crate) struct DocContext<'tcx> {
     pub(crate) inlined: FxHashSet<ItemId>,
     /// Used by `calculate_doc_coverage`.
     pub(crate) output_format: OutputFormat,
+    /// Used by `strip_private`.
+    pub(crate) show_coverage: bool,
 }
 
 impl<'tcx> DocContext<'tcx> {
@@ -381,6 +383,7 @@ pub(crate) fn run_global_ctxt(
         inlined: FxHashSet::default(),
         output_format,
         render_options,
+        show_coverage,
     };
 
     // Small hack to force the Sized trait to be present.

src/librustdoc/json/conversions.rs

@@ -43,7 +43,16 @@ impl JsonRenderer<'_> {
         let span = item.span(self.tcx);
         let clean::Item { name, attrs: _, kind: _, visibility, item_id, cfg: _ } = item;
         let inner = match *item.kind {
-            clean::StrippedItem(_) | clean::KeywordItem(_) => return None,
+            clean::KeywordItem(_) => return None,
+            clean::StrippedItem(ref inner) => {
+                match &**inner {
+                    // We document non-empty stripped modules as with `Module::is_stripped` set to
+                    // `true`, to prevent contained items from being orphaned for downstream users,
+                    // as JSON does no inlining.
+                    clean::ModuleItem(m) if !m.items.is_empty() => from_clean_item(item, self.tcx),
+                    _ => return None,
+                }
+            }
             _ => from_clean_item(item, self.tcx),
         };
         Some(Item {
@@ -220,7 +229,9 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
     let header = item.fn_header(tcx);
 
     match *item.kind {
-        ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items, tcx) }),
+        ModuleItem(m) => {
+            ItemEnum::Module(Module { is_crate, items: ids(m.items, tcx), is_stripped: false })
+        }
         ImportItem(i) => ItemEnum::Import(i.into_tcx(tcx)),
         StructItem(s) => ItemEnum::Struct(s.into_tcx(tcx)),
         UnionItem(u) => ItemEnum::Union(u.into_tcx(tcx)),
@@ -257,8 +268,19 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
             bounds: b.into_iter().map(|x| x.into_tcx(tcx)).collect(),
             default: Some(t.item_type.unwrap_or(t.type_).into_tcx(tcx)),
         },
-        // `convert_item` early returns `None` for striped items and keywords.
-        StrippedItem(_) | KeywordItem(_) => unreachable!(),
+        // `convert_item` early returns `None` for stripped items and keywords.
+        KeywordItem(_) => unreachable!(),
+        StrippedItem(inner) => {
+            match *inner {
+                ModuleItem(m) => ItemEnum::Module(Module {
+                    is_crate,
+                    items: ids(m.items, tcx),
+                    is_stripped: true,
+                }),
+                // `convert_item` early returns `None` for stripped items we're not including
+                _ => unreachable!(),
+            }
+        }
         ExternCrateItem { ref src } => ItemEnum::ExternCrate {
             name: name.as_ref().unwrap().to_string(),
             rename: src.map(|x| x.to_string()),

src/librustdoc/json/mod.rs

@@ -21,6 +21,7 @@ use rustc_span::def_id::LOCAL_CRATE;
 use rustdoc_json_types as types;
 
 use crate::clean::types::{ExternalCrate, ExternalLocation};
+use crate::clean::ItemKind;
 use crate::config::RenderOptions;
 use crate::docfs::PathError;
 use crate::error::Error;
@@ -175,6 +176,14 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
     /// the hashmap because certain items (traits and types) need to have their mappings for trait
     /// implementations filled out before they're inserted.
     fn item(&mut self, item: clean::Item) -> Result<(), Error> {
+        trace!("rendering {} {:?}", item.type_(), item.name);
+
+        // Flatten items that recursively store other items. We include orphaned items from
+        // stripped modules and etc that are otherwise reachable.
+        if let ItemKind::StrippedItem(inner) = &*item.kind {
+            inner.inner_items().for_each(|i| self.item(i.clone()).unwrap());
+        }
+
         // Flatten items that recursively store other items
         item.kind.inner_items().for_each(|i| self.item(i.clone()).unwrap());
 

src/librustdoc/passes/strip_private.rs

@@ -24,6 +24,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) ->
             retained: &mut retained,
             access_levels: &cx.cache.access_levels,
             update_retained: true,
+            is_json_output: cx.output_format.is_json() && !cx.show_coverage,
         };
         krate = ImportStripper.fold_crate(stripper.fold_crate(krate));
     }

src/librustdoc/passes/stripper.rs

@@ -3,14 +3,29 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::middle::privacy::AccessLevels;
 use std::mem;
 
-use crate::clean::{self, Item, ItemIdSet};
+use crate::clean::{self, Item, ItemId, ItemIdSet};
 use crate::fold::{strip_item, DocFolder};
 use crate::formats::cache::Cache;
 
 pub(crate) struct Stripper<'a> {
     pub(crate) retained: &'a mut ItemIdSet,
     pub(crate) access_levels: &'a AccessLevels<DefId>,
     pub(crate) update_retained: bool,
+    pub(crate) is_json_output: bool,
+}
+
+impl<'a> Stripper<'a> {
+    // We need to handle this differently for the JSON output because some non exported items could
+    // be used in public API. And so, we need these items as well. `is_exported` only checks if they
+    // are in the public API, which is not enough.
+    #[inline]
+    fn is_item_reachable(&self, item_id: ItemId) -> bool {
+        if self.is_json_output {
+            self.access_levels.is_reachable(item_id.expect_def_id())
+        } else {
+            self.access_levels.is_exported(item_id.expect_def_id())
+        }
+    }
 }
 
 impl<'a> DocFolder for Stripper<'a> {
@@ -45,9 +60,8 @@ impl<'a> DocFolder for Stripper<'a> {
             | clean::TraitAliasItem(..)
             | clean::MacroItem(..)
             | clean::ForeignTypeItem => {
-                if i.item_id.is_local()
-                    && !self.access_levels.is_exported(i.item_id.expect_def_id())
-                {
+                let item_id = i.item_id;
+                if item_id.is_local() && !self.is_item_reachable(item_id) {
                     debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name);
                     return None;
                 }

src/librustdoc/visit_ast.rs

@@ -190,6 +190,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
     ) -> bool {
         debug!("maybe_inline_local res: {:?}", res);
 
+        if self.cx.output_format.is_json() {
+            return false;
+        }
+
         let tcx = self.cx.tcx;
         let Some(res_did) = res.opt_def_id() else {
             return false;

src/rustdoc-json-types/lib.rs

@@ -9,7 +9,7 @@ use std::path::PathBuf;
 use serde::{Deserialize, Serialize};
 
 /// rustdoc format-version.
-pub const FORMAT_VERSION: u32 = 15;
+pub const FORMAT_VERSION: u32 = 16;
 
 /// A `Crate` is the root of the emitted JSON blob. It contains all type/documentation information
 /// about the language items in the local crate, as well as info about external items to allow
@@ -245,6 +245,9 @@ pub enum ItemEnum {
 pub struct Module {
     pub is_crate: bool,
     pub items: Vec<Id>,
+    /// If `true`, this module is not part of the public API, but it contains
+    /// items that are re-exported as public API.
+    pub is_stripped: bool,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]

src/test/rustdoc-json/doc_hidden_failure.rs

@@ -0,0 +1,22 @@
+// Regression test for <https://github.com/rust-lang/rust/issues/98007>.
+
+#![feature(no_core)]
+#![no_core]
+
+mod auto {
+    mod action_row {
+        pub struct ActionRowBuilder;
+    }
+
+    #[doc(hidden)]
+    pub mod builders {
+        pub use super::action_row::ActionRowBuilder;
+    }
+}
+
+// @count doc_hidden_failure.json "$.index[*][?(@.name=='builders')]" 2
+pub use auto::*;
+
+pub mod builders {
+    pub use crate::auto::builders::*;
+}

src/test/rustdoc-json/reexport/auxiliary/pub-struct.rs

@@ -0,0 +1 @@
+pub struct Foo;

src/test/rustdoc-json/reexport/glob_extern.rs

@@ -3,15 +3,16 @@
 #![no_core]
 #![feature(no_core)]
 
-// @!has glob_extern.json "$.index[*][?(@.name=='mod1')]"
+// @is glob_extern.json "$.index[*][?(@.name=='mod1')].kind" \"module\"
+// @is glob_extern.json "$.index[*][?(@.name=='mod1')].inner.is_stripped" "true"
 mod mod1 {
     extern "C" {
-        // @set public_fn_id = - "$.index[*][?(@.name=='public_fn')].id"
+        // @has - "$.index[*][?(@.name=='public_fn')].id"
         pub fn public_fn();
         // @!has - "$.index[*][?(@.name=='private_fn')]"
         fn private_fn();
     }
 }
 
-// @has - "$.index[*][?(@.name=='glob_extern')].inner.items[*]" $public_fn_id
+// @is - "$.index[*][?(@.kind=='import')].inner.glob" true
 pub use mod1::*;

src/test/rustdoc-json/reexport/glob_private.rs

@@ -3,25 +3,30 @@
 #![no_core]
 #![feature(no_core)]
 
-// @!has glob_private.json "$.index[*][?(@.name=='mod1')]"
+// @is glob_private.json "$.index[*][?(@.name=='mod1')].kind" \"module\"
+// @is glob_private.json "$.index[*][?(@.name=='mod1')].inner.is_stripped" "true"
 mod mod1 {
-    // @!has - "$.index[*][?(@.name=='mod2')]"
+    // @is - "$.index[*][?(@.name=='mod2')].kind" \"module\"
+    // @is - "$.index[*][?(@.name=='mod2')].inner.is_stripped" "true"
     mod mod2 {
         // @set m2pub_id = - "$.index[*][?(@.name=='Mod2Public')].id"
         pub struct Mod2Public;
 
         // @!has - "$.index[*][?(@.name=='Mod2Private')]"
         struct Mod2Private;
     }
+
+    // @has - "$.index[*][?(@.kind=='import' && @.inner.name=='mod2')]"
     pub use self::mod2::*;
 
     // @set m1pub_id = - "$.index[*][?(@.name=='Mod1Public')].id"
     pub struct Mod1Public;
-
     // @!has - "$.index[*][?(@.name=='Mod1Private')]"
     struct Mod1Private;
 }
+
+// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='mod1')]"
 pub use mod1::*;
 
-// @has - "$.index[*][?(@.name=='glob_private')].inner.items[*]" $m2pub_id
-// @has - "$.index[*][?(@.name=='glob_private')].inner.items[*]" $m1pub_id
+// @has - "$.index[*][?(@.name=='mod2')].inner.items[*]" $m2pub_id
+// @has - "$.index[*][?(@.name=='mod1')].inner.items[*]" $m1pub_id

src/test/rustdoc-json/reexport/in_root_and_mod.rs

@@ -1,15 +1,17 @@
 #![feature(no_core)]
 #![no_core]
 
+// @is in_root_and_mod.json "$.index[*][?(@.name=='foo')].kind" \"module\"
+// @is in_root_and_mod.json "$.index[*][?(@.name=='foo')].inner.is_stripped" "true"
 mod foo {
-    // @set foo_id = in_root_and_mod.json "$.index[*][?(@.name=='Foo')].id"
+    // @has - "$.index[*][?(@.name=='Foo')]"
     pub struct Foo;
 }
 
-// @has - "$.index[*][?(@.name=='in_root_and_mod')].inner.items[*]" $foo_id
+// @has - "$.index[*][?(@.kind=='import' && @.inner.source=='foo::Foo')]"
 pub use foo::Foo;
 
 pub mod bar {
-    // @has - "$.index[*][?(@.name=='bar')].inner.items[*]" $foo_id
+    // @has - "$.index[*][?(@.kind=='import' && @.inner.source=='crate::foo::Foo')]"
     pub use crate::foo::Foo;
 }

src/test/rustdoc-json/reexport/private_twice_one_inline.rs

@@ -0,0 +1,18 @@
+// aux-build:pub-struct.rs
+
+// Test for the ICE in rust/83057
+// Am external type re-exported with different attributes shouldn't cause an error
+
+#![no_core]
+#![feature(no_core)]
+
+extern crate pub_struct as foo;
+
+#[doc(inline)]
+pub use foo::Foo;
+
+pub mod bar {
+    pub use foo::Foo;
+}
+
+// @count private_twice_one_inline.json "$.index[*][?(@.kind=='import')]" 2

src/test/rustdoc-json/reexport/private_two_names.rs

@@ -0,0 +1,17 @@
+// Test for the ICE in rust/83720
+// A pub-in-private type re-exported under two different names shouldn't cause an error
+
+#![no_core]
+#![feature(no_core)]
+
+// @is private_two_names.json "$.index[*][?(@.name=='style')].kind" \"module\"
+// @is private_two_names.json "$.index[*][?(@.name=='style')].inner.is_stripped" "true"
+mod style {
+    // @has - "$.index[*](?(@.name=='Color'))"
+    pub struct Color;
+}
+
+// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='Color')]"
+pub use style::Color;
+// @has - "$.index[*][?(@.kind=='import' && @.inner.name=='Colour')]"
+pub use style::Color as Colour;

src/test/rustdoc-json/reexport/rename_private.rs

@@ -2,13 +2,13 @@
 
 #![no_core]
 #![feature(no_core)]
-// @!has rename_private.json "$.index[*][?(@.name=='inner')]"
+
+// @is rename_private.json "$.index[*][?(@.name=='inner')].kind" \"module\"
+// @is rename_private.json "$.index[*][?(@.name=='inner')].inner.is_stripped" "true"
 mod inner {
-    // @!has - "$.index[*][?(@.name=='Public')]"
+    // @has - "$.index[*][?(@.name=='Public')]"
     pub struct Public;
 }
 
-// @set newname_id = - "$.index[*][?(@.name=='NewName')].id"
-// @is - "$.index[*][?(@.name=='NewName')].kind" \"struct\"
-// @has - "$.index[*][?(@.name=='rename_private')].inner.items[*]" $newname_id
+// @is - "$.index[*][?(@.kind=='import')].inner.name" \"NewName\"
 pub use inner::Public as NewName;

src/test/rustdoc-json/reexport/same_type_reexported_more_than_once.rs

@@ -1,15 +1,13 @@
-// Regression test for https://github.com/rust-lang/rust/issues/97432.
+// Regression test for <https://github.com/rust-lang/rust/issues/97432>.
 
 #![feature(no_core)]
 #![no_std]
 #![no_core]
 
 // @has same_type_reexported_more_than_once.json
-// @set trait_id = - "$.index[*][?(@.name=='Trait')].id"
-// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $trait_id
+// @has - "$.index[*][?(@.name=='Trait')]"
 pub use inner::Trait;
-// @set reexport_id = - "$.index[*][?(@.name=='Reexport')].id"
-// @has - "$.index[*][?(@.name=='same_type_reexported_more_than_once')].inner.items[*]" $reexport_id
+// @has - "$.index[*].inner[?(@.name=='Reexport')].id"
 pub use inner::Trait as Reexport;
 
 mod inner {

src/test/rustdoc-json/reexport/simple_private.rs

@@ -1,13 +1,15 @@
 // edition:2018
-
 #![no_core]
 #![feature(no_core)]
 
-// @!has simple_private.json "$.index[*][?(@.name=='inner')]"
+// @is simple_private.json "$.index[*][?(@.name=='inner')].kind" \"module\"
+// @is simple_private.json "$.index[*][?(@.name=='inner')].inner.is_stripped" "true"
 mod inner {
     // @set pub_id = - "$.index[*][?(@.name=='Public')].id"
     pub struct Public;
 }
 
-// @has - "$.index[*][?(@.name=='simple_private')].inner.items[*]" $pub_id
+// @is - "$.index[*][?(@.kind=='import')].inner.name" \"Public\"
 pub use inner::Public;
+
+// @has - "$.index[*][?(@.name=='inner')].inner.items[*]" $pub_id

src/test/rustdoc-json/return_private.rs

@@ -0,0 +1,15 @@
+// Regression test for <https://github.com/rust-lang/rust/issues/96161>.
+// ignore-tidy-linelength
+
+#![feature(no_core)]
+#![no_core]
+
+mod secret {
+    pub struct Secret;
+}
+
+// @is return_private.json "$.index[*][?(@.name=='get_secret')].kind" \"function\"
+// @is return_private.json "$.index[*][?(@.name=='get_secret')].inner.decl.output.inner.name" \"secret::Secret\"
+pub fn get_secret() -> secret::Secret {
+    secret::Secret
+}