Skip to content

Commit c1a6199

Browse files
committed
Auto merge of rust-lang#128146 - notriddle:notriddle/natsortfixes, r=GuillaumeGomez
rustdoc: clean up and fix ord violations in item sorting Based on rust-lang#128139 with a few minor changes: - The name sorting function is changed to follow the [version sort] from the style guide - the `cmp` function is redesigned to more obviously make a partial order, by always return `cmp()` of the same variable as the `!=` above [version sort]: https://doc.rust-lang.org/nightly/style-guide/index.html#sorting
2 parents 6106b05 + 5384692 commit c1a6199

File tree

2 files changed

+119
-70
lines changed

2 files changed

+119
-70
lines changed

src/librustdoc/html/render/print_item.rs

+118-69
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ trait ItemTemplate<'a, 'cx: 'a>: rinja::Template + fmt::Display {
316316
fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items: &[clean::Item]) {
317317
write!(w, "{}", document(cx, item, None, HeadingOffset::H2));
318318

319-
let mut indices = (0..items.len()).filter(|i| !items[*i].is_stripped()).collect::<Vec<usize>>();
319+
let mut not_stripped_items =
320+
items.iter().filter(|i| !i.is_stripped()).enumerate().collect::<Vec<_>>();
320321

321322
// the order of item types in the listing
322323
fn reorder(ty: ItemType) -> u8 {
@@ -338,37 +339,29 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
338339
}
339340
}
340341

341-
fn cmp(
342-
i1: &clean::Item,
343-
i2: &clean::Item,
344-
idx1: usize,
345-
idx2: usize,
346-
tcx: TyCtxt<'_>,
347-
) -> Ordering {
348-
let ty1 = i1.type_();
349-
let ty2 = i2.type_();
350-
if item_ty_to_section(ty1) != item_ty_to_section(ty2)
351-
|| (ty1 != ty2 && (ty1 == ItemType::ExternCrate || ty2 == ItemType::ExternCrate))
352-
{
353-
return (reorder(ty1), idx1).cmp(&(reorder(ty2), idx2));
354-
}
355-
let s1 = i1.stability(tcx).as_ref().map(|s| s.level);
356-
let s2 = i2.stability(tcx).as_ref().map(|s| s.level);
357-
if let (Some(a), Some(b)) = (s1, s2) {
358-
match (a.is_stable(), b.is_stable()) {
359-
(true, true) | (false, false) => {}
360-
(false, true) => return Ordering::Greater,
361-
(true, false) => return Ordering::Less,
362-
}
342+
fn cmp(i1: &clean::Item, i2: &clean::Item, tcx: TyCtxt<'_>) -> Ordering {
343+
let rty1 = reorder(i1.type_());
344+
let rty2 = reorder(i2.type_());
345+
if rty1 != rty2 {
346+
return rty1.cmp(&rty2);
347+
}
348+
let is_stable1 = i1.stability(tcx).as_ref().map(|s| s.level.is_stable()).unwrap_or(true);
349+
let is_stable2 = i2.stability(tcx).as_ref().map(|s| s.level.is_stable()).unwrap_or(true);
350+
if is_stable1 != is_stable2 {
351+
// true is bigger than false in the standard bool ordering,
352+
// but we actually want stable items to come first
353+
return is_stable2.cmp(&is_stable1);
363354
}
364355
let lhs = i1.name.unwrap_or(kw::Empty);
365356
let rhs = i2.name.unwrap_or(kw::Empty);
366357
compare_names(lhs.as_str(), rhs.as_str())
367358
}
368359

360+
let tcx = cx.tcx();
361+
369362
match cx.shared.module_sorting {
370363
ModuleSorting::Alphabetical => {
371-
indices.sort_by(|&i1, &i2| cmp(&items[i1], &items[i2], i1, i2, cx.tcx()));
364+
not_stripped_items.sort_by(|(_, i1), (_, i2)| cmp(i1, i2, tcx));
372365
}
373366
ModuleSorting::DeclarationOrder => {}
374367
}
@@ -391,24 +384,19 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
391384
// can be identical even if the elements are different (mostly in imports).
392385
// So in case this is an import, we keep everything by adding a "unique id"
393386
// (which is the position in the vector).
394-
indices.dedup_by_key(|i| {
387+
not_stripped_items.dedup_by_key(|(idx, i)| {
395388
(
396-
items[*i].item_id,
397-
if items[*i].name.is_some() { Some(full_path(cx, &items[*i])) } else { None },
398-
items[*i].type_(),
399-
if items[*i].is_import() { *i } else { 0 },
389+
i.item_id,
390+
if i.name.is_some() { Some(full_path(cx, i)) } else { None },
391+
i.type_(),
392+
if i.is_import() { *idx } else { 0 },
400393
)
401394
});
402395

403-
debug!("{indices:?}");
396+
debug!("{not_stripped_items:?}");
404397
let mut last_section = None;
405398

406-
for &idx in &indices {
407-
let myitem = &items[idx];
408-
if myitem.is_stripped() {
409-
continue;
410-
}
411-
399+
for (_, myitem) in &not_stripped_items {
412400
let my_section = item_ty_to_section(myitem.type_());
413401
if Some(my_section) != last_section {
414402
if last_section.is_some() {
@@ -424,7 +412,6 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
424412
);
425413
}
426414

427-
let tcx = cx.tcx();
428415
match *myitem.kind {
429416
clean::ExternCrateItem { ref src } => {
430417
use crate::html::format::anchor;
@@ -453,7 +440,7 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
453440
let stab_tags = if let Some(import_def_id) = import.source.did {
454441
// Just need an item with the correct def_id and attrs
455442
let import_item =
456-
clean::Item { item_id: import_def_id.into(), ..myitem.clone() };
443+
clean::Item { item_id: import_def_id.into(), ..(*myitem).clone() };
457444

458445
let stab_tags = Some(extra_info_tags(&import_item, item, tcx).to_string());
459446
stab_tags
@@ -2010,40 +1997,102 @@ fn item_keyword(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item) {
20101997
}
20111998

20121999
/// Compare two strings treating multi-digit numbers as single units (i.e. natural sort order).
2013-
pub(crate) fn compare_names(mut lhs: &str, mut rhs: &str) -> Ordering {
2014-
/// Takes a non-numeric and a numeric part from the given &str.
2015-
fn take_parts<'a>(s: &mut &'a str) -> (&'a str, &'a str) {
2016-
let i = s.find(|c: char| c.is_ascii_digit());
2017-
let (a, b) = s.split_at(i.unwrap_or(s.len()));
2018-
let i = b.find(|c: char| !c.is_ascii_digit());
2019-
let (b, c) = b.split_at(i.unwrap_or(b.len()));
2020-
*s = c;
2021-
(a, b)
2022-
}
2023-
2024-
while !lhs.is_empty() || !rhs.is_empty() {
2025-
let (la, lb) = take_parts(&mut lhs);
2026-
let (ra, rb) = take_parts(&mut rhs);
2027-
// First process the non-numeric part.
2028-
match la.cmp(ra) {
2029-
Ordering::Equal => (),
2030-
x => return x,
2031-
}
2032-
// Then process the numeric part, if both sides have one (and they fit in a u64).
2033-
if let (Ok(ln), Ok(rn)) = (lb.parse::<u64>(), rb.parse::<u64>()) {
2034-
match ln.cmp(&rn) {
2035-
Ordering::Equal => (),
2036-
x => return x,
2000+
///
2001+
/// This code is copied from [`rustfmt`], and should probably be released as a crate at some point.
2002+
///
2003+
/// [`rustfmt`]:https://github.com/rust-lang/rustfmt/blob/rustfmt-2.0.0-rc.2/src/formatting/reorder.rs#L32
2004+
pub(crate) fn compare_names(left: &str, right: &str) -> Ordering {
2005+
let mut left = left.chars().peekable();
2006+
let mut right = right.chars().peekable();
2007+
2008+
loop {
2009+
// The strings are equal so far and not inside a number in both sides
2010+
let (l, r) = match (left.next(), right.next()) {
2011+
// Is this the end of both strings?
2012+
(None, None) => return Ordering::Equal,
2013+
// If for one, the shorter one is considered smaller
2014+
(None, Some(_)) => return Ordering::Less,
2015+
(Some(_), None) => return Ordering::Greater,
2016+
(Some(l), Some(r)) => (l, r),
2017+
};
2018+
let next_ordering = match (l.to_digit(10), r.to_digit(10)) {
2019+
// If neither is a digit, just compare them
2020+
(None, None) => Ord::cmp(&l, &r),
2021+
// The one with shorter non-digit run is smaller
2022+
// For `strverscmp` it's smaller iff next char in longer is greater than digits
2023+
(None, Some(_)) => Ordering::Greater,
2024+
(Some(_), None) => Ordering::Less,
2025+
// If both start numbers, we have to compare the numbers
2026+
(Some(l), Some(r)) => {
2027+
if l == 0 || r == 0 {
2028+
// Fraction mode: compare as if there was leading `0.`
2029+
let ordering = Ord::cmp(&l, &r);
2030+
if ordering != Ordering::Equal {
2031+
return ordering;
2032+
}
2033+
loop {
2034+
// Get next pair
2035+
let (l, r) = match (left.peek(), right.peek()) {
2036+
// Is this the end of both strings?
2037+
(None, None) => return Ordering::Equal,
2038+
// If for one, the shorter one is considered smaller
2039+
(None, Some(_)) => return Ordering::Less,
2040+
(Some(_), None) => return Ordering::Greater,
2041+
(Some(l), Some(r)) => (l, r),
2042+
};
2043+
// Are they digits?
2044+
match (l.to_digit(10), r.to_digit(10)) {
2045+
// If out of digits, use the stored ordering due to equal length
2046+
(None, None) => break Ordering::Equal,
2047+
// If one is shorter, it's smaller
2048+
(None, Some(_)) => return Ordering::Less,
2049+
(Some(_), None) => return Ordering::Greater,
2050+
// If both are digits, consume them and take into account
2051+
(Some(l), Some(r)) => {
2052+
left.next();
2053+
right.next();
2054+
let ordering = Ord::cmp(&l, &r);
2055+
if ordering != Ordering::Equal {
2056+
return ordering;
2057+
}
2058+
}
2059+
}
2060+
}
2061+
} else {
2062+
// Integer mode
2063+
let mut same_length_ordering = Ord::cmp(&l, &r);
2064+
loop {
2065+
// Get next pair
2066+
let (l, r) = match (left.peek(), right.peek()) {
2067+
// Is this the end of both strings?
2068+
(None, None) => return same_length_ordering,
2069+
// If for one, the shorter one is considered smaller
2070+
(None, Some(_)) => return Ordering::Less,
2071+
(Some(_), None) => return Ordering::Greater,
2072+
(Some(l), Some(r)) => (l, r),
2073+
};
2074+
// Are they digits?
2075+
match (l.to_digit(10), r.to_digit(10)) {
2076+
// If out of digits, use the stored ordering due to equal length
2077+
(None, None) => break same_length_ordering,
2078+
// If one is shorter, it's smaller
2079+
(None, Some(_)) => return Ordering::Less,
2080+
(Some(_), None) => return Ordering::Greater,
2081+
// If both are digits, consume them and take into account
2082+
(Some(l), Some(r)) => {
2083+
left.next();
2084+
right.next();
2085+
same_length_ordering = same_length_ordering.then(Ord::cmp(&l, &r));
2086+
}
2087+
}
2088+
}
2089+
}
20372090
}
2038-
}
2039-
// Then process the numeric part again, but this time as strings.
2040-
match lb.cmp(rb) {
2041-
Ordering::Equal => (),
2042-
x => return x,
2091+
};
2092+
if next_ordering != Ordering::Equal {
2093+
return next_ordering;
20432094
}
20442095
}
2045-
2046-
Ordering::Equal
20472096
}
20482097

20492098
pub(super) fn full_path(cx: &Context<'_>, item: &clean::Item) -> String {

src/librustdoc/html/render/tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn test_compare_names() {
3434
#[test]
3535
fn test_name_sorting() {
3636
let names = [
37-
"Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit01", "Fruit1", "Fruit02", "Fruit2",
37+
"Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit01", "Fruit02", "Fruit1", "Fruit2",
3838
"Fruit20", "Fruit30x", "Fruit100", "Pear",
3939
];
4040
let mut sorted = names.to_owned();

0 commit comments

Comments
 (0)