Skip to content

Commit 943cb94

Browse files
JarredAllenJarredAllen
committed
Passes all tests now!
1 parent 8590ab4 commit 943cb94

File tree

4 files changed

+45
-61
lines changed

4 files changed

+45
-61
lines changed

‎clippy_lints/src/sort_by_key_reverse.rs

Lines changed: 22 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,12 @@ fn mirrored_exprs(cx: &LateContext<'_, '_>, a_expr: &Expr<'_>, a_ident: &Ident,
5353
// The two exprs are function calls.
5454
// Check to see that the function itself and its arguments are mirrored
5555
(ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args))
56-
=> mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
57-
&& left_args.iter().zip(right_args.iter()).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)),
56+
=> {
57+
// println!("{:?}\n{:?}\n", left_expr, left_args);
58+
// println!("{:?}\n{:?}\n", right_expr, right_args);
59+
mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
60+
&& left_args.iter().zip(right_args.iter()).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
61+
},
5862
// The two exprs are method calls.
5963
// Check to see that the function is the same and the arguments are mirrored
6064
// This is enough because the receiver of the method is listed in the arguments
@@ -74,65 +78,29 @@ fn mirrored_exprs(cx: &LateContext<'_, '_>, a_expr: &Expr<'_>, a_ident: &Ident,
7478
=> left_op == right_op && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
7579
// The two exprs are literals of some kind
7680
(ExprKind::Lit(left_lit), ExprKind::Lit(right_lit)) => left_lit.node == right_lit.node,
77-
(ExprKind::Cast(left_expr, _), ExprKind::Cast(right_expr, _))
81+
(ExprKind::Cast(left_expr, left_ty), ExprKind::Cast(right_expr, right_ty))
7882
=> mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
7983
(ExprKind::DropTemps(left), ExprKind::DropTemps(right)) => mirrored_exprs(cx, left, a_ident, right, b_ident),
80-
(ExprKind::Block(left, _), ExprKind::Block(right, _)) => mirrored_blocks(cx, left, a_ident, right, b_ident),
8184
(ExprKind::Field(left_expr, left_ident), ExprKind::Field(right_expr, right_ident))
8285
=> left_ident.name == right_ident.name && mirrored_exprs(cx, left_expr, a_ident, right_expr, right_ident),
83-
// The two exprs are `a` and `b`, directly
84-
(ExprKind::Path(QPath::Resolved(_, Path { segments: &[PathSegment { ident: left_ident, .. }], .. },)),
85-
ExprKind::Path(QPath::Resolved(_, Path { segments: &[PathSegment { ident: right_ident, .. }], .. },)),
86-
) => &left_ident == a_ident && &right_ident == b_ident,
87-
// The two exprs are Paths to the same name (which is neither a nor b)
86+
// Two paths: either one is a and the other is b, or they're identical to each other
8887
(ExprKind::Path(QPath::Resolved(_, Path { segments: left_segments, .. })),
8988
ExprKind::Path(QPath::Resolved(_, Path { segments: right_segments, .. })))
90-
=> left_segments.iter().zip(right_segments.iter()).all(|(left, right)| left.ident == right.ident)
91-
&& left_segments.iter().all(|seg| &seg.ident != a_ident && &seg.ident != b_ident),
89+
=> (left_segments.iter().zip(right_segments.iter()).all(|(left, right)| left.ident == right.ident)
90+
&& left_segments.iter().all(|seg| &seg.ident != a_ident && &seg.ident != b_ident))
91+
|| (left_segments.len() == 1 && &left_segments[0].ident == a_ident && right_segments.len() == 1 && &right_segments[0].ident == b_ident),
9292
// Matching expressions, but one or both is borrowed
9393
(ExprKind::AddrOf(left_kind, Mutability::Not, left_expr), ExprKind::AddrOf(right_kind, Mutability::Not, right_expr))
9494
=> left_kind == right_kind && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
9595
(_, ExprKind::AddrOf(_, Mutability::Not, right_expr))
9696
=> mirrored_exprs(cx, a_expr, a_ident, right_expr, b_ident),
9797
(ExprKind::AddrOf(_, Mutability::Not, left_expr), _)
9898
=> mirrored_exprs(cx, left_expr, a_ident, b_expr, b_ident),
99-
// _ => false,
100-
(left, right) => {
101-
println!("{:?}\n{:?}", left, right);
102-
false
103-
},
104-
}
105-
}
106-
107-
/// Detect if the two blocks are mirrored (identical, except one
108-
/// contains a and the other replaces it with b)
109-
fn mirrored_blocks(cx: &LateContext<'_, '_>, a_block: &Block<'_>, a_ident: &Ident, b_block: &Block<'_>, b_ident: &Ident) -> bool {
110-
match (a_block, b_block) {
111-
(Block { stmts: left_stmts, expr: left_expr, .. },
112-
Block { stmts: right_stmts, expr: right_expr, .. })
113-
=> left_stmts.iter().zip(right_stmts.iter()).all(|(left, right)| match (&left.kind, &right.kind) {
114-
(StmtKind::Expr(left_expr), StmtKind::Expr(right_expr)) => mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
115-
(StmtKind::Semi(left_expr), StmtKind::Semi(right_expr)) => mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
116-
(StmtKind::Item(left_item), StmtKind::Item(right_item)) => left_item.id == right_item.id,
117-
(StmtKind::Local(left), StmtKind::Local(right)) => mirrored_locals(cx, left, a_ident, right, b_ident),
118-
_ => false,
119-
}) && match (left_expr, right_expr) {
120-
(None, None) => true,
121-
(Some(left), Some(right)) => mirrored_exprs(cx, left, a_ident, right, b_ident),
122-
_ => false,
123-
},
124-
}
125-
}
126-
127-
/// Check that the two "Local"s (let statements) are equal
128-
fn mirrored_locals(cx: &LateContext<'_, '_>, a_local: &Local<'_>, a_ident: &Ident, b_local: &Local<'_>, b_ident: &Ident) -> bool {
129-
match (a_local, b_local) {
130-
(Local { pat: left_pat, init: left_expr, .. }, Local { pat: right_pat, init: right_expr, .. })
131-
=> match (left_expr, right_expr) {
132-
(None, None) => true,
133-
(Some(left), Some(right)) => mirrored_exprs(cx, left, a_ident, right, b_ident),
134-
_ => false,
135-
},
99+
_ => false,
100+
// (left, right) => {
101+
// println!("{:?}\n{:?}", left, right);
102+
// false
103+
// },
136104
}
137105
}
138106

@@ -154,8 +122,12 @@ fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<LintTrigger>
154122
then {
155123
let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
156124
let unstable = name == "sort_unstable_by";
157-
let closure_arg = a_ident.name.to_ident_string();
158-
let closure_reverse_body = Sugg::hir(cx, &a_expr, "..").to_string();
125+
let closure_arg = format!("&{}", b_ident.name.to_ident_string());
126+
let closure_reverse_body = Sugg::hir(cx, &b_expr, "..").to_string();
127+
// Get rid of parentheses, because they aren't needed anymore
128+
// while closure_reverse_body.chars().next() == Some('(') && closure_reverse_body.chars().last() == Some(')') {
129+
// closure_reverse_body = String::from(&closure_reverse_body[1..closure_reverse_body.len()-1]);
130+
// }
159131
Some(LintTrigger { vec_name, unstable, closure_arg, closure_reverse_body })
160132
} else {
161133
None

‎tests/ui/sort_by_key_reverse.fixed

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
// run-rustfix
22
#![warn(clippy::sort_by_key_reverse)]
33

4+
use std::cmp::Reverse;
5+
6+
fn id(x: isize) -> isize {
7+
x
8+
}
9+
410
fn main() {
511
let mut vec: Vec<isize> = vec![3, 6, 1, 2, 5];
6-
vec.sort_by_key(|a| Reverse(a));
7-
vec.sort_by_key(|a| Reverse(&(a+5).abs()));
8-
vec.sort_by_key(|a| Reverse(&-a));
12+
vec.sort_by_key(|&b| Reverse(b));
13+
vec.sort_by_key(|&b| Reverse((b + 5).abs()));
14+
vec.sort_by_key(|&b| Reverse(id(-b)));
915
}

‎tests/ui/sort_by_key_reverse.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
// run-rustfix
22
#![warn(clippy::sort_by_key_reverse)]
33

4+
use std::cmp::Reverse;
5+
6+
fn id(x: isize) -> isize {
7+
x
8+
}
9+
410
fn main() {
511
let mut vec: Vec<isize> = vec![3, 6, 1, 2, 5];
612
vec.sort_by(|a, b| b.cmp(a));
713
vec.sort_by(|a, b| (b + 5).abs().cmp(&(a+5).abs()));
8-
vec.sort_by(|a, b| (-b).cmp(&-a));
14+
vec.sort_by(|a, b| id(-b).cmp(&id(-a)));
915
}

‎tests/ui/sort_by_key_reverse.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
error: use Vec::sort_by_key here instead
2-
--> $DIR/sort_by_key_reverse.rs:6:5
2+
--> $DIR/sort_by_key_reverse.rs:12:5
33
|
44
LL | vec.sort_by(|a, b| b.cmp(a));
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| Reverse(a))`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse(b))`
66
|
77
= note: `-D clippy::sort-by-key-reverse` implied by `-D warnings`
88

99
error: use Vec::sort_by_key here instead
10-
--> $DIR/sort_by_key_reverse.rs:7:5
10+
--> $DIR/sort_by_key_reverse.rs:13:5
1111
|
1212
LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a+5).abs()));
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| Reverse(&(a+5).abs()))`
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse((b + 5).abs()))`
1414

1515
error: use Vec::sort_by_key here instead
16-
--> $DIR/sort_by_key_reverse.rs:8:5
16+
--> $DIR/sort_by_key_reverse.rs:14:5
1717
|
18-
LL | vec.sort_by(|a, b| (-b).cmp(&-a));
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| Reverse(&-a))`
18+
LL | vec.sort_by(|a, b| id(-b).cmp(&id(-a)));
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse(id(-b)))`
2020

2121
error: aborting due to 3 previous errors
2222

0 commit comments

Comments
 (0)