Rollup merge of #74196 - GuillaumeGomez:auto-collapse-implementors, r=Manishearth

[rust.git] / src / tools / clippy / clippy_lints / src / unnecessary_sort_by.rs

diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs
index bb68e50b331953b0f71b847feeb3916ccc66e4d3..91c1789a2ffb1b592da5b6c06132962fa074aed1 100644 (file)
--- a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs
+++ b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs
@@ -5,24 +5,23 @@
 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:**
     ///
@@ -67,7 +66,7 @@ struct SortByKeyDetection {
 /// Detect if the two expressions are mirrored (identical, except one
 /// contains a and the other replaces it with b)
 fn mirrored_exprs(
-    cx: &LateContext<'_, '_>,
+    cx: &LateContext<'_>,
     a_expr: &Expr<'_>,
     a_ident: &Ident,
     b_expr: &Expr<'_>,
@@ -171,7 +170,7 @@ fn mirrored_exprs(
     }
 }
 
-fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<LintTrigger> {
+fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> {
     if_chain! {
         if let ExprKind::MethodCall(name_ident, _, args, _) = &expr.kind;
         if let name = name_ident.ident.name.to_ident_string();
@@ -201,32 +200,45 @@ fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<LintTrigger>
             };
             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 {
-    fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+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(
                 cx,