Skip to content

Commit 3e8211d

Browse files
Centri3Centri3
committed
New lint [string_lit_chars_any]
1 parent 3d4c536 commit 3e8211d

File tree

7 files changed

+250
-2
lines changed

7 files changed

+250
-2
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5214,6 +5214,7 @@ Released 2018-09-13
52145214
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
52155215
[`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes
52165216
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
5217+
[`string_lit_chars_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_chars_any
52175218
[`string_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_slice
52185219
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
52195220
[`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
400400
crate::methods::SKIP_WHILE_NEXT_INFO,
401401
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
402402
crate::methods::STRING_EXTEND_CHARS_INFO,
403+
crate::methods::STRING_LIT_CHARS_ANY_INFO,
403404
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
404405
crate::methods::SUSPICIOUS_MAP_INFO,
405406
crate::methods::SUSPICIOUS_SPLITN_INFO,

‎clippy_lints/src/methods/mod.rs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ mod skip_while_next;
8383
mod stable_sort_primitive;
8484
mod str_splitn;
8585
mod string_extend_chars;
86+
mod string_lit_chars_any;
8687
mod suspicious_command_arg_space;
8788
mod suspicious_map;
8889
mod suspicious_splitn;
@@ -111,7 +112,7 @@ use clippy_utils::consts::{constant, Constant};
111112
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
112113
use clippy_utils::msrvs::{self, Msrv};
113114
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
114-
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
115+
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
115116
use if_chain::if_chain;
116117
use rustc_hir as hir;
117118
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
@@ -3286,6 +3287,34 @@ declare_clippy_lint! {
32863287
"calling `.drain(..).collect()` to move all elements into a new collection"
32873288
}
32883289

3290+
declare_clippy_lint! {
3291+
/// ### What it does
3292+
/// Checks for `<string_lit>.chars().any(|i| i == c)`.
3293+
///
3294+
/// ### Why is this bad?
3295+
/// It's significantly slower than using a pattern instead, like
3296+
/// `matches!(c, '\\' | '.' | '+')`.
3297+
///
3298+
/// Despite this being faster, this is not `perf` as this is pretty common, and is a rather nice
3299+
/// way to check if a `char` is any in a set. In any case, this `restriction` lint is available
3300+
/// for situations where that additional performance is absolutely necessary.
3301+
///
3302+
/// ### Example
3303+
/// ```rust
3304+
/// # let c = 'c';
3305+
/// "\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
3306+
/// ```
3307+
/// Use instead:
3308+
/// ```rust
3309+
/// # let c = 'c';
3310+
/// matches!(c, '\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
3311+
/// ```
3312+
#[clippy::version = "1.72.0"]
3313+
pub STRING_LIT_CHARS_ANY,
3314+
restriction,
3315+
"checks for `<string_lit>.chars().any(|i| i == c)`"
3316+
}
3317+
32893318
pub struct Methods {
32903319
avoid_breaking_exported_api: bool,
32913320
msrv: Msrv,
@@ -3416,7 +3445,8 @@ impl_lint_pass!(Methods => [
34163445
CLEAR_WITH_DRAIN,
34173446
MANUAL_NEXT_BACK,
34183447
UNNECESSARY_LITERAL_UNWRAP,
3419-
DRAIN_COLLECT
3448+
DRAIN_COLLECT,
3449+
STRING_LIT_CHARS_ANY,
34203450
]);
34213451

34223452
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3787,6 +3817,13 @@ impl Methods {
37873817
}
37883818
}
37893819
},
3820+
("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
3821+
&& let body = cx.tcx.hir().body(arg.body)
3822+
&& let [param] = body.params
3823+
&& let Some(("chars", recv, _, _, _)) = method_call(recv) =>
3824+
{
3825+
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
3826+
}
37903827
("nth", [n_arg]) => match method_call(recv) {
37913828
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
37923829
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),

‎clippy_lints/src/methods/string_lit_chars_any.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
use clippy_utils::{
2+
diagnostics::span_lint_and_sugg,
3+
is_from_proc_macro, is_trait_method,
4+
msrvs::{Msrv, MATCHES_MACRO},
5+
path_to_local,
6+
source::snippet_opt,
7+
};
8+
use itertools::Itertools;
9+
use rustc_ast::LitKind;
10+
use rustc_data_structures::fx::FxHashSet;
11+
use rustc_errors::Applicability;
12+
use rustc_hir::{BinOpKind, Expr, ExprKind, HirId, Param, PatKind};
13+
use rustc_lint::LateContext;
14+
use rustc_span::sym;
15+
16+
use super::STRING_LIT_CHARS_ANY;
17+
18+
pub(super) fn check<'tcx>(
19+
cx: &LateContext<'tcx>,
20+
expr: &'tcx Expr<'tcx>,
21+
recv: &Expr<'_>,
22+
param: &'tcx Param<'tcx>,
23+
body: &Expr<'_>,
24+
msrv: &Msrv,
25+
) {
26+
if !msrv.meets(MATCHES_MACRO) || !is_trait_method(cx, expr, sym::Iterator) {
27+
return;
28+
};
29+
30+
// All bindings introduced by `param`
31+
let mut bindings = FxHashSet::default();
32+
33+
param.pat.walk_always(|pat| {
34+
if let PatKind::Binding(_, hir_id, _, _) = pat.kind {
35+
bindings.insert(hir_id);
36+
}
37+
});
38+
39+
if let ExprKind::Lit(lit_kind) = recv.kind
40+
&& let LitKind::Str(val, _) = lit_kind.node
41+
&& let ExprKind::Binary(kind, lhs, rhs) = body.kind
42+
&& let BinOpKind::Eq = kind.node
43+
&& let Some(scrutinee) = lhs_rhs_are_param_and_scrutinee(&bindings, lhs, rhs)
44+
.or_else(|| lhs_rhs_are_param_and_scrutinee(&bindings, rhs, lhs))
45+
&& !is_from_proc_macro(cx, expr)
46+
&& let Some(scrutinee_snip) = snippet_opt(cx, scrutinee.span)
47+
{
48+
// Normalize the char using `map` so `join` doesn't use `Display`, if we don't then
49+
// something like `r"\"` will become `'\'`, which is of course invalid
50+
let pat_snip = val.as_str().chars().map(|c| format!("{c:?}")).join(" | ");
51+
52+
span_lint_and_sugg(
53+
cx,
54+
STRING_LIT_CHARS_ANY,
55+
expr.span,
56+
"usage of `.chars().any(...)` to check if a char matches any from a string literal",
57+
"use `matches!(...)` instead",
58+
format!("matches!({scrutinee_snip}, {pat_snip})"),
59+
Applicability::MachineApplicable,
60+
);
61+
}
62+
}
63+
64+
/// Checks if `one` is contained within `bindings` (and so is a closure parameter), and that `two`
65+
/// is a local. As `.chars().any(...)` can only introduce one binding, we know `two` is not from the
66+
/// closure if we only call this on an `ExprKind::Binary`.
67+
fn lhs_rhs_are_param_and_scrutinee<'tcx>(
68+
bindings: &FxHashSet<HirId>,
69+
one: &'tcx Expr<'tcx>,
70+
two: &'tcx Expr<'tcx>,
71+
) -> Option<&'tcx Expr<'tcx>> {
72+
if path_to_local(one).is_some_and(|hir_id| bindings.contains(&hir_id))
73+
&& path_to_local(two).is_some_and(|hir_id| !bindings.contains(&hir_id))
74+
{
75+
return Some(two);
76+
}
77+
78+
None
79+
}

‎tests/ui/string_lit_chars_any.fixed

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::needless_raw_string_hashes, clippy::no_effect, unused)]
4+
#![warn(clippy::string_lit_chars_any)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
struct NotStringLit;
10+
11+
impl NotStringLit {
12+
fn chars(&self) -> impl Iterator<Item = char> {
13+
"c".chars()
14+
}
15+
}
16+
17+
fn main() {
18+
let c = 'c';
19+
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
20+
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
21+
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
22+
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
23+
#[rustfmt::skip]
24+
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
25+
// Do not lint
26+
NotStringLit.chars().any(|x| x == c);
27+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
28+
let c = 'c';
29+
x == c
30+
});
31+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
32+
1;
33+
x == c
34+
});
35+
matches!(
36+
c,
37+
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
38+
);
39+
external! {
40+
let c = 'c';
41+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
42+
}
43+
with_span! {
44+
span
45+
let c = 'c';
46+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
47+
}
48+
}

‎tests/ui/string_lit_chars_any.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::needless_raw_string_hashes, clippy::no_effect, unused)]
4+
#![warn(clippy::string_lit_chars_any)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
struct NotStringLit;
10+
11+
impl NotStringLit {
12+
fn chars(&self) -> impl Iterator<Item = char> {
13+
"c".chars()
14+
}
15+
}
16+
17+
fn main() {
18+
let c = 'c';
19+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
20+
r#"\.+*?()|[]{}^$#&-~"#.chars().any(|x| x == c);
21+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| c == x);
22+
r#"\.+*?()|[]{}^$#&-~"#.chars().any(|x| c == x);
23+
#[rustfmt::skip]
24+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| { x == c });
25+
// Do not lint
26+
NotStringLit.chars().any(|x| x == c);
27+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
28+
let c = 'c';
29+
x == c
30+
});
31+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
32+
1;
33+
x == c
34+
});
35+
matches!(
36+
c,
37+
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
38+
);
39+
external! {
40+
let c = 'c';
41+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
42+
}
43+
with_span! {
44+
span
45+
let c = 'c';
46+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
47+
}
48+
}

‎tests/ui/string_lit_chars_any.stderr

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
2+
--> $DIR/string_lit_chars_any.rs:19:5
3+
|
4+
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `matches!(...)` instead: `matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~')`
6+
|
7+
= note: `-D clippy::string-lit-chars-any` implied by `-D warnings`
8+
9+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
10+
--> $DIR/string_lit_chars_any.rs:20:5
11+
|
12+
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| x == c);
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `matches!(...)` instead: `matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~')`
14+
15+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
16+
--> $DIR/string_lit_chars_any.rs:21:5
17+
|
18+
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| c == x);
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `matches!(...)` instead: `matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~')`
20+
21+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
22+
--> $DIR/string_lit_chars_any.rs:22:5
23+
|
24+
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| c == x);
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `matches!(...)` instead: `matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~')`
26+
27+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
28+
--> $DIR/string_lit_chars_any.rs:24:5
29+
|
30+
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| { x == c });
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `matches!(...)` instead: `matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~')`
32+
33+
error: aborting due to 5 previous errors
34+

0 commit comments

Comments
 (0)