Auto merge of rust-lang#91094 - inquisitivecrystal:rustdoc-top-mod, r…

…=jyn514

Avoid documenting top-level private imports

PR rust-lang#88447 aimed to make rustdoc's `--document-private-items` mode only document imports that are visible outside the importing module. Unfortunately, I inadvertently set things up so that imports at the crate top-level are always documented, regardless of their visibility. This behavior was unintended and is [not desirable](rust-lang#90865 (comment)).

This PR treats top-level imports as never being visible outside their parent module. In practice, the only way a top-level import can be visible externally is if it's fully public, and there's a seperate check for that.

It's worth calling attention to the fact that this change means that `pub(crate)` imports will be visible in lower level modules, but not at the top-level. This is because, at the top level of the crate, `pub(crate)` means the same thing as `pub(self)`.

It turned out that there were existing tests checking for the only behavior, which I didn't notice at the time of my previous PR. I have updated them to check for the new behavior and substantially extended them to handle differences between the top-level module and lower level modules. I may have gone overboard, so please tell me if there's anything I should cut.

r? `@jyn514`

Fixes rust-lang#90865.

src/librustdoc/clean/mod.rs

@@ -1919,8 +1919,20 @@ fn clean_use_statement(
     let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline);
     let pub_underscore = visibility.is_public() && name == kw::Underscore;
     let current_mod = cx.tcx.parent_module_from_def_id(import.def_id);
+
+    // The parent of the module in which this import resides. This
+    // is the same as `current_mod` if that's already the top
+    // level module.
     let parent_mod = cx.tcx.parent_module_from_def_id(current_mod);
 
+    // This checks if the import can be seen from a higher level module.
+    // In other words, it checks if the visibility is the equivalent of
+    // `pub(super)` or higher. If the current module is the top level
+    // module, there isn't really a parent module, which makes the results
+    // meaningless. In this case, we make sure the answer is `false`.
+    let is_visible_from_parent_mod = visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx)
+        && !current_mod.is_top_level_module();
+
     if pub_underscore {
         if let Some(ref inline) = inline_attr {
             rustc_errors::struct_span_err!(
@@ -1939,8 +1951,7 @@ fn clean_use_statement(
     // #[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
-            && visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx)))
+        || (cx.render_options.document_private && is_visible_from_parent_mod))
         || pub_underscore
         || attrs.iter().any(|a| {
             a.has_name(sym::doc)

src/test/rustdoc/auxiliary/reexports.rs

@@ -4,39 +4,63 @@ pub macro addr_of($place:expr) {
     &raw const $place
 }
 
+pub macro addr_of_crate($place:expr) {
+    &raw const $place
+}
+
+pub macro addr_of_super($place:expr) {
+    &raw const $place
+}
+
 pub macro addr_of_self($place:expr) {
     &raw const $place
 }
 
-pub macro addr_of_crate($place:expr) {
+pub macro addr_of_local($place:expr) {
     &raw const $place
 }
 
 pub struct Foo;
-pub struct FooSelf;
 pub struct FooCrate;
+pub struct FooSuper;
+pub struct FooSelf;
+pub struct FooLocal;
 
 pub enum Bar { Foo, }
-pub enum BarSelf { Foo, }
 pub enum BarCrate { Foo, }
+pub enum BarSuper { Foo, }
+pub enum BarSelf { Foo, }
+pub enum BarLocal { Foo, }
 
 pub fn foo() {}
-pub fn foo_self() {}
 pub fn foo_crate() {}
+pub fn foo_super() {}
+pub fn foo_self() {}
+pub fn foo_local() {}
 
 pub type Type = i32;
-pub type TypeSelf = i32;
 pub type TypeCrate = i32;
+pub type TypeSuper = i32;
+pub type TypeSelf = i32;
+pub type TypeLocal = i32;
 
 pub union Union {
     a: i8,
     b: i8,
 }
+pub union UnionCrate {
+    a: i8,
+    b: i8,
+}
+pub union UnionSuper {
+    a: i8,
+    b: i8,
+}
 pub union UnionSelf {
     a: i8,
     b: i8,
 }
-pub union UnionCrate {
+pub union UnionLocal {
     a: i8,
     b: i8,
 }

src/test/rustdoc/reexports-priv.rs

@@ -7,47 +7,129 @@ extern crate reexports;
 
 // @has 'foo/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
 pub use reexports::addr_of;
-// @has 'foo/macro.addr_of_crate.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_crate($place : expr) {'
+// @!has 'foo/macro.addr_of_crate.html'
 pub(crate) use reexports::addr_of_crate;
-// @has 'foo/macro.addr_of_self.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_self($place : expr) {'
+// @!has 'foo/macro.addr_of_self.html'
 pub(self) use reexports::addr_of_self;
+// @!has 'foo/macro.addr_of_local.html'
+use reexports::addr_of_local;
 
 // @has 'foo/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
 pub use reexports::Foo;
-// @has 'foo/struct.FooCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooCrate;'
+// @!has 'foo/struct.FooCrate.html'
 pub(crate) use reexports::FooCrate;
-// @has 'foo/struct.FooSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooSelf;'
+// @!has 'foo/struct.FooSelf.html'
 pub(self) use reexports::FooSelf;
+// @!has 'foo/struct.FooLocal.html'
+use reexports::FooLocal;
 
 // @has 'foo/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
 pub use reexports::Bar;
-// @has 'foo/enum.BarCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarCrate {'
+// @!has 'foo/enum.BarCrate.html'
 pub(crate) use reexports::BarCrate;
-// @has 'foo/enum.BarSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarSelf {'
+// @!has 'foo/enum.BarSelf.html'
 pub(self) use reexports::BarSelf;
+// @!has 'foo/enum.BarLocal.html'
+use reexports::BarLocal;
 
 // @has 'foo/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
 pub use reexports::foo;
-// @has 'foo/fn.foo_crate.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_crate()'
+// @!has 'foo/fn.foo_crate.html'
 pub(crate) use reexports::foo_crate;
-// @has 'foo/fn.foo_self.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_self()'
+// @!has 'foo/fn.foo_self.html'
 pub(self) use reexports::foo_self;
+// @!has 'foo/fn.foo_local.html'
+use reexports::foo_local;
 
 // @has 'foo/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
 pub use reexports::Type;
-// @has 'foo/type.TypeCrate.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeCrate ='
+// @!has 'foo/type.TypeCrate.html'
 pub(crate) use reexports::TypeCrate;
-// @has 'foo/type.TypeSelf.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeSelf ='
+// @!has 'foo/type.TypeSelf.html'
 pub(self) use reexports::TypeSelf;
+// @!has 'foo/type.TypeLocal.html'
+use reexports::TypeLocal;
 
 // @has 'foo/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
 pub use reexports::Union;
-// @has 'foo/union.UnionCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionCrate {'
+// @!has 'foo/union.UnionCrate.html'
 pub(crate) use reexports::UnionCrate;
-// @has 'foo/union.UnionSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionSelf {'
+// @!has 'foo/union.UnionSelf.html'
 pub(self) use reexports::UnionSelf;
+// @!has 'foo/union.UnionLocal.html'
+use reexports::UnionLocal;
 
-pub mod foo {
-    // @!has 'foo/foo/union.Union.html'
-    use crate::reexports::Union;
+pub mod outer {
+    pub mod inner {
+        // @has 'foo/outer/inner/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
+        pub use reexports::addr_of;
+        // @has 'foo/outer/inner/macro.addr_of_crate.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_crate($place : expr) {'
+        pub(crate) use reexports::addr_of_crate;
+        // @has 'foo/outer/inner/macro.addr_of_super.html' '//*[@class="docblock item-decl"]' 'pub(in outer) macro addr_of_super($place : expr) {'
+        pub(super) use reexports::addr_of_super;
+        // @!has 'foo/outer/inner/macro.addr_of_self.html'
+        pub(self) use reexports::addr_of_self;
+        // @!has 'foo/outer/inner/macro.addr_of_local.html'
+        use reexports::addr_of_local;
+
+        // @has 'foo/outer/inner/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
+        pub use reexports::Foo;
+        // @has 'foo/outer/inner/struct.FooCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooCrate;'
+        pub(crate) use reexports::FooCrate;
+        // @has 'foo/outer/inner/struct.FooSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) struct FooSuper;'
+        pub(super) use reexports::FooSuper;
+        // @!has 'foo/outer/inner/struct.FooSelf.html'
+        pub(self) use reexports::FooSelf;
+        // @!has 'foo/outer/inner/struct.FooLocal.html'
+        use reexports::FooLocal;
+
+        // @has 'foo/outer/inner/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
+        pub use reexports::Bar;
+        // @has 'foo/outer/inner/enum.BarCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarCrate {'
+        pub(crate) use reexports::BarCrate;
+        // @has 'foo/outer/inner/enum.BarSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) enum BarSuper {'
+        pub(super) use reexports::BarSuper;
+        // @!has 'foo/outer/inner/enum.BarSelf.html'
+        pub(self) use reexports::BarSelf;
+        // @!has 'foo/outer/inner/enum.BarLocal.html'
+        use reexports::BarLocal;
+
+        // @has 'foo/outer/inner/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
+        pub use reexports::foo;
+        // @has 'foo/outer/inner/fn.foo_crate.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_crate()'
+        pub(crate) use reexports::foo_crate;
+        // @has 'foo/outer/inner/fn.foo_super.html' '//*[@class="rust fn"]' 'pub(in outer) fn foo_super()'
+        pub(super) use::reexports::foo_super;
+        // @!has 'foo/outer/inner/fn.foo_self.html'
+        pub(self) use reexports::foo_self;
+        // @!has 'foo/outer/inner/fn.foo_local.html'
+        use reexports::foo_local;
+
+        // @has 'foo/outer/inner/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
+        pub use reexports::Type;
+        // @has 'foo/outer/inner/type.TypeCrate.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeCrate ='
+        pub(crate) use reexports::TypeCrate;
+        // @has 'foo/outer/inner/type.TypeSuper.html' '//*[@class="rust typedef"]' 'pub(in outer) type TypeSuper ='
+        pub(super) use reexports::TypeSuper;
+        // @!has 'foo/outer/inner/type.TypeSelf.html'
+        pub(self) use reexports::TypeSelf;
+        // @!has 'foo/outer/inner/type.TypeLocal.html'
+        use reexports::TypeLocal;
+
+        // @has 'foo/outer/inner/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
+        pub use reexports::Union;
+        // @has 'foo/outer/inner/union.UnionCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionCrate {'
+        pub(crate) use reexports::UnionCrate;
+        // @has 'foo/outer/inner/union.UnionSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) union UnionSuper {'
+        pub(super) use reexports::UnionSuper;
+        // @!has 'foo/outer/inner/union.UnionSelf.html'
+        pub(self) use reexports::UnionSelf;
+        // @!has 'foo/outer/inner/union.UnionLocal.html'
+        use reexports::UnionLocal;
+    }
+}
+
+mod re_re_exports {
+        // @!has 'foo/re_re_exports/union.Union.html'
+        use crate::reexports::Union;
 }

src/test/rustdoc/reexports.rs

@@ -10,38 +10,120 @@ pub use reexports::addr_of;
 pub(crate) use reexports::addr_of_crate;
 // @!has 'foo/macro.addr_of_self.html'
 pub(self) use reexports::addr_of_self;
+// @!has 'foo/macro.addr_of_local.html'
+use reexports::addr_of_local;
 
 // @has 'foo/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
 pub use reexports::Foo;
 // @!has 'foo/struct.FooCrate.html'
 pub(crate) use reexports::FooCrate;
 // @!has 'foo/struct.FooSelf.html'
 pub(self) use reexports::FooSelf;
+// @!has 'foo/struct.FooLocal.html'
+use reexports::FooLocal;
 
 // @has 'foo/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
 pub use reexports::Bar;
 // @!has 'foo/enum.BarCrate.html'
 pub(crate) use reexports::BarCrate;
 // @!has 'foo/enum.BarSelf.html'
 pub(self) use reexports::BarSelf;
+// @!has 'foo/enum.BarLocal.html'
+use reexports::BarLocal;
 
 // @has 'foo/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
 pub use reexports::foo;
 // @!has 'foo/fn.foo_crate.html'
 pub(crate) use reexports::foo_crate;
 // @!has 'foo/fn.foo_self.html'
 pub(self) use reexports::foo_self;
+// @!has 'foo/fn.foo_local.html'
+use reexports::foo_local;
 
 // @has 'foo/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
 pub use reexports::Type;
 // @!has 'foo/type.TypeCrate.html'
 pub(crate) use reexports::TypeCrate;
 // @!has 'foo/type.TypeSelf.html'
 pub(self) use reexports::TypeSelf;
+// @!has 'foo/type.TypeLocal.html'
+use reexports::TypeLocal;
 
 // @has 'foo/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
 pub use reexports::Union;
 // @!has 'foo/union.UnionCrate.html'
 pub(crate) use reexports::UnionCrate;
 // @!has 'foo/union.UnionSelf.html'
 pub(self) use reexports::UnionSelf;
+// @!has 'foo/union.UnionLocal.html'
+use reexports::UnionLocal;
+
+pub mod outer {
+    pub mod inner {
+        // @has 'foo/outer/inner/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
+        pub use reexports::addr_of;
+        // @!has 'foo/outer/inner/macro.addr_of_crate.html'
+        pub(crate) use reexports::addr_of_crate;
+        // @!has 'foo/outer/inner/macro.addr_of_super.html'
+        pub(super) use reexports::addr_of_super;
+        // @!has 'foo/outer/inner/macro.addr_of_self.html'
+        pub(self) use reexports::addr_of_self;
+        // @!has 'foo/outer/inner/macro.addr_of_local.html'
+        use reexports::addr_of_local;
+
+        // @has 'foo/outer/inner/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
+        pub use reexports::Foo;
+        // @!has 'foo/outer/inner/struct.FooCrate.html'
+        pub(crate) use reexports::FooCrate;
+        // @!has 'foo/outer/inner/struct.FooSuper.html'
+        pub(super) use reexports::FooSuper;
+        // @!has 'foo/outer/inner/struct.FooSelf.html'
+        pub(self) use reexports::FooSelf;
+        // @!has 'foo/outer/inner/struct.FooLocal.html'
+        use reexports::FooLocal;
+
+        // @has 'foo/outer/inner/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
+        pub use reexports::Bar;
+        // @!has 'foo/outer/inner/enum.BarCrate.html'
+        pub(crate) use reexports::BarCrate;
+        // @!has 'foo/outer/inner/enum.BarSuper.html'
+        pub(super) use reexports::BarSuper;
+        // @!has 'foo/outer/inner/enum.BarSelf.html'
+        pub(self) use reexports::BarSelf;
+        // @!has 'foo/outer/inner/enum.BarLocal.html'
+        use reexports::BarLocal;
+
+        // @has 'foo/outer/inner/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
+        pub use reexports::foo;
+        // @!has 'foo/outer/inner/fn.foo_crate.html'
+        pub(crate) use reexports::foo_crate;
+        // @!has 'foo/outer/inner/fn.foo_super.html'
+        pub(super) use::reexports::foo_super;
+        // @!has 'foo/outer/inner/fn.foo_self.html'
+        pub(self) use reexports::foo_self;
+        // @!has 'foo/outer/inner/fn.foo_local.html'
+        use reexports::foo_local;
+
+        // @has 'foo/outer/inner/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
+        pub use reexports::Type;
+        // @!has 'foo/outer/inner/type.TypeCrate.html'
+        pub(crate) use reexports::TypeCrate;
+        // @!has 'foo/outer/inner/type.TypeSuper.html'
+        pub(super) use reexports::TypeSuper;
+        // @!has 'foo/outer/inner/type.TypeSelf.html'
+        pub(self) use reexports::TypeSelf;
+        // @!has 'foo/outer/inner/type.TypeLocal.html'
+        use reexports::TypeLocal;
+
+        // @has 'foo/outer/inner/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
+        pub use reexports::Union;
+        // @!has 'foo/outer/inner/union.UnionCrate.html'
+        pub(crate) use reexports::UnionCrate;
+        // @!has 'foo/outer/inner/union.UnionSuper.html'
+        pub(super) use reexports::UnionSuper;
+        // @!has 'foo/outer/inner/union.UnionSelf.html'
+        pub(self) use reexports::UnionSelf;
+        // @!has 'foo/outer/inner/union.UnionLocal.html'
+        use reexports::UnionLocal;
+    }
+}