From 24847ea53e332853597aca2c7dfe48a9f3be1de8 Mon Sep 17 00:00:00 2001
From: JarredAllen <jarredallen73@gmail.com>
Date: Sat, 16 May 2020 13:50:33 -0700
Subject: [PATCH] Attempted start at sort_by_key_reverse lint

---
 clippy_lints/src/sort_by_key_reverse.rs | 71 +++++++++++++++++++++++--
 src/lintlist/mod.rs                     |  2 +-
 tests/ui/sort_by_key_reverse.fixed      |  0
 tests/ui/sort_by_key_reverse.rs         |  3 +-
 tests/ui/sort_by_key_reverse.stderr     |  0
 5 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100644 tests/ui/sort_by_key_reverse.fixed
 create mode 100644 tests/ui/sort_by_key_reverse.stderr

diff --git a/clippy_lints/src/sort_by_key_reverse.rs b/clippy_lints/src/sort_by_key_reverse.rs
index 65830afd0f8d..7d7097a81253 100644
--- a/clippy_lints/src/sort_by_key_reverse.rs
+++ b/clippy_lints/src/sort_by_key_reverse.rs
@@ -1,28 +1,91 @@
+use crate::utils::{match_type, span_lint_and_sugg};
+use crate::utils::paths;
+use crate::utils::sugg::Sugg;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
 use rustc_lint::{LateLintPass, LateContext};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_hir::*;
 
 declare_clippy_lint! {
     /// **What it does:**
+    /// Detects when people use `Vec::sort_by` and pass in a function
+    /// which compares the second argument to the first.
     ///
     /// **Why is this bad?**
+    /// It is more clear to use `Vec::sort_by_key` and `std::cmp::Reverse`
     ///
     /// **Known problems:** None.
     ///
     /// **Example:**
     ///
     /// ```rust
-    /// // example code where clippy issues a warning
+    /// vec.sort_by(|a, b| b.foo().cmp(&a.foo()));
     /// ```
     /// Use instead:
     /// ```rust
-    /// // example code which does not raise clippy warning
+    /// vec.sort_by_key(|e| Reverse(e.foo()));
     /// ```
     pub SORT_BY_KEY_REVERSE,
     complexity,
-    "default lint description"
+    "Use of `Vec::sort_by` when `Vec::sort_by_key` would be clearer"
 }
 
 declare_lint_pass!(SortByKeyReverse => [SORT_BY_KEY_REVERSE]);
 
-impl LateLintPass<'_, '_> for SortByKeyReverse {}
+struct LintTrigger {
+    vec_name: String,
+    closure_arg: String,
+    closure_reverse_body: String,
+    unstable: bool,
+}
+
+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();
+        if name == "sort_by" || name == "sort_unstable_by";
+        if let [vec, Expr { kind: ExprKind::Closure(_, closure_decl, closure_body_id, _, _), .. }] = args;
+        if closure_decl.inputs.len() == 2;
+        if match_type(cx, &cx.tables.expr_ty(vec), &paths::VEC);
+        then {
+            let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
+            let unstable = name == "sort_unstable_by";
+            Some(LintTrigger { vec_name, unstable, closure_arg: "e".to_string(), closure_reverse_body: "e".to_string() })
+        } else {
+            None
+        }
+    }
+}
+
+impl LateLintPass<'_, '_> for SortByKeyReverse {
+    fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+        println!("{:?}", expr);
+        span_lint_and_sugg(
+            cx,
+            SORT_BY_KEY_REVERSE,
+            expr.span,
+            "use Vec::sort_by_key here instead",
+            "try",
+            String::from("being a better person"),
+            Applicability::MachineApplicable,
+        );
+        if let Some(trigger) = detect_lint(cx, expr) {
+            span_lint_and_sugg(
+                cx,
+                SORT_BY_KEY_REVERSE,
+                expr.span,
+                "use Vec::sort_by_key here instead",
+                "try",
+                format!(
+                    "{}.sort{}_by_key(|{}| Reverse({}))",
+                    trigger.vec_name,
+                    if trigger.unstable { "_unstable" } else { "" },
+                    trigger.closure_arg,
+                    trigger.closure_reverse_body,
+                ),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 1b82f34c8634..b5d9ef0110e2 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1987,7 +1987,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
     Lint {
         name: "sort_by_key_reverse",
         group: "complexity",
-        desc: "default lint description",
+        desc: "Use of `Vec::sort_by` when `Vec::sort_by_key` would be clearer",
         deprecation: None,
         module: "sort_by_key_reverse",
     },
diff --git a/tests/ui/sort_by_key_reverse.fixed b/tests/ui/sort_by_key_reverse.fixed
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/ui/sort_by_key_reverse.rs b/tests/ui/sort_by_key_reverse.rs
index 2338dc6e5940..c0350f243c7b 100644
--- a/tests/ui/sort_by_key_reverse.rs
+++ b/tests/ui/sort_by_key_reverse.rs
@@ -1,5 +1,6 @@
 #![warn(clippy::sort_by_key_reverse)]
 
 fn main() {
-    // test code goes here
+    let mut vec = vec![3, 6, 1, 2, 5];
+    vec.sort_by(|a, b| b.cmp(a));
 }
diff --git a/tests/ui/sort_by_key_reverse.stderr b/tests/ui/sort_by_key_reverse.stderr
new file mode 100644
index 000000000000..e69de29bb2d1