Skip to content

Commit 2e055d9

Browse files
borsbors
committed
Auto merge of #91094 - inquisitivecrystal:rustdoc-top-mod, r=jyn514
Avoid documenting top-level private imports PR #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](#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 #90865.
2 parents 8d0c79d + 3c51038 commit 2e055d9

File tree

4 files changed

+222
-23
lines changed

4 files changed

+222
-23
lines changed

‎src/librustdoc/clean/mod.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,8 +1919,20 @@ fn clean_use_statement(
19191919
let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline);
19201920
let pub_underscore = visibility.is_public() && name == kw::Underscore;
19211921
let current_mod = cx.tcx.parent_module_from_def_id(import.def_id);
1922+
1923+
// The parent of the module in which this import resides. This
1924+
// is the same as `current_mod` if that's already the top
1925+
// level module.
19221926
let parent_mod = cx.tcx.parent_module_from_def_id(current_mod);
19231927

1928+
// This checks if the import can be seen from a higher level module.
1929+
// In other words, it checks if the visibility is the equivalent of
1930+
// `pub(super)` or higher. If the current module is the top level
1931+
// module, there isn't really a parent module, which makes the results
1932+
// meaningless. In this case, we make sure the answer is `false`.
1933+
let is_visible_from_parent_mod = visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx)
1934+
&& !current_mod.is_top_level_module();
1935+
19241936
if pub_underscore {
19251937
if let Some(ref inline) = inline_attr {
19261938
rustc_errors::struct_span_err!(
@@ -1939,8 +1951,7 @@ fn clean_use_statement(
19391951
// #[doc(no_inline)] attribute is present.
19401952
// Don't inline doc(hidden) imports so they can be stripped at a later stage.
19411953
let mut denied = !(visibility.is_public()
1942-
|| (cx.render_options.document_private
1943-
&& visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx)))
1954+
|| (cx.render_options.document_private && is_visible_from_parent_mod))
19441955
|| pub_underscore
19451956
|| attrs.iter().any(|a| {
19461957
a.has_name(sym::doc)

‎src/test/rustdoc/auxiliary/reexports.rs‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,39 +4,63 @@ pub macro addr_of($place:expr) {
44
&raw const $place
55
}
66

7+
pub macro addr_of_crate($place:expr) {
8+
&raw const $place
9+
}
10+
11+
pub macro addr_of_super($place:expr) {
12+
&raw const $place
13+
}
14+
715
pub macro addr_of_self($place:expr) {
816
&raw const $place
917
}
1018

11-
pub macro addr_of_crate($place:expr) {
19+
pub macro addr_of_local($place:expr) {
1220
&raw const $place
1321
}
1422

1523
pub struct Foo;
16-
pub struct FooSelf;
1724
pub struct FooCrate;
25+
pub struct FooSuper;
26+
pub struct FooSelf;
27+
pub struct FooLocal;
1828

1929
pub enum Bar { Foo, }
20-
pub enum BarSelf { Foo, }
2130
pub enum BarCrate { Foo, }
31+
pub enum BarSuper { Foo, }
32+
pub enum BarSelf { Foo, }
33+
pub enum BarLocal { Foo, }
2234

2335
pub fn foo() {}
24-
pub fn foo_self() {}
2536
pub fn foo_crate() {}
37+
pub fn foo_super() {}
38+
pub fn foo_self() {}
39+
pub fn foo_local() {}
2640

2741
pub type Type = i32;
28-
pub type TypeSelf = i32;
2942
pub type TypeCrate = i32;
43+
pub type TypeSuper = i32;
44+
pub type TypeSelf = i32;
45+
pub type TypeLocal = i32;
3046

3147
pub union Union {
3248
a: i8,
3349
b: i8,
3450
}
51+
pub union UnionCrate {
52+
a: i8,
53+
b: i8,
54+
}
55+
pub union UnionSuper {
56+
a: i8,
57+
b: i8,
58+
}
3559
pub union UnionSelf {
3660
a: i8,
3761
b: i8,
3862
}
39-
pub union UnionCrate {
63+
pub union UnionLocal {
4064
a: i8,
4165
b: i8,
4266
}

‎src/test/rustdoc/reexports-priv.rs‎

Lines changed: 97 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,47 +7,129 @@ extern crate reexports;
77

88
// @has 'foo/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
99
pub use reexports::addr_of;
10-
// @has 'foo/macro.addr_of_crate.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_crate($place : expr) {'
10+
// @!has 'foo/macro.addr_of_crate.html'
1111
pub(crate) use reexports::addr_of_crate;
12-
// @has 'foo/macro.addr_of_self.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_self($place : expr) {'
12+
// @!has 'foo/macro.addr_of_self.html'
1313
pub(self) use reexports::addr_of_self;
14+
// @!has 'foo/macro.addr_of_local.html'
15+
use reexports::addr_of_local;
1416

1517
// @has 'foo/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
1618
pub use reexports::Foo;
17-
// @has 'foo/struct.FooCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooCrate;'
19+
// @!has 'foo/struct.FooCrate.html'
1820
pub(crate) use reexports::FooCrate;
19-
// @has 'foo/struct.FooSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooSelf;'
21+
// @!has 'foo/struct.FooSelf.html'
2022
pub(self) use reexports::FooSelf;
23+
// @!has 'foo/struct.FooLocal.html'
24+
use reexports::FooLocal;
2125

2226
// @has 'foo/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
2327
pub use reexports::Bar;
24-
// @has 'foo/enum.BarCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarCrate {'
28+
// @!has 'foo/enum.BarCrate.html'
2529
pub(crate) use reexports::BarCrate;
26-
// @has 'foo/enum.BarSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarSelf {'
30+
// @!has 'foo/enum.BarSelf.html'
2731
pub(self) use reexports::BarSelf;
32+
// @!has 'foo/enum.BarLocal.html'
33+
use reexports::BarLocal;
2834

2935
// @has 'foo/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
3036
pub use reexports::foo;
31-
// @has 'foo/fn.foo_crate.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_crate()'
37+
// @!has 'foo/fn.foo_crate.html'
3238
pub(crate) use reexports::foo_crate;
33-
// @has 'foo/fn.foo_self.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_self()'
39+
// @!has 'foo/fn.foo_self.html'
3440
pub(self) use reexports::foo_self;
41+
// @!has 'foo/fn.foo_local.html'
42+
use reexports::foo_local;
3543

3644
// @has 'foo/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
3745
pub use reexports::Type;
38-
// @has 'foo/type.TypeCrate.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeCrate ='
46+
// @!has 'foo/type.TypeCrate.html'
3947
pub(crate) use reexports::TypeCrate;
40-
// @has 'foo/type.TypeSelf.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeSelf ='
48+
// @!has 'foo/type.TypeSelf.html'
4149
pub(self) use reexports::TypeSelf;
50+
// @!has 'foo/type.TypeLocal.html'
51+
use reexports::TypeLocal;
4252

4353
// @has 'foo/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
4454
pub use reexports::Union;
45-
// @has 'foo/union.UnionCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionCrate {'
55+
// @!has 'foo/union.UnionCrate.html'
4656
pub(crate) use reexports::UnionCrate;
47-
// @has 'foo/union.UnionSelf.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionSelf {'
57+
// @!has 'foo/union.UnionSelf.html'
4858
pub(self) use reexports::UnionSelf;
59+
// @!has 'foo/union.UnionLocal.html'
60+
use reexports::UnionLocal;
4961

50-
pub mod foo {
51-
// @!has 'foo/foo/union.Union.html'
52-
use crate::reexports::Union;
62+
pub mod outer {
63+
pub mod inner {
64+
// @has 'foo/outer/inner/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
65+
pub use reexports::addr_of;
66+
// @has 'foo/outer/inner/macro.addr_of_crate.html' '//*[@class="docblock item-decl"]' 'pub(crate) macro addr_of_crate($place : expr) {'
67+
pub(crate) use reexports::addr_of_crate;
68+
// @has 'foo/outer/inner/macro.addr_of_super.html' '//*[@class="docblock item-decl"]' 'pub(in outer) macro addr_of_super($place : expr) {'
69+
pub(super) use reexports::addr_of_super;
70+
// @!has 'foo/outer/inner/macro.addr_of_self.html'
71+
pub(self) use reexports::addr_of_self;
72+
// @!has 'foo/outer/inner/macro.addr_of_local.html'
73+
use reexports::addr_of_local;
74+
75+
// @has 'foo/outer/inner/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
76+
pub use reexports::Foo;
77+
// @has 'foo/outer/inner/struct.FooCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) struct FooCrate;'
78+
pub(crate) use reexports::FooCrate;
79+
// @has 'foo/outer/inner/struct.FooSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) struct FooSuper;'
80+
pub(super) use reexports::FooSuper;
81+
// @!has 'foo/outer/inner/struct.FooSelf.html'
82+
pub(self) use reexports::FooSelf;
83+
// @!has 'foo/outer/inner/struct.FooLocal.html'
84+
use reexports::FooLocal;
85+
86+
// @has 'foo/outer/inner/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
87+
pub use reexports::Bar;
88+
// @has 'foo/outer/inner/enum.BarCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) enum BarCrate {'
89+
pub(crate) use reexports::BarCrate;
90+
// @has 'foo/outer/inner/enum.BarSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) enum BarSuper {'
91+
pub(super) use reexports::BarSuper;
92+
// @!has 'foo/outer/inner/enum.BarSelf.html'
93+
pub(self) use reexports::BarSelf;
94+
// @!has 'foo/outer/inner/enum.BarLocal.html'
95+
use reexports::BarLocal;
96+
97+
// @has 'foo/outer/inner/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
98+
pub use reexports::foo;
99+
// @has 'foo/outer/inner/fn.foo_crate.html' '//*[@class="rust fn"]' 'pub(crate) fn foo_crate()'
100+
pub(crate) use reexports::foo_crate;
101+
// @has 'foo/outer/inner/fn.foo_super.html' '//*[@class="rust fn"]' 'pub(in outer) fn foo_super()'
102+
pub(super) use::reexports::foo_super;
103+
// @!has 'foo/outer/inner/fn.foo_self.html'
104+
pub(self) use reexports::foo_self;
105+
// @!has 'foo/outer/inner/fn.foo_local.html'
106+
use reexports::foo_local;
107+
108+
// @has 'foo/outer/inner/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
109+
pub use reexports::Type;
110+
// @has 'foo/outer/inner/type.TypeCrate.html' '//*[@class="rust typedef"]' 'pub(crate) type TypeCrate ='
111+
pub(crate) use reexports::TypeCrate;
112+
// @has 'foo/outer/inner/type.TypeSuper.html' '//*[@class="rust typedef"]' 'pub(in outer) type TypeSuper ='
113+
pub(super) use reexports::TypeSuper;
114+
// @!has 'foo/outer/inner/type.TypeSelf.html'
115+
pub(self) use reexports::TypeSelf;
116+
// @!has 'foo/outer/inner/type.TypeLocal.html'
117+
use reexports::TypeLocal;
118+
119+
// @has 'foo/outer/inner/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
120+
pub use reexports::Union;
121+
// @has 'foo/outer/inner/union.UnionCrate.html' '//*[@class="docblock item-decl"]' 'pub(crate) union UnionCrate {'
122+
pub(crate) use reexports::UnionCrate;
123+
// @has 'foo/outer/inner/union.UnionSuper.html' '//*[@class="docblock item-decl"]' 'pub(in outer) union UnionSuper {'
124+
pub(super) use reexports::UnionSuper;
125+
// @!has 'foo/outer/inner/union.UnionSelf.html'
126+
pub(self) use reexports::UnionSelf;
127+
// @!has 'foo/outer/inner/union.UnionLocal.html'
128+
use reexports::UnionLocal;
129+
}
130+
}
131+
132+
mod re_re_exports {
133+
// @!has 'foo/re_re_exports/union.Union.html'
134+
use crate::reexports::Union;
53135
}

‎src/test/rustdoc/reexports.rs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,38 +10,120 @@ pub use reexports::addr_of;
1010
pub(crate) use reexports::addr_of_crate;
1111
// @!has 'foo/macro.addr_of_self.html'
1212
pub(self) use reexports::addr_of_self;
13+
// @!has 'foo/macro.addr_of_local.html'
14+
use reexports::addr_of_local;
1315

1416
// @has 'foo/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
1517
pub use reexports::Foo;
1618
// @!has 'foo/struct.FooCrate.html'
1719
pub(crate) use reexports::FooCrate;
1820
// @!has 'foo/struct.FooSelf.html'
1921
pub(self) use reexports::FooSelf;
22+
// @!has 'foo/struct.FooLocal.html'
23+
use reexports::FooLocal;
2024

2125
// @has 'foo/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
2226
pub use reexports::Bar;
2327
// @!has 'foo/enum.BarCrate.html'
2428
pub(crate) use reexports::BarCrate;
2529
// @!has 'foo/enum.BarSelf.html'
2630
pub(self) use reexports::BarSelf;
31+
// @!has 'foo/enum.BarLocal.html'
32+
use reexports::BarLocal;
2733

2834
// @has 'foo/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
2935
pub use reexports::foo;
3036
// @!has 'foo/fn.foo_crate.html'
3137
pub(crate) use reexports::foo_crate;
3238
// @!has 'foo/fn.foo_self.html'
3339
pub(self) use reexports::foo_self;
40+
// @!has 'foo/fn.foo_local.html'
41+
use reexports::foo_local;
3442

3543
// @has 'foo/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
3644
pub use reexports::Type;
3745
// @!has 'foo/type.TypeCrate.html'
3846
pub(crate) use reexports::TypeCrate;
3947
// @!has 'foo/type.TypeSelf.html'
4048
pub(self) use reexports::TypeSelf;
49+
// @!has 'foo/type.TypeLocal.html'
50+
use reexports::TypeLocal;
4151

4252
// @has 'foo/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
4353
pub use reexports::Union;
4454
// @!has 'foo/union.UnionCrate.html'
4555
pub(crate) use reexports::UnionCrate;
4656
// @!has 'foo/union.UnionSelf.html'
4757
pub(self) use reexports::UnionSelf;
58+
// @!has 'foo/union.UnionLocal.html'
59+
use reexports::UnionLocal;
60+
61+
pub mod outer {
62+
pub mod inner {
63+
// @has 'foo/outer/inner/macro.addr_of.html' '//*[@class="docblock item-decl"]' 'pub macro addr_of($place : expr) {'
64+
pub use reexports::addr_of;
65+
// @!has 'foo/outer/inner/macro.addr_of_crate.html'
66+
pub(crate) use reexports::addr_of_crate;
67+
// @!has 'foo/outer/inner/macro.addr_of_super.html'
68+
pub(super) use reexports::addr_of_super;
69+
// @!has 'foo/outer/inner/macro.addr_of_self.html'
70+
pub(self) use reexports::addr_of_self;
71+
// @!has 'foo/outer/inner/macro.addr_of_local.html'
72+
use reexports::addr_of_local;
73+
74+
// @has 'foo/outer/inner/struct.Foo.html' '//*[@class="docblock item-decl"]' 'pub struct Foo;'
75+
pub use reexports::Foo;
76+
// @!has 'foo/outer/inner/struct.FooCrate.html'
77+
pub(crate) use reexports::FooCrate;
78+
// @!has 'foo/outer/inner/struct.FooSuper.html'
79+
pub(super) use reexports::FooSuper;
80+
// @!has 'foo/outer/inner/struct.FooSelf.html'
81+
pub(self) use reexports::FooSelf;
82+
// @!has 'foo/outer/inner/struct.FooLocal.html'
83+
use reexports::FooLocal;
84+
85+
// @has 'foo/outer/inner/enum.Bar.html' '//*[@class="docblock item-decl"]' 'pub enum Bar {'
86+
pub use reexports::Bar;
87+
// @!has 'foo/outer/inner/enum.BarCrate.html'
88+
pub(crate) use reexports::BarCrate;
89+
// @!has 'foo/outer/inner/enum.BarSuper.html'
90+
pub(super) use reexports::BarSuper;
91+
// @!has 'foo/outer/inner/enum.BarSelf.html'
92+
pub(self) use reexports::BarSelf;
93+
// @!has 'foo/outer/inner/enum.BarLocal.html'
94+
use reexports::BarLocal;
95+
96+
// @has 'foo/outer/inner/fn.foo.html' '//*[@class="rust fn"]' 'pub fn foo()'
97+
pub use reexports::foo;
98+
// @!has 'foo/outer/inner/fn.foo_crate.html'
99+
pub(crate) use reexports::foo_crate;
100+
// @!has 'foo/outer/inner/fn.foo_super.html'
101+
pub(super) use::reexports::foo_super;
102+
// @!has 'foo/outer/inner/fn.foo_self.html'
103+
pub(self) use reexports::foo_self;
104+
// @!has 'foo/outer/inner/fn.foo_local.html'
105+
use reexports::foo_local;
106+
107+
// @has 'foo/outer/inner/type.Type.html' '//*[@class="rust typedef"]' 'pub type Type ='
108+
pub use reexports::Type;
109+
// @!has 'foo/outer/inner/type.TypeCrate.html'
110+
pub(crate) use reexports::TypeCrate;
111+
// @!has 'foo/outer/inner/type.TypeSuper.html'
112+
pub(super) use reexports::TypeSuper;
113+
// @!has 'foo/outer/inner/type.TypeSelf.html'
114+
pub(self) use reexports::TypeSelf;
115+
// @!has 'foo/outer/inner/type.TypeLocal.html'
116+
use reexports::TypeLocal;
117+
118+
// @has 'foo/outer/inner/union.Union.html' '//*[@class="docblock item-decl"]' 'pub union Union {'
119+
pub use reexports::Union;
120+
// @!has 'foo/outer/inner/union.UnionCrate.html'
121+
pub(crate) use reexports::UnionCrate;
122+
// @!has 'foo/outer/inner/union.UnionSuper.html'
123+
pub(super) use reexports::UnionSuper;
124+
// @!has 'foo/outer/inner/union.UnionSelf.html'
125+
pub(self) use reexports::UnionSelf;
126+
// @!has 'foo/outer/inner/union.UnionLocal.html'
127+
use reexports::UnionLocal;
128+
}
129+
}

0 commit comments

Comments
 (0)