Skip to content

Commit abc6784

Browse files
authored
Rollup merge of #138574 - lolbinarycat:rustdoc-deref-24686-v2, r=GuillaumeGomez
rustdoc: be more strict about "Methods from Deref" fixes #137083 fixes #24686 Currently done: * [x] fix `render_assoc_items_inner * [x] fix sidebar logic * [x] port test from #137564 * [x] add test for sidebar items Note that this does not yet fix the sidebar logic.
2 parents 66f2a19 + b46412f commit abc6784

File tree

4 files changed

+59
-3
lines changed

4 files changed

+59
-3
lines changed

src/librustdoc/clean/types.rs

+14
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,10 @@ impl Type {
15521552
matches!(self, Type::BorrowedRef { .. })
15531553
}
15541554

1555+
fn is_type_alias(&self) -> bool {
1556+
matches!(self, Type::Path { path: Path { res: Res::Def(DefKind::TyAlias, _), .. } })
1557+
}
1558+
15551559
/// Check if two types are "the same" for documentation purposes.
15561560
///
15571561
/// This is different from `Eq`, because it knows that things like
@@ -1580,6 +1584,16 @@ impl Type {
15801584
} else {
15811585
(self, other)
15821586
};
1587+
1588+
// FIXME: `Cache` does not have the data required to unwrap type aliases,
1589+
// so we just assume they are equal.
1590+
// This is only remotely acceptable because we were previously
1591+
// assuming all types were equal when used
1592+
// as a generic parameter of a type in `Deref::Target`.
1593+
if self_cleared.is_type_alias() || other_cleared.is_type_alias() {
1594+
return true;
1595+
}
1596+
15831597
match (self_cleared, other_cleared) {
15841598
// Recursive cases.
15851599
(Type::Tuple(a), Type::Tuple(b)) => {

src/librustdoc/html/render/mod.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ pub(crate) fn ensure_trailing_slash(v: &str) -> impl fmt::Display {
8989

9090
/// Specifies whether rendering directly implemented trait items or ones from a certain Deref
9191
/// impl.
92-
#[derive(Copy, Clone)]
92+
#[derive(Copy, Clone, Debug)]
9393
pub(crate) enum AssocItemRender<'a> {
9494
All,
9595
DerefFor { trait_: &'a clean::Path, type_: &'a clean::Type, deref_mut_: bool },
@@ -1296,7 +1296,8 @@ fn render_assoc_items_inner(
12961296
info!("Documenting associated items of {:?}", containing_item.name);
12971297
let cache = &cx.shared.cache;
12981298
let Some(v) = cache.impls.get(&it) else { return };
1299-
let (non_trait, traits): (Vec<_>, _) = v.iter().partition(|i| i.inner_impl().trait_.is_none());
1299+
let (mut non_trait, traits): (Vec<_>, _) =
1300+
v.iter().partition(|i| i.inner_impl().trait_.is_none());
13001301
if !non_trait.is_empty() {
13011302
let mut close_tags = <Vec<&str>>::with_capacity(1);
13021303
let mut tmp_buf = String::new();
@@ -1314,6 +1315,16 @@ fn render_assoc_items_inner(
13141315
AssocItemRender::DerefFor { trait_, type_, deref_mut_ } => {
13151316
let id =
13161317
cx.derive_id(small_url_encode(format!("deref-methods-{:#}", type_.print(cx))));
1318+
// the `impls.get` above only looks at the outermost type,
1319+
// and the Deref impl may only be implemented for certain
1320+
// values of generic parameters.
1321+
// for example, if an item impls `Deref<[u8]>`,
1322+
// we should not show methods from `[MaybeUninit<u8>]`.
1323+
// this `retain` filters out any instances where
1324+
// the types do not line up perfectly.
1325+
non_trait.retain(|impl_| {
1326+
type_.is_doc_subtype_of(&impl_.inner_impl().for_, &cx.shared.cache)
1327+
});
13171328
let derived_id = cx.derive_id(&id);
13181329
close_tags.push("</details>");
13191330
write_str(
@@ -1392,6 +1403,7 @@ fn render_assoc_items_inner(
13921403
}
13931404
}
13941405

1406+
/// `derefs` is the set of all deref targets that have already been handled.
13951407
fn render_deref_methods(
13961408
mut w: impl Write,
13971409
cx: &Context<'_>,

src/librustdoc/html/render/sidebar.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,10 @@ fn sidebar_deref_methods<'a>(
533533
debug!("found inner_impl: {impls:?}");
534534
let mut ret = impls
535535
.iter()
536-
.filter(|i| i.inner_impl().trait_.is_none())
536+
.filter(|i| {
537+
i.inner_impl().trait_.is_none()
538+
&& real_target.is_doc_subtype_of(&i.inner_impl().for_, &c)
539+
})
537540
.flat_map(|i| get_methods(i.inner_impl(), true, used_links, deref_mut, cx.tcx()))
538541
.collect::<Vec<_>>();
539542
if !ret.is_empty() {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![crate_name = "foo"]
2+
3+
// test for https://github.com/rust-lang/rust/issues/24686
4+
use std::ops::Deref;
5+
6+
pub struct Foo<T>(T);
7+
impl Foo<i32> {
8+
pub fn get_i32(&self) -> i32 { self.0 }
9+
}
10+
impl Foo<u32> {
11+
pub fn get_u32(&self) -> u32 { self.0 }
12+
}
13+
14+
// Note that the same href is used both on the method itself,
15+
// and on the sidebar items.
16+
//@ has foo/struct.Bar.html
17+
//@ has - '//a[@href="#method.get_i32"]' 'get_i32'
18+
//@ !has - '//a[@href="#method.get_u32"]' 'get_u32'
19+
//@ count - '//ul[@class="block deref-methods"]//a' 1
20+
//@ count - '//a[@href="#method.get_i32"]' 2
21+
pub struct Bar(Foo<i32>);
22+
impl Deref for Bar {
23+
type Target = Foo<i32>;
24+
fn deref(&self) -> &Foo<i32> {
25+
&self.0
26+
}
27+
}

0 commit comments

Comments
 (0)