From 8c705f83dbe1d8daef8ee0735abf8583a66f7732 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 9 Aug 2020 21:23:19 +0200 Subject: [PATCH] Rustdoc: Fix natural ordering to look at all numbers. The old implementation only looks at numbers at the end, but not in other places in a name: "u8" and "u16" got sorted properly, but "u8_bla" and "u16_bla" did not. --- src/librustdoc/html/render/mod.rs | 54 +++++++++++++++++++---------- src/librustdoc/html/render/tests.rs | 38 ++++++++++++++------ 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 5fb2d9f6f917c..f98ca8a5089d2 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1903,23 +1903,41 @@ fn document_non_exhaustive(w: &mut Buffer, item: &clean::Item) { } } -fn name_key(name: &str) -> (&str, u64, usize) { - let end = name.bytes().rposition(|b| b.is_ascii_digit()).map_or(name.len(), |i| i + 1); - - // find number at end - let split = name[0..end].bytes().rposition(|b| !b.is_ascii_digit()).map_or(0, |i| i + 1); - - // count leading zeroes - let after_zeroes = - name[split..end].bytes().position(|b| b != b'0').map_or(name.len(), |extra| split + extra); - - // sort leading zeroes last - let num_zeroes = after_zeroes - split; - - match name[split..end].parse() { - Ok(n) => (&name[..split], n, num_zeroes), - Err(_) => (name, 0, num_zeroes), +/// Compare two strings treating multi-digit numbers as single units (i.e. natural sort order). +pub fn compare_names(mut lhs: &str, mut rhs: &str) -> Ordering { + /// Takes a non-numeric and a numeric part from the given &str. + fn take_parts<'a>(s: &mut &'a str) -> (&'a str, &'a str) { + let i = s.find(|c: char| c.is_ascii_digit()); + let (a, b) = s.split_at(i.unwrap_or(s.len())); + let i = b.find(|c: char| !c.is_ascii_digit()); + let (b, c) = b.split_at(i.unwrap_or(b.len())); + *s = c; + (a, b) + } + + while !lhs.is_empty() || !rhs.is_empty() { + let (la, lb) = take_parts(&mut lhs); + let (ra, rb) = take_parts(&mut rhs); + // First process the non-numeric part. + match la.cmp(ra) { + Ordering::Equal => (), + x => return x, + } + // Then process the numeric part, if both sides have one (and they fit in a u64). + if let (Ok(ln), Ok(rn)) = (lb.parse::(), rb.parse::()) { + match ln.cmp(&rn) { + Ordering::Equal => (), + x => return x, + } + } + // Then process the numeric part again, but this time as strings. + match lb.cmp(rb) { + Ordering::Equal => (), + x => return x, + } } + + Ordering::Equal } fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean::Item]) { @@ -1962,7 +1980,7 @@ fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean: } let lhs = i1.name.as_ref().map_or("", |s| &**s); let rhs = i2.name.as_ref().map_or("", |s| &**s); - name_key(lhs).cmp(&name_key(rhs)) + compare_names(lhs, rhs) } if cx.shared.sort_modules_alphabetically { @@ -2395,7 +2413,7 @@ fn compare_impl<'a, 'b>(lhs: &'a &&Impl, rhs: &'b &&Impl) -> Ordering { let rhs = format!("{}", rhs.inner_impl().print()); // lhs and rhs are formatted as HTML, which may be unnecessary - name_key(&lhs).cmp(&name_key(&rhs)) + compare_names(&lhs, &rhs) } fn item_trait(w: &mut Buffer, cx: &Context, it: &clean::Item, t: &clean::Trait, cache: &Cache) { diff --git a/src/librustdoc/html/render/tests.rs b/src/librustdoc/html/render/tests.rs index 99ad26549f5d3..abf5f05fe58ab 100644 --- a/src/librustdoc/html/render/tests.rs +++ b/src/librustdoc/html/render/tests.rs @@ -1,24 +1,40 @@ use super::*; #[test] -fn test_name_key() { - assert_eq!(name_key("0"), ("", 0, 1)); - assert_eq!(name_key("123"), ("", 123, 0)); - assert_eq!(name_key("Fruit"), ("Fruit", 0, 0)); - assert_eq!(name_key("Fruit0"), ("Fruit", 0, 1)); - assert_eq!(name_key("Fruit0000"), ("Fruit", 0, 4)); - assert_eq!(name_key("Fruit01"), ("Fruit", 1, 1)); - assert_eq!(name_key("Fruit10"), ("Fruit", 10, 0)); - assert_eq!(name_key("Fruit123"), ("Fruit", 123, 0)); +fn test_compare_names() { + for &(a, b) in &[ + ("hello", "world"), + ("", "world"), + ("123", "hello"), + ("123", ""), + ("123test", "123"), + ("hello", ""), + ("hello", "hello"), + ("hello123", "hello123"), + ("hello123", "hello12"), + ("hello12", "hello123"), + ("hello01abc", "hello01xyz"), + ("hello0abc", "hello0"), + ("hello0", "hello0abc"), + ("01", "1"), + ] { + assert_eq!(compare_names(a, b), a.cmp(b), "{:?} - {:?}", a, b); + } + assert_eq!(compare_names("u8", "u16"), Ordering::Less); + assert_eq!(compare_names("u32", "u16"), Ordering::Greater); + assert_eq!(compare_names("u8_to_f64", "u16_to_f64"), Ordering::Less); + assert_eq!(compare_names("u32_to_f64", "u16_to_f64"), Ordering::Greater); + assert_eq!(compare_names("u16_to_f64", "u16_to_f64"), Ordering::Equal); + assert_eq!(compare_names("u16_to_f32", "u16_to_f64"), Ordering::Less); } #[test] fn test_name_sorting() { let names = [ - "Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit1", "Fruit01", "Fruit2", "Fruit02", + "Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit01", "Fruit1", "Fruit02", "Fruit2", "Fruit20", "Fruit30x", "Fruit100", "Pear", ]; let mut sorted = names.to_owned(); - sorted.sort_by_key(|&s| name_key(s)); + sorted.sort_by(|&l, r| compare_names(l, r)); assert_eq!(names, sorted); }