Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stable_sort_primitive: print the type that is being sorted in the lint message #5935

Merged
merged 1 commit into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions clippy_lints/src/stable_sort_primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,18 @@ 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, _, args, _) = &expr.kind;
if let Some(slice) = &args.get(0);
if let Some(method) = SortingKind::from_stable_name(&method_name.ident.name.as_str());
if is_slice_of_primitives(cx, slice);
if let Some(slice_type) = is_slice_of_primitives(cx, slice);
then {
let args_str = args.iter().skip(1).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 })
Some(LintDetection { slice_name: Sugg::hir(cx, slice, "..").to_string(), method, method_args: args_str, slice_type })
} else {
None
}
Expand All @@ -111,9 +112,10 @@ impl LateLintPass<'_> for StableSortPrimitive {
STABLE_SORT_PRIMITIVE,
expr.span,
format!(
"used {} instead of {}",
"used {} instead of {} to sort primitive type `{}`",
detection.method.stable_name(),
detection.method.unstable_name()
detection.method.unstable_name(),
detection.slice_type,
)
.as_str(),
"try",
Expand Down
27 changes: 23 additions & 4 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1409,11 +1409,13 @@ pub fn is_recursively_primitive_type(ty: Ty<'_>) -> bool {
}
}

/// Returns true iff the given expression is a slice of primitives (as defined in the
/// `is_recursively_primitive_type` function).
pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// Returns Option<String> where String is a textual representation of the type encapsulated in the
/// slice iff the given expression is a slice of primitives (as defined in the
/// `is_recursively_primitive_type` function) and None otherwise.
pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
let expr_type = cx.typeck_results().expr_ty_adjusted(expr);
match expr_type.kind {
let expr_kind = &expr_type.kind;
let is_primitive = match expr_kind {
ty::Slice(ref element_type)
| ty::Ref(
_,
Expand All @@ -1424,7 +1426,24 @@ pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
_,
) => is_recursively_primitive_type(element_type),
_ => false,
};

if is_primitive {
// if we have wrappers like Array, Slice or Tuple, print these
// and get the type enclosed in the slice ref
match expr_type.peel_refs().walk().nth(1).unwrap().expect_ty().kind {
ty::Slice(..) => return Some("slice".into()),
ty::Array(..) => return Some("array".into()),
ty::Tuple(..) => return Some("tuple".into()),
_ => {
// is_recursively_primitive_type() should have taken care
// of the rest and we can rely on the type that is found
let refs_peeled = expr_type.peel_refs();
return Some(refs_peeled.walk().last().unwrap().to_string());
},
}
}
None
}

#[macro_export]
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/stable_sort_primitive.stderr
Original file line number Diff line number Diff line change
@@ -1,42 +1,42 @@
error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `i32`
--> $DIR/stable_sort_primitive.rs:7:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`
|
= note: `-D clippy::stable-sort-primitive` implied by `-D warnings`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `bool`
--> $DIR/stable_sort_primitive.rs:9:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `char`
--> $DIR/stable_sort_primitive.rs:11:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `str`
--> $DIR/stable_sort_primitive.rs:13:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `tuple`
--> $DIR/stable_sort_primitive.rs:15:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `array`
--> $DIR/stable_sort_primitive.rs:17:5
|
LL | vec.sort();
| ^^^^^^^^^^ help: try: `vec.sort_unstable()`

error: used sort instead of sort_unstable
error: used sort instead of sort_unstable to sort primitive type `i32`
--> $DIR/stable_sort_primitive.rs:19:5
|
LL | arr.sort();
Expand Down