Rollup merge of rust-lang#119997 - GuillaumeGomez:fix-stripped-impl-o…

…n-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with `@petrochenkov.` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? `@notriddle`

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,21 +219,21 @@ 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()) {
                 for typaram in generics {
                     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;
                     }
                 }

tests/rustdoc/async-fn.rs

@@ -48,6 +48,8 @@ impl Foo {
 
 pub trait Pattern<'a> {}
 
+impl Pattern<'_> for () {}
+
 pub trait Trait<const N: usize> {}
 // @has async_fn/fn.const_generics.html
 // @has - '//pre[@class="rust item-decl"]' 'pub async fn const_generics<const N: usize>(_: impl Trait<N>)'
@@ -57,18 +59,18 @@ pub async fn const_generics<const N: usize>(_: impl Trait<N>) {}
 // 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<dyn Bar>'
-pub async fn static_trait(foo: &str) -> Box<dyn Bar> {}
+pub async fn static_trait(foo: &str) -> Box<dyn Bar> { 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<dyn Bar + '_>"
-pub async fn lifetime_for_trait(foo: &str) -> Box<dyn Bar + '_> {}
+pub async fn lifetime_for_trait(foo: &str) -> Box<dyn Bar + '_> { 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,18 +80,20 @@ 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<Item = &usize>'
-    pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator<Item = &usize> {}
+    pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator<Item = &usize> {
+        [0].iter()
+    }
     // taken from `tokio` as an example of a method that was particularly bad before
     // @has - '//*[@class="method"]' "pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()>"
-    pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()> {}
+    pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()> { Err(()) }
     // @has - '//*[@class="method"]' "pub async fn mut_self(&mut self)"
     pub async fn mut_self(&mut self) {}
 }
 
 // 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> {}

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) {}
+}