2 use crate::utils::paths;
3 use crate::utils::sugg::Sugg;
4 use if_chain::if_chain;
5 use rustc_errors::Applicability;
6 use rustc_hir::{Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
7 use rustc_lint::{LateContext, LateLintPass};
8 use rustc_middle::ty::{self, subst::GenericArgKind};
9 use rustc_session::{declare_lint_pass, declare_tool_lint};
10 use rustc_span::symbol::Ident;
12 declare_clippy_lint! {
14 /// Detects uses of `Vec::sort_by` passing in a closure
15 /// which compares the two arguments, either directly or indirectly.
17 /// **Why is this bad?**
18 /// It is more clear to use `Vec::sort_by_key` (or `Vec::sort` if
19 /// possible) than to use `Vec::sort_by` and a more complicated
22 /// **Known problems:**
23 /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't already
24 /// imported by a use statement, then it will need to be added manually.
30 /// # impl A { fn foo(&self) {} }
31 /// # let mut vec: Vec<A> = Vec::new();
32 /// vec.sort_by(|a, b| a.foo().cmp(&b.foo()));
37 /// # impl A { fn foo(&self) {} }
38 /// # let mut vec: Vec<A> = Vec::new();
39 /// vec.sort_by_key(|a| a.foo());
41 pub UNNECESSARY_SORT_BY,
43 "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer"
46 declare_lint_pass!(UnnecessarySortBy => [UNNECESSARY_SORT_BY]);
50 SortByKey(SortByKeyDetection),
53 struct SortDetection {
58 struct SortByKeyDetection {
66 /// Detect if the two expressions are mirrored (identical, except one
67 /// contains a and the other replaces it with b)
75 match (&a_expr.kind, &b_expr.kind) {
76 // Two boxes with mirrored contents
77 (ExprKind::Box(left_expr), ExprKind::Box(right_expr)) => {
78 mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
80 // Two arrays with mirrored contents
81 (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => left_exprs
83 .zip(right_exprs.iter())
84 .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)),
85 // The two exprs are function calls.
86 // Check to see that the function itself and its arguments are mirrored
87 (ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args)) => {
88 mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
91 .zip(right_args.iter())
92 .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
94 // The two exprs are method calls.
95 // Check to see that the function is the same and the arguments are mirrored
96 // This is enough because the receiver of the method is listed in the arguments
98 ExprKind::MethodCall(left_segment, _, left_args, _),
99 ExprKind::MethodCall(right_segment, _, right_args, _),
101 left_segment.ident == right_segment.ident
104 .zip(right_args.iter())
105 .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
107 // Two tuples with mirrored contents
108 (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => left_exprs
110 .zip(right_exprs.iter())
111 .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)),
112 // Two binary ops, which are the same operation and which have mirrored arguments
113 (ExprKind::Binary(left_op, left_left, left_right), ExprKind::Binary(right_op, right_left, right_right)) => {
114 left_op.node == right_op.node
115 && mirrored_exprs(cx, left_left, a_ident, right_left, b_ident)
116 && mirrored_exprs(cx, left_right, a_ident, right_right, b_ident)
118 // Two unary ops, which are the same operation and which have the same argument
119 (ExprKind::Unary(left_op, left_expr), ExprKind::Unary(right_op, right_expr)) => {
120 left_op == right_op && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
122 // The two exprs are literals of some kind
123 (ExprKind::Lit(left_lit), ExprKind::Lit(right_lit)) => left_lit.node == right_lit.node,
124 (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(cx, left, a_ident, right, b_ident),
125 (ExprKind::DropTemps(left_block), ExprKind::DropTemps(right_block)) => {
126 mirrored_exprs(cx, left_block, a_ident, right_block, b_ident)
128 (ExprKind::Field(left_expr, left_ident), ExprKind::Field(right_expr, right_ident)) => {
129 left_ident.name == right_ident.name && mirrored_exprs(cx, left_expr, a_ident, right_expr, right_ident)
131 // Two paths: either one is a and the other is b, or they're identical to each other
133 ExprKind::Path(QPath::Resolved(
136 segments: left_segments,
140 ExprKind::Path(QPath::Resolved(
143 segments: right_segments,
150 .zip(right_segments.iter())
151 .all(|(left, right)| left.ident == right.ident)
154 .all(|seg| &seg.ident != a_ident && &seg.ident != b_ident))
155 || (left_segments.len() == 1
156 && &left_segments[0].ident == a_ident
157 && right_segments.len() == 1
158 && &right_segments[0].ident == b_ident)
160 // Matching expressions, but one or both is borrowed
162 ExprKind::AddrOf(left_kind, Mutability::Not, left_expr),
163 ExprKind::AddrOf(right_kind, Mutability::Not, right_expr),
164 ) => left_kind == right_kind && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
165 (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => {
166 mirrored_exprs(cx, a_expr, a_ident, right_expr, b_ident)
168 (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(cx, left_expr, a_ident, b_expr, b_ident),
173 fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> {
175 if let ExprKind::MethodCall(name_ident, _, args, _) = &expr.kind;
176 if let name = name_ident.ident.name.to_ident_string();
177 if name == "sort_by" || name == "sort_unstable_by";
178 if let [vec, Expr { kind: ExprKind::Closure(_, _, closure_body_id, _, _), .. }] = args;
179 if utils::match_type(cx, &cx.typeck_results().expr_ty(vec), &paths::VEC);
180 if let closure_body = cx.tcx.hir().body(*closure_body_id);
182 Param { pat: Pat { kind: PatKind::Binding(_, _, left_ident, _), .. }, ..},
183 Param { pat: Pat { kind: PatKind::Binding(_, _, right_ident, _), .. }, .. }
184 ] = &closure_body.params;
185 if let ExprKind::MethodCall(method_path, _, [ref left_expr, ref right_expr], _) = &closure_body.value.kind;
186 if method_path.ident.name.to_ident_string() == "cmp";
188 let (closure_body, closure_arg, reverse) = if mirrored_exprs(
195 (Sugg::hir(cx, &left_expr, "..").to_string(), left_ident.name.to_string(), false)
196 } else if mirrored_exprs(&cx, &left_expr, &right_ident, &right_expr, &left_ident) {
197 (Sugg::hir(cx, &left_expr, "..").to_string(), right_ident.name.to_string(), true)
201 let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
202 let unstable = name == "sort_unstable_by";
205 if let ExprKind::Path(QPath::Resolved(_, Path {
206 segments: [PathSegment { ident: left_name, .. }], ..
207 })) = &left_expr.kind;
208 if left_name == left_ident;
210 return Some(LintTrigger::Sort(SortDetection { vec_name, unstable }))
212 if !key_returns_borrow(cx, left_expr) {
213 return Some(LintTrigger::SortByKey(SortByKeyDetection {
229 fn key_returns_borrow(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
230 if let Some(def_id) = utils::fn_def_id(cx, expr) {
231 let output = cx.tcx.fn_sig(def_id).output();
232 let ty = output.skip_binder();
233 return matches!(ty.kind, ty::Ref(..))
234 || ty.walk().any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
240 impl LateLintPass<'_> for UnnecessarySortBy {
241 fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
242 match detect_lint(cx, expr) {
243 Some(LintTrigger::SortByKey(trigger)) => utils::span_lint_and_sugg(
247 "use Vec::sort_by_key here instead",
250 "{}.sort{}_by_key(|&{}| {})",
252 if trigger.unstable { "_unstable" } else { "" },
255 format!("Reverse({})", trigger.closure_body)
257 trigger.closure_body.to_string()
261 Applicability::MaybeIncorrect
263 Applicability::MachineApplicable
266 Some(LintTrigger::Sort(trigger)) => utils::span_lint_and_sugg(
270 "use Vec::sort here instead",
275 if trigger.unstable { "_unstable" } else { "" },
277 Applicability::MachineApplicable,