Move StableSortPrimitive to Methods lint pass

This commit is contained in:
Jason Newcomb 2022-06-05 21:07:29 -04:00
parent 06d752e28d
commit e834855950
6 changed files with 76 additions and 148 deletions

View File

@ -353,6 +353,7 @@
methods::SINGLE_CHAR_ADD_STR, methods::SINGLE_CHAR_ADD_STR,
methods::SINGLE_CHAR_PATTERN, methods::SINGLE_CHAR_PATTERN,
methods::SKIP_WHILE_NEXT, methods::SKIP_WHILE_NEXT,
methods::STABLE_SORT_PRIMITIVE,
methods::STRING_EXTEND_CHARS, methods::STRING_EXTEND_CHARS,
methods::SUSPICIOUS_MAP, methods::SUSPICIOUS_MAP,
methods::SUSPICIOUS_SPLITN, methods::SUSPICIOUS_SPLITN,
@ -504,7 +505,6 @@
single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS, single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT, size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,
slow_vector_initialization::SLOW_VECTOR_INITIALIZATION, slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
stable_sort_primitive::STABLE_SORT_PRIMITIVE,
std_instead_of_core::ALLOC_INSTEAD_OF_CORE, std_instead_of_core::ALLOC_INSTEAD_OF_CORE,
std_instead_of_core::STD_INSTEAD_OF_ALLOC, std_instead_of_core::STD_INSTEAD_OF_ALLOC,
std_instead_of_core::STD_INSTEAD_OF_CORE, std_instead_of_core::STD_INSTEAD_OF_CORE,

View File

@ -64,6 +64,7 @@
LintId::of(methods::MANUAL_OK_OR), LintId::of(methods::MANUAL_OK_OR),
LintId::of(methods::MAP_UNWRAP_OR), LintId::of(methods::MAP_UNWRAP_OR),
LintId::of(methods::NAIVE_BYTECOUNT), LintId::of(methods::NAIVE_BYTECOUNT),
LintId::of(methods::STABLE_SORT_PRIMITIVE),
LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_JOIN),
LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(misc::USED_UNDERSCORE_BINDING),
LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER), LintId::of(mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER),
@ -85,7 +86,6 @@
LintId::of(ref_option_ref::REF_OPTION_REF), LintId::of(ref_option_ref::REF_OPTION_REF),
LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), 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(strings::STRING_ADD_ASSIGN),
LintId::of(transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(transmute::TRANSMUTE_PTR_TO_PTR),
LintId::of(types::LINKEDLIST), LintId::of(types::LINKEDLIST),

View File

@ -354,7 +354,6 @@ macro_rules! declare_clippy_lint {
mod single_component_path_imports; mod single_component_path_imports;
mod size_of_in_element_count; mod size_of_in_element_count;
mod slow_vector_initialization; mod slow_vector_initialization;
mod stable_sort_primitive;
mod std_instead_of_core; mod std_instead_of_core;
mod strings; mod strings;
mod strlen_on_c_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(&macro_matcher))); store.register_early_pass(move || Box::new(nonstandard_macro_braces::MacroBraces::new(&macro_matcher)));
store.register_late_pass(|| Box::new(macro_use::MacroUseImports::default())); 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(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(unwrap_in_result::UnwrapInResult));
store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned));
store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync)); store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync));

View File

@ -72,6 +72,7 @@
mod single_char_pattern; mod single_char_pattern;
mod single_char_push_string; mod single_char_push_string;
mod skip_while_next; mod skip_while_next;
mod stable_sort_primitive;
mod str_splitn; mod str_splitn;
mod string_extend_chars; mod string_extend_chars;
mod suspicious_map; mod suspicious_map;
@ -2793,6 +2794,44 @@
"using `.repeat(1)` instead of `String.clone()`, `str.to_string()` or `slice.to_vec()` " "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 { pub struct Methods {
avoid_breaking_exported_api: bool, avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>, msrv: Option<RustcVersion>,
@ -2909,6 +2948,7 @@ pub fn new(
PATH_BUF_PUSH_OVERWRITE, PATH_BUF_PUSH_OVERWRITE,
RANGE_ZIP_WITH_LEN, RANGE_ZIP_WITH_LEN,
REPEAT_ONCE, REPEAT_ONCE,
STABLE_SORT_PRIMITIVE,
]); ]);
/// Extracts a method call name, args, and `Span` of the method name. /// 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", [arg]) => {
repeat_once::check(cx, expr, recv, arg); repeat_once::check(cx, expr, recv, arg);
}, },
("sort", []) => {
stable_sort_primitive::check(cx, expr, recv);
},
("splitn" | "rsplitn", [count_arg, pat_arg]) => { ("splitn" | "rsplitn", [count_arg, pat_arg]) => {
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
suspicious_splitn::check(cx, name, expr, recv, count); suspicious_splitn::check(cx, name, expr, recv, count);

View File

@ -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",
);
},
);
}
}

View File

@ -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<SortingKind> {
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<LintDetection> {
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::<Vec<String>>().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",
);
},
);
}
}
}