From fb30b64f6379eb22880d70bfebce66ceccc75269 Mon Sep 17 00:00:00 2001 From: Nahua Kang Date: Mon, 8 Aug 2022 22:31:53 +0200 Subject: [PATCH] Adjust test cases; run cargo dev bless --- .../src/methods/collapsible_str_replace.rs | 65 ++++++++----------- clippy_lints/src/methods/mod.rs | 6 +- tests/ui/collapsible_str_replace.fixed | 65 +++++++++++++++++++ tests/ui/collapsible_str_replace.rs | 65 +++++++------------ tests/ui/collapsible_str_replace.stderr | 56 ++++++++++++++++ 5 files changed, 174 insertions(+), 83 deletions(-) create mode 100644 tests/ui/collapsible_str_replace.fixed create mode 100644 tests/ui/collapsible_str_replace.stderr diff --git a/clippy_lints/src/methods/collapsible_str_replace.rs b/clippy_lints/src/methods/collapsible_str_replace.rs index 1760232041a..4c6288e798c 100644 --- a/clippy_lints/src/methods/collapsible_str_replace.rs +++ b/clippy_lints/src/methods/collapsible_str_replace.rs @@ -1,3 +1,5 @@ +// run-rustfix + use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::get_parent_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_errors::Applicability; use rustc_hir as hir; -use rustc_hir::*; +use rustc_hir::{ExprKind, Path, QPath}; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::source_map::Spanned; @@ -21,44 +23,31 @@ pub(super) fn check<'tcx>( expr: &'tcx hir::Expr<'tcx>, name: &str, recv: &'tcx hir::Expr<'tcx>, - args: &'tcx [hir::Expr<'tcx>], ) { - match (name, args) { - ("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_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); + if name == "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_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); - if_chain! { - if original_recv_is_str_kind; - if let Some(parent) = get_parent_expr(cx, expr); - if let Some((name, ..)) = method_call(parent); + if_chain! { + if original_recv_is_str_kind; + if let Some(parent) = get_parent_expr(cx, expr); + if let Some((name, ..)) = method_call(parent); + if name == "replace"; - then { - match name { - // If the parent node is a `str::replace` call, we've already handled the lint, don't lint again - "replace" => return, - _ => { - check_consecutive_replace_calls(cx, expr); - return; - }, - } - } + then { + // If the parent node is a `str::replace` call, we've already handled the lint, don't lint again + return; } + } - match method_call(recv) { - // Check if there's an earlier `str::replace` call - Some(("replace", ..)) => { - if original_recv_is_str_kind { - check_consecutive_replace_calls(cx, expr); - return; - } - }, - _ => {}, + if let Some(("replace", ..)) = method_call(recv) { + // Check if there's an earlier `str::replace` call + if original_recv_is_str_kind { + check_consecutive_replace_calls(cx, expr); } - }, - _ => {}, + } } } @@ -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` /// 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; 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 { return Some(from_args); - } else { - return None; } + + None } /// 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_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) { 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::>(); // Check if the set of `to` argument representations has more than one unique value if to_arg_repr_set.len() != 1 { return None; diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f9358693623..8b2fa8e9457 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -152,11 +152,11 @@ declare_clippy_lint! { /// let hello = "hesuo worpd" /// .replace('s', "l") /// .replace("u", "l") - /// .replace('p', "l") + /// .replace('p', "l"); /// ``` /// Use instead: /// ```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"] pub COLLAPSIBLE_STR_REPLACE, @@ -3521,7 +3521,7 @@ impl Methods { }, ("replace" | "replacen", [arg1, arg2] | [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]) => { if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { diff --git a/tests/ui/collapsible_str_replace.fixed b/tests/ui/collapsible_str_replace.fixed new file mode 100644 index 00000000000..0bf857d9837 --- /dev/null +++ b/tests/ui/collapsible_str_replace.fixed @@ -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"); +} diff --git a/tests/ui/collapsible_str_replace.rs b/tests/ui/collapsible_str_replace.rs index 05a4fd08a32..45d9fd87e5e 100644 --- a/tests/ui/collapsible_str_replace.rs +++ b/tests/ui/collapsible_str_replace.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::collapsible_str_replace)] fn get_filter() -> &'static str { @@ -13,75 +15,54 @@ fn main() { let l = "l"; // LINT CASES - let replacement = misspelled.replace('s', "l").replace('u', "l"); - println!("{replacement}"); + let _ = misspelled.replace('s', "l").replace('u', "l"); - let replacement = misspelled.replace('s', l).replace('u', l); - println!("{replacement}"); + let _ = misspelled.replace('s', l).replace('u', l); - let replacement = misspelled.replace('s', "l").replace('u', "l").replace('p', "l"); - println!("{replacement}"); + let _ = misspelled.replace('s', "l").replace('u', "l").replace('p', "l"); - let replacement = misspelled + let _ = misspelled .replace('s', "l") .replace('u', "l") .replace('p', "l") .replace('d', "l"); - println!("{replacement}"); // 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 replacement = misspelled.replace(s, "l").replace('u', "l"); - println!("{replacement}"); + let _ = misspelled.replace(s, "l").replace('u', "l"); - let replacement = misspelled.replace(s, "l").replace('u', "l").replace('p', "l"); - println!("{replacement}"); + let _ = misspelled.replace(s, "l").replace('u', "l").replace('p', "l"); - let replacement = misspelled.replace(s, "l").replace(u, "l").replace('p', "l"); - println!("{replacement}"); + let _ = misspelled.replace(s, "l").replace(u, "l").replace('p', "l"); - let replacement = misspelled.replace(s, "l").replace(u, "l").replace(p, "l"); - println!("{replacement}"); + let _ = misspelled.replace(s, "l").replace(u, "l").replace(p, "l"); // NO LINT CASES - let replacement = misspelled.replace('s', "l"); - println!("{replacement}"); + let _ = misspelled.replace('s', "l"); - let replacement = misspelled.replace(s, "l"); - println!("{replacement}"); + let _ = misspelled.replace(s, "l"); // If the consecutive `str::replace` calls have different `to` arguments, do not lint - let replacement = misspelled.replace('s', "l").replace('u', "p"); - println!("{replacement}"); + let _ = misspelled.replace('s', "l").replace('u', "p"); - let replacement = misspelled.replace(&get_filter(), "l"); - println!("{replacement}"); + let _ = misspelled.replace(&get_filter(), "l"); - let replacement = misspelled.replace(&['s', 'u', 'p'], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&['s', 'u', 'p'], "l"); - let replacement = misspelled.replace(&['s', 'u', 'p'], l); - println!("{replacement}"); + let _ = misspelled.replace(&['s', 'u', 'p'], l); - let replacement = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l"); - let replacement = misspelled.replace('s', "l").replace(&['u', 'p'], "l"); - println!("{replacement}"); + let _ = misspelled.replace('s', "l").replace(&['u', 'p'], "l"); - let replacement = misspelled.replace(&['s', 'u'], "l").replace('p', "l"); - println!("replacement"); + let _ = misspelled.replace(&['s', 'u'], "l").replace('p', "l"); - let replacement = misspelled.replace(&['s', u, 'p'], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&['s', u, 'p'], "l"); - let replacement = misspelled.replace(&[s, u, 'p'], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&[s, u, 'p'], "l"); - let replacement = misspelled.replace(&[s, u, p], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&[s, u, p], "l"); - let replacement = misspelled.replace(&[s, u], "l").replace(&[u, p], "l"); - println!("{replacement}"); + let _ = misspelled.replace(&[s, u], "l").replace(&[u, p], "l"); } diff --git a/tests/ui/collapsible_str_replace.stderr b/tests/ui/collapsible_str_replace.stderr new file mode 100644 index 00000000000..372fe1da448 --- /dev/null +++ b/tests/ui/collapsible_str_replace.stderr @@ -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 +