Adjust test cases; run cargo dev bless

This commit is contained in:
Nahua Kang 2022-08-08 22:31:53 +02:00
parent c989746ccf
commit fb30b64f63
5 changed files with 174 additions and 83 deletions

View File

@ -1,3 +1,5 @@
// run-rustfix
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::get_parent_expr; use clippy_utils::get_parent_expr;
use clippy_utils::visitors::for_each_expr; use clippy_utils::visitors::for_each_expr;
@ -7,7 +9,7 @@ use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::*; use rustc_hir::{ExprKind, Path, QPath};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_span::source_map::Spanned; use rustc_span::source_map::Spanned;
@ -21,44 +23,31 @@ pub(super) fn check<'tcx>(
expr: &'tcx hir::Expr<'tcx>, expr: &'tcx hir::Expr<'tcx>,
name: &str, name: &str,
recv: &'tcx hir::Expr<'tcx>, recv: &'tcx hir::Expr<'tcx>,
args: &'tcx [hir::Expr<'tcx>],
) { ) {
match (name, args) { if name == "replace" {
("replace", ..) => { // The receiver of the method call must be `str` type to lint `collapsible_str_replace`
// The receiver of the method call must be `str` type to lint `collapsible_str_replace` let original_recv = find_original_recv(recv);
let original_recv = find_original_recv(recv); let original_recv_ty_kind = cx.typeck_results().expr_ty(original_recv).peel_refs().kind();
let original_recv_ty_kind = cx.typeck_results().expr_ty(original_recv).peel_refs().kind(); let original_recv_is_str_kind = matches!(original_recv_ty_kind, ty::Str);
let original_recv_is_str_kind = matches!(original_recv_ty_kind, ty::Str);
if_chain! { if_chain! {
if original_recv_is_str_kind; if original_recv_is_str_kind;
if let Some(parent) = get_parent_expr(cx, expr); if let Some(parent) = get_parent_expr(cx, expr);
if let Some((name, ..)) = method_call(parent); if let Some((name, ..)) = method_call(parent);
if name == "replace";
then { then {
match name { // If the parent node is a `str::replace` call, we've already handled the lint, don't lint again
// If the parent node is a `str::replace` call, we've already handled the lint, don't lint again return;
"replace" => return,
_ => {
check_consecutive_replace_calls(cx, expr);
return;
},
}
}
} }
}
match method_call(recv) { if let Some(("replace", ..)) = method_call(recv) {
// Check if there's an earlier `str::replace` call // Check if there's an earlier `str::replace` call
Some(("replace", ..)) => { if original_recv_is_str_kind {
if original_recv_is_str_kind { check_consecutive_replace_calls(cx, expr);
check_consecutive_replace_calls(cx, expr);
return;
}
},
_ => {},
} }
}, }
_ => {},
} }
} }
@ -116,7 +105,7 @@ fn check_consecutive_replace_calls<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir
/// Check if all the `from` arguments of a chain of consecutive calls to `str::replace` /// Check if all the `from` arguments of a chain of consecutive calls to `str::replace`
/// are all of `ExprKind::Lit` types. If any is not, return false. /// are all of `ExprKind::Lit` types. If any is not, return false.
fn replace_call_from_args_are_only_lit_chars<'tcx>(from_args: &Vec<&'tcx hir::Expr<'tcx>>) -> bool { fn replace_call_from_args_are_only_lit_chars<'tcx>(from_args: &[&'tcx hir::Expr<'tcx>]) -> bool {
let mut only_lit_chars = true; let mut only_lit_chars = true;
for from_arg in from_args.iter() { for from_arg in from_args.iter() {
@ -159,9 +148,9 @@ fn get_replace_call_from_args_if_all_char_ty<'tcx>(
if all_from_args_are_chars { if all_from_args_are_chars {
return Some(from_args); return Some(from_args);
} else {
return None;
} }
None
} }
/// Return a unique String representation of the `to` argument used in a chain of `str::replace` /// Return a unique String representation of the `to` argument used in a chain of `str::replace`
@ -186,13 +175,13 @@ fn get_replace_call_unique_to_arg_repr<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> Opt
// let mut to_arg_repr_set = FxHashSet::default(); // let mut to_arg_repr_set = FxHashSet::default();
let mut to_arg_reprs = Vec::new(); let mut to_arg_reprs = Vec::new();
for &to_arg in to_args.iter() { for &to_arg in &to_args {
if let Some(to_arg_repr) = get_replace_call_char_arg_repr(to_arg) { if let Some(to_arg_repr) = get_replace_call_char_arg_repr(to_arg) {
to_arg_reprs.push(to_arg_repr); to_arg_reprs.push(to_arg_repr);
} }
} }
let to_arg_repr_set = FxHashSet::from_iter(to_arg_reprs.iter().cloned()); let to_arg_repr_set = to_arg_reprs.iter().cloned().collect::<FxHashSet<_>>();
// Check if the set of `to` argument representations has more than one unique value // Check if the set of `to` argument representations has more than one unique value
if to_arg_repr_set.len() != 1 { if to_arg_repr_set.len() != 1 {
return None; return None;

View File

@ -152,11 +152,11 @@ declare_clippy_lint! {
/// let hello = "hesuo worpd" /// let hello = "hesuo worpd"
/// .replace('s', "l") /// .replace('s', "l")
/// .replace("u", "l") /// .replace("u", "l")
/// .replace('p', "l") /// .replace('p', "l");
/// ``` /// ```
/// Use instead: /// Use instead:
/// ```rust /// ```rust
/// let hello = "hesuo worpd".replace(|c| matches!(c, 's' | 'u' | 'p'), "l") /// let hello = "hesuo worpd".replace(|c| matches!(c, 's' | 'u' | 'p'), "l");
/// ``` /// ```
#[clippy::version = "1.64.0"] #[clippy::version = "1.64.0"]
pub COLLAPSIBLE_STR_REPLACE, pub COLLAPSIBLE_STR_REPLACE,
@ -3521,7 +3521,7 @@ impl Methods {
}, },
("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => { ("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => {
no_effect_replace::check(cx, expr, arg1, arg2); no_effect_replace::check(cx, expr, arg1, arg2);
collapsible_str_replace::check(cx, expr, name, recv, args); collapsible_str_replace::check(cx, expr, name, 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) {

View File

@ -0,0 +1,65 @@
// run-rustfix
#![warn(clippy::collapsible_str_replace)]
fn get_filter() -> &'static str {
"u"
}
fn main() {
let misspelled = "hesuo worpd";
let p = 'p';
let s = 's';
let u = 'u';
let l = "l";
// LINT CASES
let _ = misspelled.replace(|c| matches!(c, 'u' | 's'), "l");
let _ = misspelled.replace(|c| matches!(c, 'u' | 's'), l);
let _ = misspelled.replace(|c| matches!(c, 'p' | 'u' | 's'), "l");
let _ = misspelled
.replace(|c| matches!(c, 'd' | 'p' | 'u' | 's'), "l");
// FALLBACK CASES
// If there are consecutive calls to `str::replace` and all or any chars are variables,
// recommend the fallback `misspelled.replace(&[s, u, p], "l")`
let _ = misspelled.replace(&['u' , s], "l");
let _ = misspelled.replace(&['p' , 'u' , s], "l");
let _ = misspelled.replace(&['p' , u , s], "l");
let _ = misspelled.replace(&[p , u , s], "l");
// NO LINT CASES
let _ = misspelled.replace('s', "l");
let _ = misspelled.replace(s, "l");
// If the consecutive `str::replace` calls have different `to` arguments, do not lint
let _ = misspelled.replace('s', "l").replace('u', "p");
let _ = misspelled.replace(&get_filter(), "l");
let _ = misspelled.replace(&['s', 'u', 'p'], "l");
let _ = misspelled.replace(&['s', 'u', 'p'], l);
let _ = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l");
let _ = misspelled.replace('s', "l").replace(&['u', 'p'], "l");
let _ = misspelled.replace(&['s', 'u'], "l").replace('p', "l");
let _ = misspelled.replace(&['s', u, 'p'], "l");
let _ = misspelled.replace(&[s, u, 'p'], "l");
let _ = misspelled.replace(&[s, u, p], "l");
let _ = misspelled.replace(&[s, u], "l").replace(&[u, p], "l");
}

View File

@ -1,3 +1,5 @@
// run-rustfix
#![warn(clippy::collapsible_str_replace)] #![warn(clippy::collapsible_str_replace)]
fn get_filter() -> &'static str { fn get_filter() -> &'static str {
@ -13,75 +15,54 @@ fn main() {
let l = "l"; let l = "l";
// LINT CASES // LINT CASES
let replacement = misspelled.replace('s', "l").replace('u', "l"); let _ = misspelled.replace('s', "l").replace('u', "l");
println!("{replacement}");
let replacement = misspelled.replace('s', l).replace('u', l); let _ = misspelled.replace('s', l).replace('u', l);
println!("{replacement}");
let replacement = misspelled.replace('s', "l").replace('u', "l").replace('p', "l"); let _ = misspelled.replace('s', "l").replace('u', "l").replace('p', "l");
println!("{replacement}");
let replacement = misspelled let _ = misspelled
.replace('s', "l") .replace('s', "l")
.replace('u', "l") .replace('u', "l")
.replace('p', "l") .replace('p', "l")
.replace('d', "l"); .replace('d', "l");
println!("{replacement}");
// FALLBACK CASES // FALLBACK CASES
// If there are consecutive calls to `str::replace` and all or any chars are variables, // If there are consecutive calls to `str::replace` and all or any chars are variables,
// recommend the fallback `misspelled.replace(&[s, u, p], "l")` // recommend the fallback `misspelled.replace(&[s, u, p], "l")`
let replacement = misspelled.replace(s, "l").replace('u', "l"); let _ = misspelled.replace(s, "l").replace('u', "l");
println!("{replacement}");
let replacement = misspelled.replace(s, "l").replace('u', "l").replace('p', "l"); let _ = misspelled.replace(s, "l").replace('u', "l").replace('p', "l");
println!("{replacement}");
let replacement = misspelled.replace(s, "l").replace(u, "l").replace('p', "l"); let _ = misspelled.replace(s, "l").replace(u, "l").replace('p', "l");
println!("{replacement}");
let replacement = misspelled.replace(s, "l").replace(u, "l").replace(p, "l"); let _ = misspelled.replace(s, "l").replace(u, "l").replace(p, "l");
println!("{replacement}");
// NO LINT CASES // NO LINT CASES
let replacement = misspelled.replace('s', "l"); let _ = misspelled.replace('s', "l");
println!("{replacement}");
let replacement = misspelled.replace(s, "l"); let _ = misspelled.replace(s, "l");
println!("{replacement}");
// If the consecutive `str::replace` calls have different `to` arguments, do not lint // If the consecutive `str::replace` calls have different `to` arguments, do not lint
let replacement = misspelled.replace('s', "l").replace('u', "p"); let _ = misspelled.replace('s', "l").replace('u', "p");
println!("{replacement}");
let replacement = misspelled.replace(&get_filter(), "l"); let _ = misspelled.replace(&get_filter(), "l");
println!("{replacement}");
let replacement = misspelled.replace(&['s', 'u', 'p'], "l"); let _ = misspelled.replace(&['s', 'u', 'p'], "l");
println!("{replacement}");
let replacement = misspelled.replace(&['s', 'u', 'p'], l); let _ = misspelled.replace(&['s', 'u', 'p'], l);
println!("{replacement}");
let replacement = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l"); let _ = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l");
println!("{replacement}");
let replacement = misspelled.replace('s', "l").replace(&['u', 'p'], "l"); let _ = misspelled.replace('s', "l").replace(&['u', 'p'], "l");
println!("{replacement}");
let replacement = misspelled.replace(&['s', 'u'], "l").replace('p', "l"); let _ = misspelled.replace(&['s', 'u'], "l").replace('p', "l");
println!("replacement");
let replacement = misspelled.replace(&['s', u, 'p'], "l"); let _ = misspelled.replace(&['s', u, 'p'], "l");
println!("{replacement}");
let replacement = misspelled.replace(&[s, u, 'p'], "l"); let _ = misspelled.replace(&[s, u, 'p'], "l");
println!("{replacement}");
let replacement = misspelled.replace(&[s, u, p], "l"); let _ = misspelled.replace(&[s, u, p], "l");
println!("{replacement}");
let replacement = misspelled.replace(&[s, u], "l").replace(&[u, p], "l"); let _ = misspelled.replace(&[s, u], "l").replace(&[u, p], "l");
println!("{replacement}");
} }

View File

@ -0,0 +1,56 @@
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:18:24
|
LL | let _ = misspelled.replace('s', "l").replace('u', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(|c| matches!(c, 'u' | 's'), "l")`
|
= note: `-D clippy::collapsible-str-replace` implied by `-D warnings`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:20:24
|
LL | let _ = misspelled.replace('s', l).replace('u', l);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(|c| matches!(c, 'u' | 's'), l)`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:22:24
|
LL | let _ = misspelled.replace('s', "l").replace('u', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(|c| matches!(c, 'p' | 'u' | 's'), "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:25:10
|
LL | .replace('s', "l")
| __________^
LL | | .replace('u', "l")
LL | | .replace('p', "l")
LL | | .replace('d', "l");
| |__________________________^ help: replace with: `replace(|c| matches!(c, 'd' | 'p' | 'u' | 's'), "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:33:24
|
LL | let _ = misspelled.replace(s, "l").replace('u', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(&['u' , s], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:35:24
|
LL | let _ = misspelled.replace(s, "l").replace('u', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(&['p' , 'u' , s], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:37:24
|
LL | let _ = misspelled.replace(s, "l").replace(u, "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(&['p' , u , s], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:39:24
|
LL | let _ = misspelled.replace(s, "l").replace(u, "l").replace(p, "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(&[p , u , s], "l")`
error: aborting due to 8 previous errors