Auto merge of #9269 - nahuakang:collapsible_str_replace, r=flip1995

Lint `collapsible_str_replace`

fixes #6651

```
changelog: [`collapsible_str_replace`]: create new lint `collapsible_str_replace`
```

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
This commit is contained in:
bors 2022-08-20 13:44:35 +00:00
commit 5820addb24
9 changed files with 371 additions and 3 deletions

View File

@ -3642,6 +3642,7 @@ Released 2018-09-13
[`collapsible_else_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if
[`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
[`collapsible_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match
[`collapsible_str_replace`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_str_replace
[`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime

View File

@ -154,6 +154,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::CHARS_NEXT_CMP),
LintId::of(methods::CLONE_DOUBLE_REF),
LintId::of(methods::CLONE_ON_COPY),
LintId::of(methods::COLLAPSIBLE_STR_REPLACE),
LintId::of(methods::ERR_EXPECT),
LintId::of(methods::EXPECT_FUN_CALL),
LintId::of(methods::EXTEND_WITH_DRAIN),

View File

@ -287,6 +287,7 @@ store.register_lints(&[
methods::CLONE_DOUBLE_REF,
methods::CLONE_ON_COPY,
methods::CLONE_ON_REF_PTR,
methods::COLLAPSIBLE_STR_REPLACE,
methods::ERR_EXPECT,
methods::EXPECT_FUN_CALL,
methods::EXPECT_USED,

View File

@ -13,6 +13,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
LintId::of(loops::MISSING_SPIN_LOOP),
LintId::of(loops::NEEDLESS_COLLECT),
LintId::of(manual_retain::MANUAL_RETAIN),
LintId::of(methods::COLLAPSIBLE_STR_REPLACE),
LintId::of(methods::EXPECT_FUN_CALL),
LintId::of(methods::EXTEND_WITH_DRAIN),
LintId::of(methods::ITER_NTH),

View File

@ -0,0 +1,96 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{eq_expr_value, get_parent_expr};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use std::collections::VecDeque;
use super::method_call;
use super::COLLAPSIBLE_STR_REPLACE;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
from: &'tcx hir::Expr<'tcx>,
to: &'tcx hir::Expr<'tcx>,
) {
let replace_methods = collect_replace_calls(cx, expr, to);
if replace_methods.methods.len() > 1 {
let from_kind = cx.typeck_results().expr_ty(from).peel_refs().kind();
// If the parent node's `to` argument is the same as the `to` argument
// of the last replace call in the current chain, don't lint as it was already linted
if let Some(parent) = get_parent_expr(cx, expr)
&& let Some(("replace", [_, current_from, current_to], _)) = method_call(parent)
&& eq_expr_value(cx, to, current_to)
&& from_kind == cx.typeck_results().expr_ty(current_from).peel_refs().kind()
{
return;
}
check_consecutive_replace_calls(cx, expr, &replace_methods, to);
}
}
struct ReplaceMethods<'tcx> {
methods: VecDeque<&'tcx hir::Expr<'tcx>>,
from_args: VecDeque<&'tcx hir::Expr<'tcx>>,
}
fn collect_replace_calls<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
to_arg: &'tcx hir::Expr<'tcx>,
) -> ReplaceMethods<'tcx> {
let mut methods = VecDeque::new();
let mut from_args = VecDeque::new();
let _: Option<()> = for_each_expr(expr, |e| {
if let Some(("replace", [_, from, to], _)) = method_call(e) {
if eq_expr_value(cx, to_arg, to) && cx.typeck_results().expr_ty(from).peel_refs().is_char() {
methods.push_front(e);
from_args.push_front(from);
ControlFlow::Continue(())
} else {
ControlFlow::BREAK
}
} else {
ControlFlow::Continue(())
}
});
ReplaceMethods { methods, from_args }
}
/// Check a chain of `str::replace` calls for `collapsible_str_replace` lint.
fn check_consecutive_replace_calls<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
replace_methods: &ReplaceMethods<'tcx>,
to_arg: &'tcx hir::Expr<'tcx>,
) {
let from_args = &replace_methods.from_args;
let from_arg_reprs: Vec<String> = from_args
.iter()
.map(|from_arg| snippet(cx, from_arg.span, "..").to_string())
.collect();
let app = Applicability::MachineApplicable;
let earliest_replace_call = replace_methods.methods.front().unwrap();
if let Some((_, [..], span_lo)) = method_call(earliest_replace_call) {
span_lint_and_sugg(
cx,
COLLAPSIBLE_STR_REPLACE,
expr.span.with_lo(span_lo.lo()),
"used consecutive `str::replace` call",
"replace with",
format!(
"replace([{}], {})",
from_arg_reprs.join(", "),
snippet(cx, to_arg.span, ".."),
),
app,
);
}
}

View File

@ -12,6 +12,7 @@ mod chars_next_cmp_with_unwrap;
mod clone_on_copy;
mod clone_on_ref_ptr;
mod cloned_instead_of_copied;
mod collapsible_str_replace;
mod err_expect;
mod expect_fun_call;
mod expect_used;
@ -137,6 +138,32 @@ declare_clippy_lint! {
"used `cloned` where `copied` could be used instead"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for consecutive calls to `str::replace` (2 or more)
/// that can be collapsed into a single call.
///
/// ### Why is this bad?
/// Consecutive `str::replace` calls scan the string multiple times
/// with repetitive code.
///
/// ### Example
/// ```rust
/// let hello = "hesuo worpd"
/// .replace('s', "l")
/// .replace("u", "l")
/// .replace('p', "l");
/// ```
/// Use instead:
/// ```rust
/// let hello = "hesuo worpd".replace(&['s', 'u', 'p'], "l");
/// ```
#[clippy::version = "1.64.0"]
pub COLLAPSIBLE_STR_REPLACE,
perf,
"collapse consecutive calls to str::replace (2 or more) into a single call"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `_.cloned().<func>()` where call to `.cloned()` can be postponed.
@ -3001,6 +3028,7 @@ impl_lint_pass!(Methods => [
CLONE_ON_COPY,
CLONE_ON_REF_PTR,
CLONE_DOUBLE_REF,
COLLAPSIBLE_STR_REPLACE,
ITER_OVEREAGER_CLONED,
CLONED_INSTEAD_OF_COPIED,
FLAT_MAP_OPTION,
@ -3479,6 +3507,14 @@ impl Methods {
("repeat", [arg]) => {
repeat_once::check(cx, expr, recv, arg);
},
(name @ ("replace" | "replacen"), [arg1, arg2] | [arg1, arg2, _]) => {
no_effect_replace::check(cx, expr, arg1, arg2);
// Check for repeated `str::replace` calls to perform `collapsible_str_replace` lint
if name == "replace" && let Some(("replace", ..)) = method_call(recv) {
collapsible_str_replace::check(cx, expr, arg1, arg2);
}
},
("resize", [count_arg, default_arg]) => {
vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span);
},
@ -3556,9 +3592,6 @@ impl Methods {
unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");
},
},
("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => {
no_effect_replace::check(cx, expr, arg1, arg2);
},
("zip", [arg]) => {
if let ExprKind::MethodCall(name, [iter_recv], _) = recv.kind
&& name.ident.name == sym::iter

View File

@ -0,0 +1,73 @@
// run-rustfix
#![warn(clippy::collapsible_str_replace)]
fn get_filter() -> char {
'u'
}
fn main() {
let d = 'd';
let p = 'p';
let s = 's';
let u = 'u';
let l = "l";
let mut iter = ["l", "z"].iter();
// LINT CASES
let _ = "hesuo worpd".replace(['s', 'u'], "l");
let _ = "hesuo worpd".replace(['s', 'u'], l);
let _ = "hesuo worpd".replace(['s', 'u', 'p'], "l");
let _ = "hesuo worpd"
.replace(['s', 'u', 'p', 'd'], "l");
let _ = "hesuo world".replace([s, 'u'], "l");
let _ = "hesuo worpd".replace([s, 'u', 'p'], "l");
let _ = "hesuo worpd".replace([s, u, 'p'], "l");
let _ = "hesuo worpd".replace([s, u, p], "l");
let _ = "hesuo worlp".replace(['s', 'u'], "l").replace('p', "d");
let _ = "hesuo worpd".replace('s', "x").replace(['u', 'p'], "l");
// Note: Future iterations could lint `replace(|c| matches!(c, "su" | 'd' | 'p'), "l")`
let _ = "hesudo worpd".replace("su", "l").replace(['d', 'p'], "l");
let _ = "hesudo worpd".replace([d, 'p'], "l").replace("su", "l");
let _ = "hesuo world".replace([get_filter(), 's'], "l");
// NO LINT CASES
let _ = "hesuo world".replace('s', "l").replace('u', "p");
let _ = "hesuo worpd".replace('s', "l").replace('p', l);
let _ = "hesudo worpd".replace('d', "l").replace("su", "l").replace('p', "l");
// Note: Future iterations of `collapsible_str_replace` might lint this and combine to `[s, u, p]`
let _ = "hesuo worpd".replace([s, u], "l").replace([u, p], "l");
let _ = "hesuo worpd".replace(['s', 'u'], "l").replace(['u', 'p'], "l");
let _ = "hesuo worpd".replace('s', "l").replace(['u', 'p'], "l");
let _ = "hesuo worpd".replace(['s', 'u', 'p'], "l").replace('r', "l");
let _ = "hesuo worpd".replace(['s', 'u', 'p'], l).replace('r', l);
let _ = "hesuo worpd".replace(['s', u, 'p'], "l").replace('r', "l");
let _ = "hesuo worpd".replace([s, u], "l").replace(p, "l");
// Regression test
let _ = "hesuo worpd"
.replace('u', iter.next().unwrap())
.replace('s', iter.next().unwrap());
}

View File

@ -0,0 +1,76 @@
// run-rustfix
#![warn(clippy::collapsible_str_replace)]
fn get_filter() -> char {
'u'
}
fn main() {
let d = 'd';
let p = 'p';
let s = 's';
let u = 'u';
let l = "l";
let mut iter = ["l", "z"].iter();
// LINT CASES
let _ = "hesuo worpd".replace('s', "l").replace('u', "l");
let _ = "hesuo worpd".replace('s', l).replace('u', l);
let _ = "hesuo worpd".replace('s', "l").replace('u', "l").replace('p', "l");
let _ = "hesuo worpd"
.replace('s', "l")
.replace('u', "l")
.replace('p', "l")
.replace('d', "l");
let _ = "hesuo world".replace(s, "l").replace('u', "l");
let _ = "hesuo worpd".replace(s, "l").replace('u', "l").replace('p', "l");
let _ = "hesuo worpd".replace(s, "l").replace(u, "l").replace('p', "l");
let _ = "hesuo worpd".replace(s, "l").replace(u, "l").replace(p, "l");
let _ = "hesuo worlp".replace('s', "l").replace('u', "l").replace('p', "d");
let _ = "hesuo worpd".replace('s', "x").replace('u', "l").replace('p', "l");
// Note: Future iterations could lint `replace(|c| matches!(c, "su" | 'd' | 'p'), "l")`
let _ = "hesudo worpd".replace("su", "l").replace('d', "l").replace('p', "l");
let _ = "hesudo worpd".replace(d, "l").replace('p', "l").replace("su", "l");
let _ = "hesuo world".replace(get_filter(), "l").replace('s', "l");
// NO LINT CASES
let _ = "hesuo world".replace('s', "l").replace('u', "p");
let _ = "hesuo worpd".replace('s', "l").replace('p', l);
let _ = "hesudo worpd".replace('d', "l").replace("su", "l").replace('p', "l");
// Note: Future iterations of `collapsible_str_replace` might lint this and combine to `[s, u, p]`
let _ = "hesuo worpd".replace([s, u], "l").replace([u, p], "l");
let _ = "hesuo worpd".replace(['s', 'u'], "l").replace(['u', 'p'], "l");
let _ = "hesuo worpd".replace('s', "l").replace(['u', 'p'], "l");
let _ = "hesuo worpd".replace(['s', 'u', 'p'], "l").replace('r', "l");
let _ = "hesuo worpd".replace(['s', 'u', 'p'], l).replace('r', l);
let _ = "hesuo worpd".replace(['s', u, 'p'], "l").replace('r', "l");
let _ = "hesuo worpd".replace([s, u], "l").replace(p, "l");
// Regression test
let _ = "hesuo worpd"
.replace('u', iter.next().unwrap())
.replace('s', iter.next().unwrap());
}

View File

@ -0,0 +1,86 @@
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:19:27
|
LL | let _ = "hesuo worpd".replace('s', "l").replace('u', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['s', 'u'], "l")`
|
= note: `-D clippy::collapsible-str-replace` implied by `-D warnings`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:21:27
|
LL | let _ = "hesuo worpd".replace('s', l).replace('u', l);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['s', 'u'], l)`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:23:27
|
LL | let _ = "hesuo worpd".replace('s', "l").replace('u', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['s', 'u', 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:26:10
|
LL | .replace('s', "l")
| __________^
LL | | .replace('u', "l")
LL | | .replace('p', "l")
LL | | .replace('d', "l");
| |__________________________^ help: replace with: `replace(['s', 'u', 'p', 'd'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:31:27
|
LL | let _ = "hesuo world".replace(s, "l").replace('u', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([s, 'u'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:33:27
|
LL | let _ = "hesuo worpd".replace(s, "l").replace('u', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([s, 'u', 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:35:27
|
LL | let _ = "hesuo worpd".replace(s, "l").replace(u, "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([s, u, 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:37:27
|
LL | let _ = "hesuo worpd".replace(s, "l").replace(u, "l").replace(p, "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([s, u, p], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:39:27
|
LL | let _ = "hesuo worlp".replace('s', "l").replace('u', "l").replace('p', "d");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['s', 'u'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:41:45
|
LL | let _ = "hesuo worpd".replace('s', "x").replace('u', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['u', 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:44:47
|
LL | let _ = "hesudo worpd".replace("su", "l").replace('d', "l").replace('p', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace(['d', 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:46:28
|
LL | let _ = "hesudo worpd".replace(d, "l").replace('p', "l").replace("su", "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([d, 'p'], "l")`
error: used consecutive `str::replace` call
--> $DIR/collapsible_str_replace.rs:48:27
|
LL | let _ = "hesuo world".replace(get_filter(), "l").replace('s', "l");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `replace([get_filter(), 's'], "l")`
error: aborting due to 13 previous errors