From e834855950701dae9657e7c0dbde195f85a96bf3 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 5 Jun 2022 21:07:29 -0400 Subject: [PATCH] Move `StableSortPrimitive` to `Methods` lint pass --- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_pedantic.rs | 2 +- clippy_lints/src/lib.rs | 2 - clippy_lints/src/methods/mod.rs | 43 ++++++ .../src/methods/stable_sort_primitive.rs | 31 ++++ clippy_lints/src/stable_sort_primitive.rs | 144 ------------------ 6 files changed, 76 insertions(+), 148 deletions(-) create mode 100644 clippy_lints/src/methods/stable_sort_primitive.rs delete mode 100644 clippy_lints/src/stable_sort_primitive.rs diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 659f0288d17..e8c796b4e9b 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -353,6 +353,7 @@ methods::SINGLE_CHAR_ADD_STR, methods::SINGLE_CHAR_PATTERN, methods::SKIP_WHILE_NEXT, + methods::STABLE_SORT_PRIMITIVE, methods::STRING_EXTEND_CHARS, methods::SUSPICIOUS_MAP, methods::SUSPICIOUS_SPLITN, @@ -504,7 +505,6 @@ single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS, size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT, slow_vector_initialization::SLOW_VECTOR_INITIALIZATION, - stable_sort_primitive::STABLE_SORT_PRIMITIVE, std_instead_of_core::ALLOC_INSTEAD_OF_CORE, std_instead_of_core::STD_INSTEAD_OF_ALLOC, std_instead_of_core::STD_INSTEAD_OF_CORE, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 5c04a331d0a..13474127e8d 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -64,6 +64,7 @@ LintId::of(methods::MANUAL_OK_OR), LintId::of(methods::MAP_UNWRAP_OR), LintId::of(methods::NAIVE_BYTECOUNT), + LintId::of(methods::STABLE_SORT_PRIMITIVE), LintId::of(methods::UNNECESSARY_JOIN), LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER), @@ -85,7 +86,6 @@ LintId::of(ref_option_ref::REF_OPTION_REF), LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), - LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE), LintId::of(strings::STRING_ADD_ASSIGN), LintId::of(transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(types::LINKEDLIST), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 40c77ede9ad..c424c373d32 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -354,7 +354,6 @@ macro_rules! declare_clippy_lint { mod single_component_path_imports; mod size_of_in_element_count; mod slow_vector_initialization; -mod stable_sort_primitive; mod std_instead_of_core; mod strings; mod strlen_on_c_strings; @@ -822,7 +821,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || Box::new(nonstandard_macro_braces::MacroBraces::new(¯o_matcher))); store.register_late_pass(|| Box::new(macro_use::MacroUseImports::default())); store.register_late_pass(|| Box::new(pattern_type_mismatch::PatternTypeMismatch)); - store.register_late_pass(|| Box::new(stable_sort_primitive::StableSortPrimitive)); store.register_late_pass(|| Box::new(unwrap_in_result::UnwrapInResult)); store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync)); diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index bbd6f56c5ea..a47081d8752 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -72,6 +72,7 @@ mod single_char_pattern; mod single_char_push_string; mod skip_while_next; +mod stable_sort_primitive; mod str_splitn; mod string_extend_chars; mod suspicious_map; @@ -2793,6 +2794,44 @@ "using `.repeat(1)` instead of `String.clone()`, `str.to_string()` or `slice.to_vec()` " } +declare_clippy_lint! { + /// ### What it does + /// When sorting primitive values (integers, bools, chars, as well + /// as arrays, slices, and tuples of such items), it is typically better to + /// use an unstable sort than a stable sort. + /// + /// ### Why is this bad? + /// Typically, using a stable sort consumes more memory and cpu cycles. + /// Because values which compare equal are identical, preserving their + /// relative order (the guarantee that a stable sort provides) means + /// nothing, while the extra costs still apply. + /// + /// ### Known problems + /// + /// As pointed out in + /// [issue #8241](https://github.com/rust-lang/rust-clippy/issues/8241), + /// a stable sort can instead be significantly faster for certain scenarios + /// (eg. when a sorted vector is extended with new data and resorted). + /// + /// For more information and benchmarking results, please refer to the + /// issue linked above. + /// + /// ### Example + /// ```rust + /// let mut vec = vec![2, 1, 3]; + /// vec.sort(); + /// ``` + /// Use instead: + /// ```rust + /// let mut vec = vec![2, 1, 3]; + /// vec.sort_unstable(); + /// ``` + #[clippy::version = "1.47.0"] + pub STABLE_SORT_PRIMITIVE, + pedantic, + "use of sort() when sort_unstable() is equivalent" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -2909,6 +2948,7 @@ pub fn new( PATH_BUF_PUSH_OVERWRITE, RANGE_ZIP_WITH_LEN, REPEAT_ONCE, + STABLE_SORT_PRIMITIVE, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3300,6 +3340,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { ("repeat", [arg]) => { repeat_once::check(cx, expr, recv, arg); }, + ("sort", []) => { + stable_sort_primitive::check(cx, expr, recv); + }, ("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/methods/stable_sort_primitive.rs b/clippy_lints/src/methods/stable_sort_primitive.rs new file mode 100644 index 00000000000..91951c65bb3 --- /dev/null +++ b/clippy_lints/src/methods/stable_sort_primitive.rs @@ -0,0 +1,31 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_slice_of_primitives; +use clippy_utils::source::snippet_with_context; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; + +use super::STABLE_SORT_PRIMITIVE; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { + if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) + && let Some(impl_id) = cx.tcx.impl_of_method(method_id) + && cx.tcx.type_of(impl_id).is_slice() + && let Some(slice_type) = is_slice_of_primitives(cx, recv) + { + span_lint_and_then( + cx, + STABLE_SORT_PRIMITIVE, + e.span, + &format!("used `sort` on primitive type `{}`", slice_type), + |diag| { + let mut app = Applicability::MachineApplicable; + let recv_snip = snippet_with_context(cx, recv.span, e.span.ctxt(), "..", &mut app).0; + diag.span_suggestion(e.span, "try", format!("{}.sort_unstable()", recv_snip), app); + diag.note( + "an unstable sort typically performs faster without any observable difference for this data type", + ); + }, + ); + } +} diff --git a/clippy_lints/src/stable_sort_primitive.rs b/clippy_lints/src/stable_sort_primitive.rs deleted file mode 100644 index 6d54935f81a..00000000000 --- a/clippy_lints/src/stable_sort_primitive.rs +++ /dev/null @@ -1,144 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::{is_slice_of_primitives, sugg::Sugg}; -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; - -declare_clippy_lint! { - /// ### What it does - /// When sorting primitive values (integers, bools, chars, as well - /// as arrays, slices, and tuples of such items), it is typically better to - /// use an unstable sort than a stable sort. - /// - /// ### Why is this bad? - /// Typically, using a stable sort consumes more memory and cpu cycles. - /// Because values which compare equal are identical, preserving their - /// relative order (the guarantee that a stable sort provides) means - /// nothing, while the extra costs still apply. - /// - /// ### Known problems - /// - /// As pointed out in - /// [issue #8241](https://github.com/rust-lang/rust-clippy/issues/8241), - /// a stable sort can instead be significantly faster for certain scenarios - /// (eg. when a sorted vector is extended with new data and resorted). - /// - /// For more information and benchmarking results, please refer to the - /// issue linked above. - /// - /// ### Example - /// ```rust - /// let mut vec = vec![2, 1, 3]; - /// vec.sort(); - /// ``` - /// Use instead: - /// ```rust - /// let mut vec = vec![2, 1, 3]; - /// vec.sort_unstable(); - /// ``` - #[clippy::version = "1.47.0"] - pub STABLE_SORT_PRIMITIVE, - pedantic, - "use of sort() when sort_unstable() is equivalent" -} - -declare_lint_pass!(StableSortPrimitive => [STABLE_SORT_PRIMITIVE]); - -/// The three "kinds" of sorts -enum SortingKind { - Vanilla, - /* The other kinds of lint are currently commented out because they - * can map distinct values to equal ones. If the key function is - * provably one-to-one, or if the Cmp function conserves equality, - * then they could be linted on, but I don't know if we can check - * for that. */ - - /* ByKey, - * ByCmp, */ -} -impl SortingKind { - /// The name of the stable version of this kind of sort - fn stable_name(&self) -> &str { - match self { - SortingKind::Vanilla => "sort", - /* SortingKind::ByKey => "sort_by_key", - * SortingKind::ByCmp => "sort_by", */ - } - } - /// The name of the unstable version of this kind of sort - fn unstable_name(&self) -> &str { - match self { - SortingKind::Vanilla => "sort_unstable", - /* SortingKind::ByKey => "sort_unstable_by_key", - * SortingKind::ByCmp => "sort_unstable_by", */ - } - } - /// Takes the name of a function call and returns the kind of sort - /// that corresponds to that function name (or None if it isn't) - fn from_stable_name(name: &str) -> Option { - match name { - "sort" => Some(SortingKind::Vanilla), - // "sort_by" => Some(SortingKind::ByCmp), - // "sort_by_key" => Some(SortingKind::ByKey), - _ => None, - } - } -} - -/// A detected instance of this lint -struct LintDetection { - slice_name: String, - method: SortingKind, - method_args: String, - slice_type: String, -} - -fn detect_stable_sort_primitive(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - if_chain! { - if let ExprKind::MethodCall(method_name, [slice, args @ ..], _) = &expr.kind; - if let Some(method) = SortingKind::from_stable_name(method_name.ident.name.as_str()); - if let Some(slice_type) = is_slice_of_primitives(cx, slice); - then { - let args_str = args.iter().map(|arg| Sugg::hir(cx, arg, "..").to_string()).collect::>().join(", "); - Some(LintDetection { slice_name: Sugg::hir(cx, slice, "..").to_string(), method, method_args: args_str, slice_type }) - } else { - None - } - } -} - -impl LateLintPass<'_> for StableSortPrimitive { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if let Some(detection) = detect_stable_sort_primitive(cx, expr) { - span_lint_and_then( - cx, - STABLE_SORT_PRIMITIVE, - expr.span, - format!( - "used `{}` on primitive type `{}`", - detection.method.stable_name(), - detection.slice_type, - ) - .as_str(), - |diag| { - diag.span_suggestion( - expr.span, - "try", - format!( - "{}.{}({})", - detection.slice_name, - detection.method.unstable_name(), - detection.method_args, - ), - Applicability::MachineApplicable, - ); - diag.note( - "an unstable sort typically performs faster without any observable difference for this data type", - ); - }, - ); - } - } -}