Skip to content

Commit 6e86687

Browse files
committed
Handle replace calls with char slices
1 parent 89698b9 commit 6e86687

File tree

3 files changed

+167
-16
lines changed

3 files changed

+167
-16
lines changed
Lines changed: 151 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,154 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
// use clippy_utils::source::snippet_with_context;
3+
use clippy_utils::visitors::for_each_expr;
4+
use core::ops::ControlFlow;
5+
use if_chain::if_chain;
6+
use rustc_ast::ast::LitKind;
7+
use rustc_errors::Applicability;
18
use rustc_hir as hir;
29
use rustc_hir::*;
3-
use rustc_lint::{LateContext, LateLintPass};
4-
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_lint::LateContext;
11+
use rustc_middle::ty;
12+
use rustc_span::source_map::Spanned;
13+
use std::unreachable;
14+
// use rustc_span::Span;
515

6-
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {}
16+
use super::method_call;
17+
use super::COLLAPSIBLE_STR_REPLACE;
18+
19+
pub(super) fn check<'tcx>(
20+
cx: &LateContext<'tcx>,
21+
expr: &'tcx hir::Expr<'tcx>,
22+
name: &str,
23+
recv: &'tcx hir::Expr<'tcx>,
24+
args: &'tcx [hir::Expr<'tcx>],
25+
) {
26+
match (name, args) {
27+
("replace", [from, to]) => {
28+
// Check for `str::replace` calls with char slice for linting
29+
let original_recv = find_original_recv(recv);
30+
let original_recv_ty = cx.typeck_results().expr_ty(original_recv).peel_refs();
31+
if_chain! {
32+
// Check the receiver of the method call is `str` type
33+
if matches!(original_recv_ty.kind(), ty::Str);
34+
let from_ty = cx.typeck_results().expr_ty(from).peel_refs();
35+
if let ty::Array(array_ty, _) = from_ty.kind();
36+
if matches!(array_ty.kind(), ty::Char);
37+
then {
38+
check_replace_call_with_char_slice(cx, from, to);
39+
}
40+
}
41+
42+
match method_call(recv) {
43+
// Check if there's an earlier `str::replace` call
44+
Some(("replace", [prev_recv, prev_from, prev_to], prev_span)) => {
45+
println!("Consecutive replace calls");
46+
// Check that the original receiver is of `ty::Str` type
47+
// Check that all the `from` args are char literals
48+
// Check that all the `to` args are the same variable or has the same &str value
49+
// If so, then lint
50+
},
51+
_ => {},
52+
}
53+
},
54+
_ => {},
55+
}
56+
}
57+
58+
fn find_original_recv<'tcx>(recv: &'tcx hir::Expr<'tcx>) -> &'tcx hir::Expr<'tcx> {
59+
let mut original_recv = recv;
60+
61+
let _: Option<()> = for_each_expr(recv, |e| {
62+
if let Some((name, [prev_recv, args @ ..], _)) = method_call(e) {
63+
match (name, args) {
64+
("replace", [_, _]) => {
65+
original_recv = prev_recv;
66+
ControlFlow::Continue(())
67+
},
68+
_ => ControlFlow::BREAK,
69+
}
70+
} else {
71+
ControlFlow::Continue(())
72+
}
73+
});
74+
75+
original_recv
76+
}
77+
78+
fn check_replace_call_with_char_slice<'tcx>(
79+
cx: &LateContext<'tcx>,
80+
from_arg: &'tcx hir::Expr<'tcx>,
81+
to_arg: &'tcx hir::Expr<'tcx>,
82+
) {
83+
let mut has_no_var = true;
84+
let mut char_list: Vec<char> = Vec::new();
85+
// Go through the `from_arg` to collect all char literals
86+
let _: Option<()> = for_each_expr(from_arg, |e| {
87+
if let ExprKind::Lit(Spanned {
88+
node: LitKind::Char(val),
89+
..
90+
}) = e.kind
91+
{
92+
char_list.push(val);
93+
ControlFlow::Continue(())
94+
} else if let ExprKind::Path(..) = e.kind {
95+
// If a variable is found in the char slice, no lint for first version of this lint
96+
has_no_var = false;
97+
ControlFlow::BREAK
98+
} else {
99+
ControlFlow::Continue(())
100+
}
101+
});
102+
103+
if has_no_var {
104+
let to_arg_repr = match to_arg.kind {
105+
ExprKind::Lit(Spanned {
106+
node: LitKind::Str(to_arg_val, _),
107+
..
108+
}) => {
109+
let repr = to_arg_val.as_str();
110+
let double_quote = "\"";
111+
double_quote.to_owned() + repr + double_quote
112+
},
113+
ExprKind::Path(QPath::Resolved(
114+
_,
115+
Path {
116+
segments: path_segments,
117+
..
118+
},
119+
)) => {
120+
// join the path_segments values by "::"
121+
let path_segment_ident_names: Vec<&str> = path_segments
122+
.iter()
123+
.map(|path_seg| path_seg.ident.name.as_str())
124+
.collect();
125+
126+
path_segment_ident_names.join("::")
127+
},
128+
_ => unreachable!(),
129+
};
130+
131+
let app = Applicability::MachineApplicable;
132+
span_lint_and_sugg(
133+
cx,
134+
COLLAPSIBLE_STR_REPLACE,
135+
from_arg.span,
136+
"used slice of chars in `str::replace` call",
137+
"replace with",
138+
format!(
139+
"replace(|c| matches!(c, {}), {})",
140+
format_slice_of_chars_for_sugg(&char_list),
141+
to_arg_repr,
142+
),
143+
app,
144+
);
145+
}
146+
}
147+
148+
fn format_slice_of_chars_for_sugg(chars: &Vec<char>) -> String {
149+
let single_quoted_chars: Vec<String> = chars
150+
.iter()
151+
.map(|c| "'".to_owned() + &c.to_string() + &"'".to_owned())
152+
.collect();
153+
single_quoted_chars.join(" | ")
154+
}

clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3519,7 +3519,7 @@ impl Methods {
35193519
("sort_unstable_by", [arg]) => {
35203520
unnecessary_sort_by::check(cx, expr, recv, arg, true);
35213521
},
3522-
("replace", [_, _]) => collapsible_str_replace::check(cx, expr, recv, args),
3522+
("replace", [_, _]) => collapsible_str_replace::check(cx, expr, name, recv, args),
35233523
("splitn" | "rsplitn", [count_arg, pat_arg]) => {
35243524
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
35253525
suspicious_splitn::check(cx, name, expr, recv, count);

tests/ui/collapsible_str_replace.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,28 @@ fn main() {
1010
let p = 'p';
1111
let s = 's';
1212
let u = 'u';
13+
let l = "l";
1314

1415
// LINT CASES
1516
// If the first argument to a single `str::replace` call is a slice and none of the chars
1617
// are variables, recommend `collapsible_str_replace`
1718
let replacement = misspelled.replace(&['s', 'u', 'p'], "l");
1819
println!("{replacement}");
1920

21+
let replacement = misspelled.replace(&['s', 'u', 'p'], l);
22+
println!("{replacement}");
23+
24+
// If multiple `str::replace` calls contain slices and none of the chars are variables,
25+
// recommend `collapsible_str_replace`
26+
let replacement = misspelled.replace(&['s', 'u'], "l").replace(&['u', 'p'], "l");
27+
println!("{replacement}");
28+
29+
let replacement = misspelled.replace('s', "l").replace(&['u', 'p'], "l");
30+
println!("{replacement}");
31+
32+
let replacement = misspelled.replace(&['s', 'u'], "l").replace('p', "l");
33+
println!("replacement");
34+
2035
// If there are consecutive calls to `str::replace` and none of the chars are variables,
2136
// recommend `collapsible_str_replace`
2237
let replacement = misspelled.replace('s', "l").replace('u', "l");
@@ -38,18 +53,6 @@ fn main() {
3853
let replacement = misspelled.replace(&get_filter(), "l");
3954

4055
// NO LINT TIL IMPROVEMENT
41-
// If multiple `str::replace` calls contain slices and none of the chars are variables,
42-
// the first iteration does not recommend `collapsible_str_replace`
43-
let replacement = misspelled.replace(&['s', 'u', 'p'], "l").replace(&['s', 'p'], "l");
44-
println!("{replacement}");
45-
46-
// If a mixture of `str::replace` calls with slice and char arguments are used for `from` arg,
47-
// the first iteration does not recommend `collapsible_str_replace`
48-
let replacement = misspelled.replace(&['s', 'u'], "l").replace('p', "l");
49-
println!("replacement");
50-
51-
let replacement = misspelled.replace('p', "l").replace(&['s', 'u'], "l");
52-
5356
// The first iteration of `collapsible_str_replace` will not create lint if the first argument to
5457
// a single `str::replace` call is a slice and one or more of its chars are variables
5558
let replacement = misspelled.replace(&['s', u, 'p'], "l");

0 commit comments

Comments
 (0)