-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:**
///
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.
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
},
)),
) => {
- (left_segments
- .iter()
- .zip(right_segments.iter())
+ (iter::zip(*left_segments, *right_segments)
.all(|(left, right)| left.ident == right.ident)
&& left_segments
.iter()
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,
};
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,
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,
projects / rust.git / blobdiff