diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index e51ce0fa2..bb5b28662 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -213,6 +213,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::UNNECESSARY_FIND_MAP),
LintId::of(methods::UNNECESSARY_FOLD),
LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS),
+ LintId::of(methods::UNNECESSARY_SORT_BY),
LintId::of(methods::UNNECESSARY_TO_OWNED),
LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT),
LintId::of(methods::USELESS_ASREF),
@@ -334,7 +335,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS),
LintId::of(unnamed_address::VTABLE_ADDRESS_COMPARISONS),
LintId::of(unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS),
- LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(unused_io_amount::UNUSED_IO_AMOUNT),
LintId::of(unused_unit::UNUSED_UNIT),
diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs
index f79105f49..0f7433a79 100644
--- a/clippy_lints/src/lib.register_complexity.rs
+++ b/clippy_lints/src/lib.register_complexity.rs
@@ -57,6 +57,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(methods::SKIP_WHILE_NEXT),
LintId::of(methods::UNNECESSARY_FILTER_MAP),
LintId::of(methods::UNNECESSARY_FIND_MAP),
+ LintId::of(methods::UNNECESSARY_SORT_BY),
LintId::of(methods::USELESS_ASREF),
LintId::of(misc::SHORT_CIRCUIT_STATEMENT),
LintId::of(misc_early::UNNEEDED_WILDCARD_PATTERN),
@@ -99,7 +100,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(types::TYPE_COMPLEXITY),
LintId::of(types::VEC_BOX),
LintId::of(unit_types::UNIT_ARG),
- LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(unwrap::UNNECESSARY_UNWRAP),
LintId::of(useless_conversion::USELESS_CONVERSION),
LintId::of(zero_div_zero::ZERO_DIVIDED_BY_ZERO),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 8aa1810fb..f5497c6bd 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -364,6 +364,7 @@ store.register_lints(&[
methods::UNNECESSARY_FOLD,
methods::UNNECESSARY_JOIN,
methods::UNNECESSARY_LAZY_EVALUATIONS,
+ methods::UNNECESSARY_SORT_BY,
methods::UNNECESSARY_TO_OWNED,
methods::UNWRAP_OR_ELSE_DEFAULT,
methods::UNWRAP_USED,
@@ -567,7 +568,6 @@ store.register_lints(&[
unnamed_address::VTABLE_ADDRESS_COMPARISONS,
unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS,
unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS,
- unnecessary_sort_by::UNNECESSARY_SORT_BY,
unnecessary_wraps::UNNECESSARY_WRAPS,
unnested_or_patterns::UNNESTED_OR_PATTERNS,
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 71ba3f18d..cfcd4e3d1 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -376,7 +376,6 @@ mod unit_types;
mod unnamed_address;
mod unnecessary_owned_empty_strings;
mod unnecessary_self_imports;
-mod unnecessary_sort_by;
mod unnecessary_wraps;
mod unnested_or_patterns;
mod unsafe_removed_from_name;
@@ -716,7 +715,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(ptr_offset_with_cast::PtrOffsetWithCast));
store.register_late_pass(|| Box::new(redundant_clone::RedundantClone));
store.register_late_pass(|| Box::new(slow_vector_initialization::SlowVectorInit));
- store.register_late_pass(|| Box::new(unnecessary_sort_by::UnnecessarySortBy));
store.register_late_pass(move || Box::new(unnecessary_wraps::UnnecessaryWraps::new(avoid_breaking_exported_api)));
store.register_late_pass(|| Box::new(assertions_on_constants::AssertionsOnConstants));
store.register_late_pass(|| Box::new(assertions_on_result_states::AssertionsOnResultStates));
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index ce10b64c9..54a0275da 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -84,6 +84,7 @@ mod unnecessary_fold;
mod unnecessary_iter_cloned;
mod unnecessary_join;
mod unnecessary_lazy_eval;
+mod unnecessary_sort_by;
mod unnecessary_to_owned;
mod unwrap_or_else_default;
mod unwrap_used;
@@ -2872,6 +2873,40 @@ declare_clippy_lint! {
"hashing a unit value, which does nothing"
}
+declare_clippy_lint! {
+ /// ### What it does
+ /// 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 a more complicated
+ /// closure.
+ ///
+ /// ### Known problems
+ /// 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
+ /// ```rust
+ /// # struct A;
+ /// # impl A { fn foo(&self) {} }
+ /// # let mut vec: Vec = Vec::new();
+ /// vec.sort_by(|a, b| a.foo().cmp(&b.foo()));
+ /// ```
+ /// Use instead:
+ /// ```rust
+ /// # struct A;
+ /// # impl A { fn foo(&self) {} }
+ /// # let mut vec: Vec = Vec::new();
+ /// vec.sort_by_key(|a| a.foo());
+ /// ```
+ #[clippy::version = "1.46.0"]
+ pub UNNECESSARY_SORT_BY,
+ complexity,
+ "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer"
+}
+
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option,
@@ -2990,6 +3025,7 @@ impl_lint_pass!(Methods => [
REPEAT_ONCE,
STABLE_SORT_PRIMITIVE,
UNIT_HASH,
+ UNNECESSARY_SORT_BY,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3387,6 +3423,12 @@ impl Methods {
("sort", []) => {
stable_sort_primitive::check(cx, expr, recv);
},
+ ("sort_by", [arg]) => {
+ unnecessary_sort_by::check(cx, expr, recv, arg, false);
+ },
+ ("sort_unstable_by", [arg]) => {
+ unnecessary_sort_by::check(cx, expr, recv, arg, true);
+ },
("splitn" | "rsplitn", [count_arg, pat_arg]) => {
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
suspicious_splitn::check(cx, name, expr, recv, count);
diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs
similarity index 63%
rename from clippy_lints/src/unnecessary_sort_by.rs
rename to clippy_lints/src/methods/unnecessary_sort_by.rs
index ea5aadbbc..1966990bd 100644
--- a/clippy_lints/src/unnecessary_sort_by.rs
+++ b/clippy_lints/src/methods/unnecessary_sort_by.rs
@@ -1,51 +1,17 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_trait_method;
use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
+use clippy_utils::ty::implements_trait;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::LateContext;
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 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 a more complicated
- /// closure.
- ///
- /// ### Known problems
- /// 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
- /// ```rust
- /// # struct A;
- /// # impl A { fn foo(&self) {} }
- /// # let mut vec: Vec = Vec::new();
- /// vec.sort_by(|a, b| a.foo().cmp(&b.foo()));
- /// ```
- /// Use instead:
- /// ```rust
- /// # struct A;
- /// # impl A { fn foo(&self) {} }
- /// # let mut vec: Vec = Vec::new();
- /// vec.sort_by_key(|a| a.foo());
- /// ```
- #[clippy::version = "1.46.0"]
- pub UNNECESSARY_SORT_BY,
- complexity,
- "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer"
-}
-
-declare_lint_pass!(UnnecessarySortBy => [UNNECESSARY_SORT_BY]);
+use super::UNNECESSARY_SORT_BY;
enum LintTrigger {
Sort(SortDetection),
@@ -54,7 +20,6 @@ enum LintTrigger {
struct SortDetection {
vec_name: String,
- unstable: bool,
}
struct SortByKeyDetection {
@@ -62,7 +27,6 @@ struct SortByKeyDetection {
closure_arg: String,
closure_body: String,
reverse: bool,
- unstable: bool,
}
/// Detect if the two expressions are mirrored (identical, except one
@@ -150,20 +114,20 @@ fn mirrored_exprs(a_expr: &Expr<'_>, a_ident: &Ident, b_expr: &Expr<'_>, b_ident
}
}
-fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option {
+fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) -> Option {
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 { body: closure_body_id, .. }), .. }] = args;
- if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(vec), sym::Vec);
- if let closure_body = cx.tcx.hir().body(*closure_body_id);
+ if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+ if let Some(impl_id) = cx.tcx.impl_of_method(method_id);
+ if cx.tcx.type_of(impl_id).is_slice();
+ if let ExprKind::Closure(&Closure { body, .. }) = arg.kind;
+ if let closure_body = cx.tcx.hir().body(body);
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 let ExprKind::MethodCall(method_path, [left_expr, right_expr], _) = closure_body.value.kind;
if method_path.ident.name == sym::cmp;
+ if is_trait_method(cx, &closure_body.value, sym::Ord);
then {
let (closure_body, closure_arg, reverse) = if mirrored_exprs(
left_expr,
@@ -177,19 +141,18 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option {
} else {
return None;
};
- let vec_name = Sugg::hir(cx, &args[0], "..").to_string();
- let unstable = name == "sort_unstable_by";
+ let vec_name = Sugg::hir(cx, recv, "..").to_string();
if_chain! {
- if let ExprKind::Path(QPath::Resolved(_, Path {
- segments: [PathSegment { ident: left_name, .. }], ..
- })) = &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, &[])
- });
+ if let ExprKind::Path(QPath::Resolved(_, Path {
+ segments: [PathSegment { ident: left_name, .. }], ..
+ })) = &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 }));
+ return Some(LintTrigger::Sort(SortDetection { vec_name }));
}
}
@@ -199,7 +162,6 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option {
closure_arg,
closure_body,
reverse,
- unstable,
}));
}
}
@@ -213,46 +175,50 @@ fn expr_borrows(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
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)) => span_lint_and_sugg(
- cx,
- UNNECESSARY_SORT_BY,
- expr.span,
- "use Vec::sort_by_key here instead",
- "try",
- format!(
- "{}.sort{}_by_key(|{}| {})",
- trigger.vec_name,
- if trigger.unstable { "_unstable" } else { "" },
- trigger.closure_arg,
- if trigger.reverse {
- format!("std::cmp::Reverse({})", trigger.closure_body)
- } else {
- trigger.closure_body.to_string()
- },
- ),
+pub(super) fn check<'tcx>(
+ cx: &LateContext<'tcx>,
+ expr: &'tcx Expr<'_>,
+ recv: &'tcx Expr<'_>,
+ arg: &'tcx Expr<'_>,
+ is_unstable: bool,
+) {
+ match detect_lint(cx, expr, recv, arg) {
+ 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(|{}| {})",
+ trigger.vec_name,
+ if is_unstable { "_unstable" } else { "" },
+ trigger.closure_arg,
if trigger.reverse {
- Applicability::MaybeIncorrect
+ format!("std::cmp::Reverse({})", trigger.closure_body)
} else {
- Applicability::MachineApplicable
+ trigger.closure_body.to_string()
},
),
- Some(LintTrigger::Sort(trigger)) => span_lint_and_sugg(
- cx,
- UNNECESSARY_SORT_BY,
- expr.span,
- "use Vec::sort here instead",
- "try",
- format!(
- "{}.sort{}()",
- trigger.vec_name,
- if trigger.unstable { "_unstable" } else { "" },
- ),
- Applicability::MachineApplicable,
+ if trigger.reverse {
+ Applicability::MaybeIncorrect
+ } else {
+ Applicability::MachineApplicable
+ },
+ ),
+ Some(LintTrigger::Sort(trigger)) => span_lint_and_sugg(
+ cx,
+ UNNECESSARY_SORT_BY,
+ expr.span,
+ "use Vec::sort here instead",
+ "try",
+ format!(
+ "{}.sort{}()",
+ trigger.vec_name,
+ if is_unstable { "_unstable" } else { "" },
),
- None => {},
- }
+ Applicability::MachineApplicable,
+ ),
+ None => {},
}
}