X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Funnecessary_sort_by.rs;h=6becff9662a76ff79b74e191baf506b7788ee73b;hb=b094bb1bd7f1cc702823c91ca509f338fedee24a;hp=d940776817ca078453e21d6a5aa46d9814bafdb4;hpb=245b006a2e9453307443356d511e9e89967cfef9;p=rust.git diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/unnecessary_sort_by.rs index d940776817c..6becff9662a 100644 --- a/clippy_lints/src/unnecessary_sort_by.rs +++ b/clippy_lints/src/unnecessary_sort_by.rs @@ -1,28 +1,29 @@ -use crate::utils; -use crate::utils::paths; -use crate::utils::sugg::Sugg; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::sugg::Sugg; +use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, subst::GenericArgKind}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; use rustc_span::symbol::Ident; +use std::iter; declare_clippy_lint! { /// **What it does:** - /// Detects when people use `Vec::sort_by` and pass in a function + /// Detects uses of `Vec::sort_by` passing in a closure /// which compares the two arguments, either directly or indirectly. /// /// **Why is this bad?** /// It is more clear to use `Vec::sort_by_key` (or `Vec::sort` if - /// possible) than to use `Vec::sort_by` and and a more complicated + /// possible) than to use `Vec::sort_by` and a more complicated /// closure. /// /// **Known problems:** - /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't - /// imported by a use statement in the current frame, then a `use` - /// statement that imports it will need to be added (which this lint - /// can't do). + /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't already + /// imported by a use statement, then it will need to be added manually. /// /// **Example:** /// @@ -79,17 +80,15 @@ fn mirrored_exprs( mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) }, // Two arrays with mirrored contents - (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => left_exprs - .iter() - .zip(right_exprs.iter()) - .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)), + (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => { + iter::zip(*left_exprs, *right_exprs) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + } // The two exprs are function calls. // Check to see that the function itself and its arguments are mirrored (ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args)) => { mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) - && left_args - .iter() - .zip(right_args.iter()) + && iter::zip(*left_args, *right_args) .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) }, // The two exprs are method calls. @@ -100,16 +99,14 @@ fn mirrored_exprs( ExprKind::MethodCall(right_segment, _, right_args, _), ) => { left_segment.ident == right_segment.ident - && left_args - .iter() - .zip(right_args.iter()) + && iter::zip(*left_args, *right_args) .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) - }, + } // Two tuples with mirrored contents - (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => left_exprs - .iter() - .zip(right_exprs.iter()) - .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)), + (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => { + iter::zip(*left_exprs, *right_exprs) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + } // Two binary ops, which are the same operation and which have mirrored arguments (ExprKind::Binary(left_op, left_left, left_right), ExprKind::Binary(right_op, right_left, right_right)) => { left_op.node == right_op.node @@ -146,9 +143,7 @@ fn mirrored_exprs( }, )), ) => { - (left_segments - .iter() - .zip(right_segments.iter()) + (iter::zip(*left_segments, *right_segments) .all(|(left, right)| left.ident == right.ident) && left_segments .iter() @@ -177,14 +172,14 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { if let name = name_ident.ident.name.to_ident_string(); if name == "sort_by" || name == "sort_unstable_by"; if let [vec, Expr { kind: ExprKind::Closure(_, _, closure_body_id, _, _), .. }] = args; - if utils::match_type(cx, &cx.tables().expr_ty(vec), &paths::VEC); + if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(vec), sym::vec_type); if let closure_body = cx.tcx.hir().body(*closure_body_id); if let &[ Param { pat: Pat { kind: PatKind::Binding(_, _, left_ident, _), .. }, ..}, Param { pat: Pat { kind: PatKind::Binding(_, _, right_ident, _), .. }, .. } ] = &closure_body.params; if let ExprKind::MethodCall(method_path, _, [ref left_expr, ref right_expr], _) = &closure_body.value.kind; - if method_path.ident.name.to_ident_string() == "cmp"; + if method_path.ident.name == sym::cmp; then { let (closure_body, closure_arg, reverse) = if mirrored_exprs( &cx, @@ -201,41 +196,46 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { }; let vec_name = Sugg::hir(cx, &args[0], "..").to_string(); let unstable = name == "sort_unstable_by"; - if_chain! { - if let ExprKind::Path(QPath::Resolved(_, Path { - segments: [PathSegment { ident: left_name, .. }], .. - })) = &left_expr.kind; - if left_name == left_ident; - then { - Some(LintTrigger::Sort(SortDetection { vec_name, unstable })) - } - else { - Some(LintTrigger::SortByKey(SortByKeyDetection { - vec_name, - unstable, - closure_arg, - closure_body, - reverse - })) + + if let ExprKind::Path(QPath::Resolved(_, Path { + segments: [PathSegment { ident: left_name, .. }], .. + })) = &left_expr.kind { + if left_name == left_ident { + return Some(LintTrigger::Sort(SortDetection { vec_name, unstable })); } } - } else { - None + + if !expr_borrows(cx, left_expr) { + return Some(LintTrigger::SortByKey(SortByKeyDetection { + vec_name, + closure_arg, + closure_body, + reverse, + unstable, + })); + } } } + + None +} + +fn expr_borrows(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let ty = cx.typeck_results().expr_ty(expr); + matches!(ty.kind(), ty::Ref(..)) || ty.walk().any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))) } impl LateLintPass<'_> for UnnecessarySortBy { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { match detect_lint(cx, expr) { - Some(LintTrigger::SortByKey(trigger)) => utils::span_lint_and_sugg( + Some(LintTrigger::SortByKey(trigger)) => span_lint_and_sugg( cx, UNNECESSARY_SORT_BY, expr.span, "use Vec::sort_by_key here instead", "try", format!( - "{}.sort{}_by_key(|&{}| {})", + "{}.sort{}_by_key(|{}| {})", trigger.vec_name, if trigger.unstable { "_unstable" } else { "" }, trigger.closure_arg, @@ -251,7 +251,7 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { Applicability::MachineApplicable }, ), - Some(LintTrigger::Sort(trigger)) => utils::span_lint_and_sugg( + Some(LintTrigger::Sort(trigger)) => span_lint_and_sugg( cx, UNNECESSARY_SORT_BY, expr.span,