diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/unnecessary_sort_by.rs index dd74bf367f3..26b56e0f2f3 100644 --- a/clippy_lints/src/unnecessary_sort_by.rs +++ b/clippy_lints/src/unnecessary_sort_by.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{implements_trait, 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}; @@ -193,10 +193,15 @@ 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 { + })) = &left_expr.kind; + if left_name == left_ident; + if cx.tcx.get_diagnostic_item(sym::Ord).map_or(false, |id| { + implements_trait(cx, cx.typeck_results().expr_ty(left_expr), id, &[]) + }); + then { return Some(LintTrigger::Sort(SortDetection { vec_name, unstable })); } } diff --git a/tests/ui/unnecessary_sort_by.fixed b/tests/ui/unnecessary_sort_by.fixed index b45b27d8f23..d806d620b17 100644 --- a/tests/ui/unnecessary_sort_by.fixed +++ b/tests/ui/unnecessary_sort_by.fixed @@ -2,6 +2,7 @@ #![allow(clippy::stable_sort_primitive)] +use std::cell::Ref; use std::cmp::Reverse; fn unnecessary_sort_by() { @@ -33,6 +34,10 @@ fn unnecessary_sort_by() { // `Reverse(b)` would borrow in the following cases, don't lint vec.sort_by(|a, b| b.cmp(a)); vec.sort_unstable_by(|a, b| b.cmp(a)); + + // No warning if element does not implement `Ord` + let mut vec: Vec> = Vec::new(); + vec.sort_unstable_by(|a, b| a.cmp(b)); } // Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` diff --git a/tests/ui/unnecessary_sort_by.rs b/tests/ui/unnecessary_sort_by.rs index be2abe7f701..6ee9c3af455 100644 --- a/tests/ui/unnecessary_sort_by.rs +++ b/tests/ui/unnecessary_sort_by.rs @@ -2,6 +2,7 @@ #![allow(clippy::stable_sort_primitive)] +use std::cell::Ref; use std::cmp::Reverse; fn unnecessary_sort_by() { @@ -33,6 +34,10 @@ fn unnecessary_sort_by() { // `Reverse(b)` would borrow in the following cases, don't lint vec.sort_by(|a, b| b.cmp(a)); vec.sort_unstable_by(|a, b| b.cmp(a)); + + // No warning if element does not implement `Ord` + let mut vec: Vec> = Vec::new(); + vec.sort_unstable_by(|a, b| a.cmp(b)); } // Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` diff --git a/tests/ui/unnecessary_sort_by.stderr b/tests/ui/unnecessary_sort_by.stderr index 50607933e18..ca9641e8803 100644 --- a/tests/ui/unnecessary_sort_by.stderr +++ b/tests/ui/unnecessary_sort_by.stderr @@ -1,5 +1,5 @@ error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:14:5 + --> $DIR/unnecessary_sort_by.rs:15:5 | LL | vec.sort_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()` @@ -7,67 +7,67 @@ LL | vec.sort_by(|a, b| a.cmp(b)); = note: `-D clippy::unnecessary-sort-by` implied by `-D warnings` error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:15:5 + --> $DIR/unnecessary_sort_by.rs:16:5 | LL | vec.sort_unstable_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:16:5 + --> $DIR/unnecessary_sort_by.rs:17:5 | LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:17:5 + --> $DIR/unnecessary_sort_by.rs:18:5 | LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| id(-a))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:20:5 + --> $DIR/unnecessary_sort_by.rs:21:5 | LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| Reverse((b + 5).abs()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:21:5 + --> $DIR/unnecessary_sort_by.rs:22:5 | LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| Reverse(id(-b)))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:31:5 + --> $DIR/unnecessary_sort_by.rs:32:5 | LL | vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:32:5 + --> $DIR/unnecessary_sort_by.rs:33:5 | LL | vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:88:9 + --> $DIR/unnecessary_sort_by.rs:93:9 | LL | args.sort_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:89:9 + --> $DIR/unnecessary_sort_by.rs:94:9 | LL | args.sort_unstable_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:91:9 + --> $DIR/unnecessary_sort_by.rs:96:9 | LL | args.sort_by(|a, b| b.name().cmp(&a.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| Reverse(b.name()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:92:9 + --> $DIR/unnecessary_sort_by.rs:97:9 | LL | args.sort_unstable_by(|a, b| b.name().cmp(&a.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| Reverse(b.name()))`