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::symbol::Ident;
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:**
///
};
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
- }))
+ return Some(LintTrigger::Sort(SortDetection { vec_name, unstable }))
+ } else {
+ if !key_returns_borrow(cx, left_expr) {
+ return Some(LintTrigger::SortByKey(SortByKeyDetection {
+ vec_name,
+ unstable,
+ closure_arg,
+ closure_body,
+ reverse
+ }))
+ }
}
}
- } else {
- None
}
}
+
+ None
+}
+
+fn key_returns_borrow(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+ if let Some(def_id) = utils::fn_def_id(cx, expr) {
+ let output = cx.tcx.fn_sig(def_id).output();
+ let ty = output.skip_binder();
+ return matches!(ty.kind, ty::Ref(..))
+ || ty.walk().any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
+ }
+
+ false
}
impl LateLintPass<'_> for UnnecessarySortBy {