From 6237bebd349c7ba9286e2daa76eef07bd6b7c5be Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Jan 2024 16:46:54 +0100 Subject: [PATCH 1/2] Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias --- src/librustdoc/passes/stripper.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index df955421ba487..1bea93c784218 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -56,13 +56,10 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> { | clean::TraitItem(..) | clean::FunctionItem(..) | clean::VariantItem(..) - | clean::MethodItem(..) | clean::ForeignFunctionItem(..) | clean::ForeignStaticItem(..) | clean::ConstantItem(..) | clean::UnionItem(..) - | clean::AssocConstItem(..) - | clean::AssocTypeItem(..) | clean::TraitAliasItem(..) | clean::MacroItem(..) | clean::ForeignTypeItem => { @@ -80,6 +77,16 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> { } } + clean::MethodItem(..) | clean::AssocConstItem(..) | clean::AssocTypeItem(..) => { + let item_id = i.item_id; + if item_id.is_local() + && !self.effective_visibilities.is_reachable(self.tcx, item_id.expect_def_id()) + { + debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name); + return None; + } + } + clean::StructFieldItem(..) => { if i.visibility(self.tcx) != Some(Visibility::Public) { return Some(strip_item(i)); @@ -192,16 +199,16 @@ impl<'a> DocFolder for ImplStripper<'a, '_> { && imp.items.iter().all(|i| { let item_id = i.item_id; item_id.is_local() - && !is_item_reachable( - self.tcx, - self.is_json_output, - &self.cache.effective_visibilities, - item_id, - ) + && !self + .cache + .effective_visibilities + .is_reachable(self.tcx, item_id.expect_def_id()) }) { + debug!("ImplStripper: no public item; removing {imp:?}"); return None; } else if imp.items.is_empty() && i.doc_value().is_empty() { + debug!("ImplStripper: no item and no doc; removing {imp:?}"); return None; } } @@ -212,13 +219,13 @@ impl<'a> DocFolder for ImplStripper<'a, '_> { && !imp.for_.is_assoc_ty() && !self.should_keep_impl(&i, did) { - debug!("ImplStripper: impl item for stripped type; removing"); + debug!("ImplStripper: impl item for stripped type; removing {imp:?}"); return None; } if let Some(did) = imp.trait_.as_ref().map(|t| t.def_id()) && !self.should_keep_impl(&i, did) { - debug!("ImplStripper: impl item for stripped trait; removing"); + debug!("ImplStripper: impl item for stripped trait; removing {imp:?}"); return None; } if let Some(generics) = imp.trait_.as_ref().and_then(|t| t.generics()) { @@ -226,7 +233,7 @@ impl<'a> DocFolder for ImplStripper<'a, '_> { if let Some(did) = typaram.def_id(self.cache) && !self.should_keep_impl(&i, did) { - debug!("ImplStripper: stripped item in trait's generics; removing impl"); + debug!("ImplStripper: stripped item in trait's generics; removing {imp:?}"); return None; } } From 0933f48ac0a8631b3f7c9c508f38843ef10e839d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Jan 2024 16:47:14 +0100 Subject: [PATCH 2/2] Add regression test for #119015 and update tests --- tests/rustdoc/async-fn.rs | 18 ++++++++----- .../rustdoc/impl-on-ty-alias-issue-119015.rs | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 tests/rustdoc/impl-on-ty-alias-issue-119015.rs diff --git a/tests/rustdoc/async-fn.rs b/tests/rustdoc/async-fn.rs index 70bcbcb6ff44a..b3c35c8c3f309 100644 --- a/tests/rustdoc/async-fn.rs +++ b/tests/rustdoc/async-fn.rs @@ -48,6 +48,8 @@ impl Foo { pub trait Pattern<'a> {} +impl Pattern<'_> for () {} + pub trait Trait {} // @has async_fn/fn.const_generics.html // @has - '//pre[@class="rust item-decl"]' 'pub async fn const_generics(_: impl Trait)' @@ -57,18 +59,18 @@ pub async fn const_generics(_: impl Trait) {} // regression test for #63037 // @has async_fn/fn.elided.html // @has - '//pre[@class="rust item-decl"]' 'pub async fn elided(foo: &str) -> &str' -pub async fn elided(foo: &str) -> &str {} +pub async fn elided(foo: &str) -> &str { "" } // This should really be shown as written, but for implementation reasons it's difficult. // See `impl Clean for TyKind::Ref`. // @has async_fn/fn.user_elided.html // @has - '//pre[@class="rust item-decl"]' 'pub async fn user_elided(foo: &str) -> &str' -pub async fn user_elided(foo: &'_ str) -> &str {} +pub async fn user_elided(foo: &'_ str) -> &str { "" } // @has async_fn/fn.static_trait.html // @has - '//pre[@class="rust item-decl"]' 'pub async fn static_trait(foo: &str) -> Box' -pub async fn static_trait(foo: &str) -> Box {} +pub async fn static_trait(foo: &str) -> Box { Box::new(()) } // @has async_fn/fn.lifetime_for_trait.html // @has - '//pre[@class="rust item-decl"]' "pub async fn lifetime_for_trait(foo: &str) -> Box" -pub async fn lifetime_for_trait(foo: &str) -> Box {} +pub async fn lifetime_for_trait(foo: &str) -> Box { Box::new(()) } // @has async_fn/fn.elided_in_input_trait.html // @has - '//pre[@class="rust item-decl"]' "pub async fn elided_in_input_trait(t: impl Pattern<'_>)" pub async fn elided_in_input_trait(t: impl Pattern<'_>) {} @@ -78,10 +80,12 @@ struct AsyncFdReadyGuard<'a, T> { x: &'a T } impl Foo { // @has async_fn/struct.Foo.html // @has - '//*[@class="method"]' 'pub async fn complicated_lifetimes( &self, context: &impl Bar ) -> impl Iterator' - pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator {} + pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator { + [0].iter() + } // taken from `tokio` as an example of a method that was particularly bad before // @has - '//*[@class="method"]' "pub async fn readable(&self) -> Result, ()>" - pub async fn readable(&self) -> Result, ()> {} + pub async fn readable(&self) -> Result, ()> { Err(()) } // @has - '//*[@class="method"]' "pub async fn mut_self(&mut self)" pub async fn mut_self(&mut self) {} } @@ -89,7 +93,7 @@ impl Foo { // test named lifetimes, just in case // @has async_fn/fn.named.html // @has - '//pre[@class="rust item-decl"]' "pub async fn named<'a, 'b>(foo: &'a str) -> &'b str" -pub async fn named<'a, 'b>(foo: &'a str) -> &'b str {} +pub async fn named<'a, 'b>(foo: &'a str) -> &'b str { "" } // @has async_fn/fn.named_trait.html // @has - '//pre[@class="rust item-decl"]' "pub async fn named_trait<'a, 'b>(foo: impl Pattern<'a>) -> impl Pattern<'b>" pub async fn named_trait<'a, 'b>(foo: impl Pattern<'a>) -> impl Pattern<'b> {} diff --git a/tests/rustdoc/impl-on-ty-alias-issue-119015.rs b/tests/rustdoc/impl-on-ty-alias-issue-119015.rs new file mode 100644 index 0000000000000..68996deae6fda --- /dev/null +++ b/tests/rustdoc/impl-on-ty-alias-issue-119015.rs @@ -0,0 +1,27 @@ +#![crate_name = "foo"] + +// @has 'foo/index.html' +// There should be only `type A`. +// @count - '//*[@class="item-table"]//*[@class="item-name"]' 1 +// @has - '//*[@class="item-name"]/a[@href="type.A.html"]' 'A' + +mod foo { + pub struct S; +} + +use foo::S; + +pub type A = S; + +// @has 'foo/type.A.html' +// @has - '//*[@id="method.default"]/h4' 'fn default() -> Self' +impl Default for A { + fn default() -> Self { + S + } +} + +// @has - '//*[@id="method.a"]/h4' 'pub fn a(&self)' +impl A { + pub fn a(&self) {} +}