diff --git a/rustfmt-core/rustfmt-lib/src/imports.rs b/rustfmt-core/rustfmt-lib/src/imports.rs index 94cc419eee5..f686e22d250 100644 --- a/rustfmt-core/rustfmt-lib/src/imports.rs +++ b/rustfmt-core/rustfmt-lib/src/imports.rs @@ -11,6 +11,7 @@ use crate::config::{Edition, IndentStyle}; use crate::lists::{ definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator, }; +use crate::reorder::{compare_as_versions, compare_opt_ident_as_versions}; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::Shape; use crate::source_map::SpanUtils; @@ -654,12 +655,7 @@ impl Ord for UseSegment { match (self, other) { (&Slf(ref a), &Slf(ref b)) | (&Super(ref a), &Super(ref b)) - | (&Crate(ref a), &Crate(ref b)) => match (a, b) { - (Some(sa), Some(sb)) => { - sa.trim_start_matches("r#").cmp(sb.trim_start_matches("r#")) - } - (_, _) => a.cmp(b), - }, + | (&Crate(ref a), &Crate(ref b)) => compare_opt_ident_as_versions(&a, &b), (&Glob, &Glob) => Ordering::Equal, (&Ident(ref pia, ref aa), &Ident(ref pib, ref ab)) => { let ia = pia.trim_start_matches("r#"); @@ -677,18 +673,8 @@ impl Ord for UseSegment { if !is_upper_snake_case(ia) && is_upper_snake_case(ib) { return Ordering::Less; } - let ident_ord = ia.cmp(ib); - if ident_ord != Ordering::Equal { - return ident_ord; - } - match (aa, ab) { - (None, Some(_)) => Ordering::Less, - (Some(_), None) => Ordering::Greater, - (Some(aas), Some(abs)) => aas - .trim_start_matches("r#") - .cmp(abs.trim_start_matches("r#")), - (None, None) => Ordering::Equal, - } + + compare_as_versions(&ia, &ib).then_with(|| compare_opt_ident_as_versions(&aa, &ab)) } (&List(ref a), &List(ref b)) => { for (a, b) in a.iter().zip(b.iter()) { @@ -716,17 +702,17 @@ impl Ord for UseSegment { impl Ord for UseTree { fn cmp(&self, other: &UseTree) -> Ordering { for (a, b) in self.path.iter().zip(other.path.iter()) { - let ord = a.cmp(b); // The comparison without aliases is a hack to avoid situations like // comparing `a::b` to `a as c` - where the latter should be ordered // first since it is shorter. - if ord != Ordering::Equal && a.remove_alias().cmp(&b.remove_alias()) != Ordering::Equal - { + let ord = a.remove_alias().cmp(&b.remove_alias()); + if ord != Ordering::Equal { return ord; } } - self.path.len().cmp(&other.path.len()) + Ord::cmp(&self.path.len(), &other.path.len()) + .then(Ord::cmp(&self.path.last(), &other.path.last())) } } diff --git a/rustfmt-core/rustfmt-lib/src/items.rs b/rustfmt-core/rustfmt-lib/src/items.rs index 98554f15deb..1c453be752c 100644 --- a/rustfmt-core/rustfmt-lib/src/items.rs +++ b/rustfmt-core/rustfmt-lib/src/items.rs @@ -23,6 +23,7 @@ use crate::expr::{ use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator}; use crate::macros::{rewrite_macro, MacroPosition}; use crate::overflow; +use crate::reorder::compare_as_versions; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; use crate::source_map::{LineRangeUtils, SpanUtils}; @@ -701,10 +702,10 @@ impl<'a> FmtVisitor<'a> { (TyAlias(_, _, _, ref lty), TyAlias(_, _, _, ref rty)) if both_type(lty, rty) || both_opaque(lty, rty) => { - a.ident.as_str().cmp(&b.ident.as_str()) + compare_as_versions(&a.ident.as_str(), &b.ident.as_str()) } (Const(..), Const(..)) | (MacCall(..), MacCall(..)) => { - a.ident.as_str().cmp(&b.ident.as_str()) + compare_as_versions(&a.ident.as_str(), &b.ident.as_str()) } (Fn(..), Fn(..)) => a.span.lo().cmp(&b.span.lo()), (TyAlias(_, _, _, ref ty), _) if is_type(ty) => Ordering::Less, diff --git a/rustfmt-core/rustfmt-lib/src/reorder.rs b/rustfmt-core/rustfmt-lib/src/reorder.rs index 3b9f2475acc..39094864e46 100644 --- a/rustfmt-core/rustfmt-lib/src/reorder.rs +++ b/rustfmt-core/rustfmt-lib/src/reorder.rs @@ -22,18 +22,133 @@ use crate::spanned::Spanned; use crate::utils::{contains_skip, mk_sp}; use crate::visitor::FmtVisitor; +/// Compare strings according to version sort (roughly equivalent to `strverscmp`) +pub(crate) fn compare_as_versions(left: &str, right: &str) -> Ordering { + let mut left = left.chars().peekable(); + let mut right = right.chars().peekable(); + + loop { + // The strings are equal so far and not inside a number in both sides + let (l, r) = match (left.next(), right.next()) { + // Is this the end of both strings? + (None, None) => return Ordering::Equal, + // If for one, the shorter one is considered smaller + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(l), Some(r)) => (l, r), + }; + let next_ordering = match (l.to_digit(10), r.to_digit(10)) { + // If neither is a digit, just compare them + (None, None) => Ord::cmp(&l, &r), + // The one with shorter non-digit run is smaller + // For `strverscmp` it's smaller iff next char in longer is greater than digits + (None, Some(_)) => Ordering::Greater, + (Some(_), None) => Ordering::Less, + // If both start numbers, we have to compare the numbers + (Some(l), Some(r)) => { + if l == 0 || r == 0 { + // Fraction mode: compare as if there was leading `0.` + let ordering = Ord::cmp(&l, &r); + if ordering != Ordering::Equal { + return ordering; + } + loop { + // Get next pair + let (l, r) = match (left.peek(), right.peek()) { + // Is this the end of both strings? + (None, None) => return Ordering::Equal, + // If for one, the shorter one is considered smaller + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(l), Some(r)) => (l, r), + }; + // Are they digits? + match (l.to_digit(10), r.to_digit(10)) { + // If out of digits, use the stored ordering due to equal length + (None, None) => break Ordering::Equal, + // If one is shorter, it's smaller + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + // If both are digits, consume them and take into account + (Some(l), Some(r)) => { + left.next(); + right.next(); + let ordering = Ord::cmp(&l, &r); + if ordering != Ordering::Equal { + return ordering; + } + } + } + } + } else { + // Integer mode + let mut same_length_ordering = Ord::cmp(&l, &r); + loop { + // Get next pair + let (l, r) = match (left.peek(), right.peek()) { + // Is this the end of both strings? + (None, None) => return same_length_ordering, + // If for one, the shorter one is considered smaller + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(l), Some(r)) => (l, r), + }; + // Are they digits? + match (l.to_digit(10), r.to_digit(10)) { + // If out of digits, use the stored ordering due to equal length + (None, None) => break same_length_ordering, + // If one is shorter, it's smaller + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + // If both are digits, consume them and take into account + (Some(l), Some(r)) => { + left.next(); + right.next(); + same_length_ordering = same_length_ordering.then(Ord::cmp(&l, &r)); + } + } + } + } + } + }; + if next_ordering != Ordering::Equal { + return next_ordering; + } + } +} + +/// Compare identifiers, trimming `r#` if present, according to version sort +pub(crate) fn compare_ident_as_versions(left: &str, right: &str) -> Ordering { + compare_as_versions( + left.trim_start_matches("r#"), + right.trim_start_matches("r#"), + ) +} + +pub(crate) fn compare_opt_ident_as_versions(left: &Option, right: &Option) -> Ordering +where + S: AsRef, +{ + match (left, right) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(left), Some(right)) => compare_ident_as_versions(left.as_ref(), right.as_ref()), + } +} + /// Choose the ordering between the given two items. fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { match (&a.kind, &b.kind) { (&ast::ItemKind::Mod(..), &ast::ItemKind::Mod(..)) => { - a.ident.as_str().cmp(&b.ident.as_str()) + compare_as_versions(&a.ident.as_str(), &b.ident.as_str()) } (&ast::ItemKind::ExternCrate(ref a_name), &ast::ItemKind::ExternCrate(ref b_name)) => { // `extern crate foo as bar;` // ^^^ Comparing this. let a_orig_name = a_name.map_or_else(|| a.ident.as_str(), rustc_span::Symbol::as_str); let b_orig_name = b_name.map_or_else(|| b.ident.as_str(), rustc_span::Symbol::as_str); - let result = a_orig_name.cmp(&b_orig_name); + let result = compare_as_versions(&a_orig_name, &b_orig_name); if result != Ordering::Equal { return result; } @@ -44,7 +159,7 @@ fn compare_items(a: &ast::Item, b: &ast::Item) -> Ordering { (Some(..), None) => Ordering::Greater, (None, Some(..)) => Ordering::Less, (None, None) => Ordering::Equal, - (Some(..), Some(..)) => a.ident.as_str().cmp(&b.ident.as_str()), + (Some(..), Some(..)) => compare_as_versions(&a.ident.as_str(), &b.ident.as_str()), } } _ => unreachable!(), @@ -264,3 +379,37 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } } } + +#[cfg(test)] +mod tests { + #[test] + fn test_compare_as_versions() { + use super::compare_as_versions; + use std::cmp::Ordering; + let mut strings: &[&'static str] = &[ + "9", "i8", "ia32", "u009", "u08", "u08", "u080", "u8", "u8", "u16", "u32", "u128", + ]; + while !strings.is_empty() { + let (first, tail) = strings.split_first().unwrap(); + for second in tail { + if first == second { + assert_eq!(compare_as_versions(first, second), Ordering::Equal); + assert_eq!(compare_as_versions(second, first), Ordering::Equal); + } else { + assert_eq!(compare_as_versions(first, second), Ordering::Less); + assert_eq!(compare_as_versions(second, first), Ordering::Greater); + } + } + strings = tail; + } + } + #[test] + fn test_compare_opt_ident_as_versions() { + use super::compare_opt_ident_as_versions; + use std::cmp::Ordering; + let items: &[Option<&'static str>] = &[None, Some("a"), Some("r#a"), Some("a")]; + for (p, n) in items[..items.len() - 1].iter().zip(items[1..].iter()) { + assert!(compare_opt_ident_as_versions(p, n) != Ordering::Greater); + } + } +} diff --git a/rustfmt-core/rustfmt-lib/tests/source/imports-reorder-lines.rs b/rustfmt-core/rustfmt-lib/tests/source/imports-reorder-lines.rs index 2b018544eae..373efd52b46 100644 --- a/rustfmt-core/rustfmt-lib/tests/source/imports-reorder-lines.rs +++ b/rustfmt-core/rustfmt-lib/tests/source/imports-reorder-lines.rs @@ -30,3 +30,14 @@ mod test {} use test::{a as aa, c}; use test::{self as bb, b}; + +#[path = "empty_file.rs"] +mod v10; +#[path = "empty_file.rs"] +mod v2; + +extern crate crate10; +extern crate crate2; + +extern crate my as c10; +extern crate my as c2; diff --git a/rustfmt-core/rustfmt-lib/tests/source/imports-reorder.rs b/rustfmt-core/rustfmt-lib/tests/source/imports-reorder.rs index cbe9d6ca78a..736400c2b04 100644 --- a/rustfmt-core/rustfmt-lib/tests/source/imports-reorder.rs +++ b/rustfmt-core/rustfmt-lib/tests/source/imports-reorder.rs @@ -3,3 +3,9 @@ use path::{C,/*A*/ A, B /* B */, self /* self */}; use {ab, ac, aa, Z, b}; + +// The sort order shall follow versionsort +use {u8, u128, u64, u16, u32}; +use {v1, v0200, v0030, v0002, v02000, v02001}; +// Order by alias should use versionsort too +use {crate as crate10, crate as crate2, crate as crate1}; diff --git a/rustfmt-core/rustfmt-lib/tests/source/issue-2863.rs b/rustfmt-core/rustfmt-lib/tests/source/issue-2863.rs index 1bda857be76..73f4aeedc95 100644 --- a/rustfmt-core/rustfmt-lib/tests/source/issue-2863.rs +++ b/rustfmt-core/rustfmt-lib/tests/source/issue-2863.rs @@ -22,4 +22,6 @@ impl IntoIterator for SafeVec { type E = impl Trait; const AnotherConst: i32 = 100; fn foo8() {println!("hello, world");} + const AnyConst10: i32 = 100; + const AnyConst2: i32 = 100; } diff --git a/rustfmt-core/rustfmt-lib/tests/target/imports-reorder-lines.rs b/rustfmt-core/rustfmt-lib/tests/target/imports-reorder-lines.rs index 5b85503b55d..8e9c6557a13 100644 --- a/rustfmt-core/rustfmt-lib/tests/target/imports-reorder-lines.rs +++ b/rustfmt-core/rustfmt-lib/tests/target/imports-reorder-lines.rs @@ -15,17 +15,28 @@ use aaa::*; mod test {} // If item names are equal, order by rename -use test::{a as bb, b}; use test::{a as aa, c}; +use test::{a as bb, b}; mod test {} // If item names are equal, order by rename - no rename comes before a rename -use test::{a as bb, b}; use test::{a, c}; +use test::{a as bb, b}; mod test {} // `self` always comes first use test::{self as bb, b}; use test::{a as aa, c}; + +#[path = "empty_file.rs"] +mod v2; +#[path = "empty_file.rs"] +mod v10; + +extern crate crate2; +extern crate crate10; + +extern crate my as c2; +extern crate my as c10; diff --git a/rustfmt-core/rustfmt-lib/tests/target/imports-reorder.rs b/rustfmt-core/rustfmt-lib/tests/target/imports-reorder.rs index 84e97c0224f..4e711431fb2 100644 --- a/rustfmt-core/rustfmt-lib/tests/target/imports-reorder.rs +++ b/rustfmt-core/rustfmt-lib/tests/target/imports-reorder.rs @@ -3,3 +3,9 @@ use path::{self /* self */, /* A */ A, B /* B */, C}; use {aa, ab, ac, b, Z}; + +// The sort order shall follow versionsort +use {u8, u16, u32, u64, u128}; +use {v0002, v0030, v0200, v02000, v02001, v1}; +// Order by alias should use versionsort too +use {crate as crate1, crate as crate2, crate as crate10}; diff --git a/rustfmt-core/rustfmt-lib/tests/target/issue-2863.rs b/rustfmt-core/rustfmt-lib/tests/target/issue-2863.rs index 35a80f7a621..67c14d6272e 100644 --- a/rustfmt-core/rustfmt-lib/tests/target/issue-2863.rs +++ b/rustfmt-core/rustfmt-lib/tests/target/issue-2863.rs @@ -13,6 +13,8 @@ impl IntoIterator for SafeVec { type F = impl Trait; const AnotherConst: i32 = 100; + const AnyConst2: i32 = 100; + const AnyConst10: i32 = 100; const SomeConst: i32 = 100; // comment on foo()