Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rustdoc] Sort impl associated items by kinds and then by appearance #129471

Merged
merged 6 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1794,13 +1794,64 @@ fn render_impl(
let mut default_impl_items = Buffer::empty_from(w);
let impl_ = i.inner_impl();

// Impl items are grouped by kinds:
//
// 1. Constants
// 2. Types
// 3. Functions
//
// This order is because you can have associated constants used in associated types (like array
// length), and both in associcated functions. So with this order, when reading from top to
// bottom, you should see items definitions before they're actually used most of the time.
let mut assoc_types = Vec::new();
let mut methods = Vec::new();

if !impl_.is_negative_trait_impl() {
for trait_item in &impl_.items {
match *trait_item.kind {
clean::MethodItem(..) | clean::TyMethodItem(_) => methods.push(trait_item),
clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => {
assoc_types.push(trait_item)
}
clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => {
// We render it directly since they're supposed to come first.
doc_impl_item(
&mut default_impl_items,
&mut impl_items,
cx,
trait_item,
if trait_.is_some() { &i.impl_item } else { parent },
link,
render_mode,
false,
trait_,
rendering_params,
);
}
_ => {}
}
}

for assoc_type in assoc_types {
doc_impl_item(
&mut default_impl_items,
&mut impl_items,
cx,
trait_item,
assoc_type,
if trait_.is_some() { &i.impl_item } else { parent },
link,
render_mode,
false,
trait_,
rendering_params,
);
}
for method in methods {
doc_impl_item(
&mut default_impl_items,
&mut impl_items,
cx,
method,
if trait_.is_some() { &i.impl_item } else { parent },
link,
render_mode,
Expand Down
32 changes: 16 additions & 16 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -843,55 +843,55 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean:
}
}

if !required_types.is_empty() {
if !required_consts.is_empty() {
write_section_heading(
w,
"Required Associated Types",
"required-associated-types",
"Required Associated Constants",
"required-associated-consts",
None,
"<div class=\"methods\">",
);
for t in required_types {
for t in required_consts {
trait_item(w, cx, t, it);
}
w.write_str("</div>");
}
if !provided_types.is_empty() {
if !provided_consts.is_empty() {
write_section_heading(
w,
"Provided Associated Types",
"provided-associated-types",
"Provided Associated Constants",
"provided-associated-consts",
None,
"<div class=\"methods\">",
);
for t in provided_types {
for t in provided_consts {
trait_item(w, cx, t, it);
}
w.write_str("</div>");
}

if !required_consts.is_empty() {
if !required_types.is_empty() {
write_section_heading(
w,
"Required Associated Constants",
"required-associated-consts",
"Required Associated Types",
"required-associated-types",
None,
"<div class=\"methods\">",
);
for t in required_consts {
for t in required_types {
trait_item(w, cx, t, it);
}
w.write_str("</div>");
}
if !provided_consts.is_empty() {
if !provided_types.is_empty() {
write_section_heading(
w,
"Provided Associated Constants",
"provided-associated-consts",
"Provided Associated Types",
"provided-associated-types",
None,
"<div class=\"methods\">",
);
for t in provided_consts {
for t in provided_types {
trait_item(w, cx, t, it);
}
w.write_str("</div>");
Expand Down
47 changes: 31 additions & 16 deletions src/librustdoc/html/render/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,10 @@ fn sidebar_trait<'a>(

blocks.extend(
[
("required-associated-types", "Required Associated Types", req_assoc),
("provided-associated-types", "Provided Associated Types", prov_assoc),
("required-associated-consts", "Required Associated Constants", req_assoc_const),
("provided-associated-consts", "Provided Associated Constants", prov_assoc_const),
("required-associated-types", "Required Associated Types", req_assoc),
("provided-associated-types", "Provided Associated Types", prov_assoc),
("required-methods", "Required Methods", req_method),
("provided-methods", "Provided Methods", prov_method),
("foreign-impls", "Implementations on Foreign Types", foreign_impls),
Expand Down Expand Up @@ -394,29 +394,22 @@ fn sidebar_assoc_items<'a>(
let cache = cx.cache();

let mut assoc_consts = Vec::new();
let mut assoc_types = Vec::new();
let mut methods = Vec::new();
if let Some(v) = cache.impls.get(&did) {
let mut used_links = FxHashSet::default();
let mut id_map = IdMap::new();

{
let used_links_bor = &mut used_links;
assoc_consts.extend(
v.iter()
.filter(|i| i.inner_impl().trait_.is_none())
.flat_map(|i| get_associated_constants(i.inner_impl(), used_links_bor)),
);
for impl_ in v.iter().map(|i| i.inner_impl()).filter(|i| i.trait_.is_none()) {
assoc_consts.extend(get_associated_constants(impl_, used_links_bor));
assoc_types.extend(get_associated_types(impl_, used_links_bor));
methods.extend(get_methods(impl_, false, used_links_bor, false, cx.tcx()));
}
// We want links' order to be reproducible so we don't use unstable sort.
assoc_consts.sort();

#[rustfmt::skip] // rustfmt makes the pipeline less readable
methods.extend(
v.iter()
.filter(|i| i.inner_impl().trait_.is_none())
.flat_map(|i| get_methods(i.inner_impl(), false, used_links_bor, false, cx.tcx())),
);

// We want links' order to be reproducible so we don't use unstable sort.
assoc_types.sort();
methods.sort();
}

Expand All @@ -426,6 +419,11 @@ fn sidebar_assoc_items<'a>(
"associatedconstant",
assoc_consts,
),
LinkBlock::new(
Link::new("implementations", "Associated Types"),
"associatedtype",
assoc_types,
),
LinkBlock::new(Link::new("implementations", "Methods"), "method", methods),
];

Expand All @@ -452,6 +450,7 @@ fn sidebar_assoc_items<'a>(
&mut blocks,
);
}

links.append(&mut blocks);
}
}
Expand Down Expand Up @@ -715,3 +714,19 @@ fn get_associated_constants<'a>(
})
.collect::<Vec<_>>()
}

fn get_associated_types<'a>(
i: &'a clean::Impl,
used_links: &mut FxHashSet<String>,
) -> Vec<Link<'a>> {
i.items
.iter()
.filter_map(|item| match item.name {
Some(ref name) if !name.is_empty() && item.is_associated_type() => Some(Link::new(
get_next_url(used_links, format!("{typ}.{name}", typ = ItemType::AssocType)),
name.as_str(),
)),
_ => None,
})
.collect::<Vec<_>>()
}
42 changes: 42 additions & 0 deletions tests/rustdoc/impl-associated-items-order.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// This test ensures that impl associated items always follow this order:
//
// 1. Consts
// 2. Types
// 3. Functions

#![feature(inherent_associated_types)]
#![allow(incomplete_features)]
#![crate_name = "foo"]

//@ has 'foo/struct.Bar.html'
pub struct Bar;

impl Bar {
//@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[3]/h4' \
// 'pub fn foo()'
pub fn foo() {}
//@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \
// 'pub const X: u8 = 12u8'
pub const X: u8 = 12;
//@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \
// 'pub type Y = u8'
pub type Y = u8;
}

pub trait Foo {
const W: u32;
fn yeay();
type Z;
}

impl Foo for Bar {
//@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[2]/h4' \
// 'type Z = u8'
type Z = u8;
//@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \
// 'const W: u32 = 12u32'
const W: u32 = 12;
//@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \
// 'fn yeay()'
fn yeay() {}
}
42 changes: 42 additions & 0 deletions tests/rustdoc/impl-associated-items-sidebar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// This test ensures that impl/trait associated items are listed in the sidebar.

// ignore-tidy-linelength

#![feature(inherent_associated_types)]
#![feature(associated_type_defaults)]
#![allow(incomplete_features)]
#![crate_name = "foo"]

//@ has 'foo/struct.Bar.html'
pub struct Bar;

impl Bar {
//@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Associated Constants'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedconstant"]/li/a[@href="#associatedconstant.X"]' 'X'
pub const X: u8 = 12;
//@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Associated Types'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedtype"]/li/a[@href="#associatedtype.Y"]' 'Y'
pub type Y = u8;
}

//@ has 'foo/trait.Foo.html'
pub trait Foo {
//@ has - '//*[@class="sidebar-elems"]//h3[5]' 'Required Methods'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][5]/li/a[@href="#tymethod.yeay"]' 'yeay'
fn yeay();
//@ has - '//*[@class="sidebar-elems"]//h3[6]' 'Provided Methods'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][6]/li/a[@href="#method.boo"]' 'boo'
fn boo() {}
//@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Required Associated Constants'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][1]/li/a[@href="#associatedconstant.W"]' 'W'
const W: u32;
//@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Provided Associated Constants'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][2]/li/a[@href="#associatedconstant.U"]' 'U'
const U: u32 = 0;
//@ has - '//*[@class="sidebar-elems"]//h3[3]' 'Required Associated Types'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][3]/li/a[@href="#associatedtype.Z"]' 'Z'
type Z;
//@ has - '//*[@class="sidebar-elems"]//h3[4]' 'Provided Associated Types'
//@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][4]/li/a[@href="#associatedtype.T"]' 'T'
type T = u32;
}
Loading