Skip to content

Commit a57ce88

Browse files
Noratriebytmimi
authored andcommitted
Fix use <2024 identifier sorting transitivity
The sorting tries to create a lowercase>Capital>SCREAMING relation, but does this in a way that causes non-transitive behavior for identifiers starting with `_`. These identifiers are neither Capital nor SCREAMING so they are always compared via lexicographic ASCII matching. This causes a problem, because `A < _ < a` lexicographically in ASCII. But `A > a` with our special case rules! When doing these manual comparisons, hitting these issues is very easy. To fix this, we can simply create a sorting key that mirrors our desired order and sort that normally. This way, it's guaranteed to have all the necessary properties. By *always* sorting uppercase identifiers first, we ensure that `A > _`.
1 parent 2d2bb53 commit a57ce88

5 files changed

Lines changed: 62 additions & 23 deletions

File tree

src/imports.rs

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -945,20 +945,16 @@ impl Ord for UseSegment {
945945
let ident_ord = if self.style_edition >= StyleEdition::Edition2024 {
946946
version_sort(ia, ib)
947947
} else {
948-
// snake_case < CamelCase < UPPER_SNAKE_CASE
949-
if ia.starts_with(char::is_uppercase) && !ib.starts_with(char::is_uppercase) {
950-
return Ordering::Greater;
948+
fn sorting_key(ident: &str) -> (bool, bool, &str) {
949+
// snake_case < CamelCase < UPPER_SNAKE_CASE
950+
(
951+
is_upper_snake_case(ident),
952+
ident.starts_with(char::is_uppercase),
953+
ident,
954+
)
951955
}
952-
if !ia.starts_with(char::is_uppercase) && ib.starts_with(char::is_uppercase) {
953-
return Ordering::Less;
954-
}
955-
if is_upper_snake_case(ia) && !is_upper_snake_case(ib) {
956-
return Ordering::Greater;
957-
}
958-
if !is_upper_snake_case(ia) && is_upper_snake_case(ib) {
959-
return Ordering::Less;
960-
}
961-
ia.cmp(ib)
956+
957+
sorting_key(ia).cmp(&sorting_key(ib))
962958
};
963959

964960
if ident_ord != Ordering::Equal {
@@ -977,16 +973,7 @@ impl Ord for UseSegment {
977973
(None, None) => Ordering::Equal,
978974
}
979975
}
980-
(List(ref a), List(ref b)) => {
981-
for (a, b) in a.iter().zip(b.iter()) {
982-
let ord = a.cmp(b);
983-
if ord != Ordering::Equal {
984-
return ord;
985-
}
986-
}
987-
988-
a.len().cmp(&b.len())
989-
}
976+
(List(ref a), List(ref b)) => a.iter().cmp(b.iter()),
990977
(Slf(_), _) => Ordering::Less,
991978
(_, Slf(_)) => Ordering::Greater,
992979
(Super(_), _) => Ordering::Less,
@@ -1518,6 +1505,10 @@ mod test {
15181505
assert!(parse_use_tree("a").normalize() < parse_use_tree("{a, b}").normalize());
15191506
assert!(parse_use_tree("*").normalize() < parse_use_tree("{a, b}").normalize());
15201507

1508+
assert!(parse_use_tree("A").normalize() > parse_use_tree("a").normalize());
1509+
assert!(parse_use_tree("A").normalize() > parse_use_tree("_b").normalize());
1510+
assert!(parse_use_tree("a").normalize() > parse_use_tree("_b").normalize());
1511+
15211512
assert!(
15221513
parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, dddddddd}").normalize()
15231514
< parse_use_tree("aaaaaaaaaaaaaaa::{bb, cc, ddddddddd}").normalize()
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// rustfmt-edition: 2024
2+
// rustfmt-style_edition: 2021
3+
// rustfmt-reorder_imports: true
4+
5+
use a::{
6+
Big, small, t_i_n_y, HUGE, SCREAMING_LOUDLY, or_quietly, OrInTheMiddle
7+
};
8+
9+
use a::{
10+
_bind_this_one_has_underscore, A1, A2, A3, A4, A5, A6, A7, A9, UnescapeOptions, bind_11,
11+
bind_12, build_13, check_14, check_15, coerce_16, construct_17, construct_18, create_19, df_20,
12+
expand_21,
13+
};
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// rustfmt-edition: 2021
2+
// rustfmt-style_edition: 2021
3+
// rustfmt-reorder_imports: true
4+
5+
use a::{
6+
Big, small, t_i_n_y, HUGE, SCREAMING_LOUDLY, or_quietly, OrInTheMiddle
7+
};
8+
9+
use a::{
10+
_bind_this_one_has_underscore, A1, A2, A3, A4, A5, A6, A7, A9, UnescapeOptions, bind_11,
11+
bind_12, build_13, check_14, check_15, coerce_16, construct_17, construct_18, create_19, df_20,
12+
expand_21,
13+
};
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// rustfmt-edition: 2024
2+
// rustfmt-style_edition: 2021
3+
// rustfmt-reorder_imports: true
4+
5+
use a::{or_quietly, small, t_i_n_y, Big, OrInTheMiddle, HUGE, SCREAMING_LOUDLY};
6+
7+
use a::{
8+
_bind_this_one_has_underscore, bind_11, bind_12, build_13, check_14, check_15, coerce_16,
9+
construct_17, construct_18, create_19, df_20, expand_21, UnescapeOptions, A1, A2, A3, A4, A5,
10+
A6, A7, A9,
11+
};
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// rustfmt-edition: 2021
2+
// rustfmt-style_edition: 2021
3+
// rustfmt-reorder_imports: true
4+
5+
use a::{or_quietly, small, t_i_n_y, Big, OrInTheMiddle, HUGE, SCREAMING_LOUDLY};
6+
7+
use a::{
8+
_bind_this_one_has_underscore, bind_11, bind_12, build_13, check_14, check_15, coerce_16,
9+
construct_17, construct_18, create_19, df_20, expand_21, UnescapeOptions, A1, A2, A3, A4, A5,
10+
A6, A7, A9,
11+
};

0 commit comments

Comments
 (0)