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.

2 files changed, 63 insertions, 29 deletions

diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 5fb2d9f6f91..f98ca8a5089 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::<u64>(), rb.parse::<u64>()) {
+            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 99ad26549f5..abf5f05fe58 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);
 }