Skip to content

Commit b62c879

Browse files
Centri3Centri3
committed
New lint [string_lit_chars_any]
1 parent d9c24d1 commit b62c879

File tree

7 files changed

+254
-1
lines changed

7 files changed

+254
-1
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5254,6 +5254,7 @@ Released 2018-09-13
52545254
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
52555255
[`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes
52565256
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
5257+
[`string_lit_chars_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_chars_any
52575258
[`string_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_slice
52585259
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
52595260
[`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
@@ -403,6 +403,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
403403
crate::methods::SKIP_WHILE_NEXT_INFO,
404404
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
405405
crate::methods::STRING_EXTEND_CHARS_INFO,
406+
crate::methods::STRING_LIT_CHARS_ANY_INFO,
406407
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
407408
crate::methods::SUSPICIOUS_MAP_INFO,
408409
crate::methods::SUSPICIOUS_SPLITN_INFO,

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ mod skip_while_next;
8585
mod stable_sort_primitive;
8686
mod str_splitn;
8787
mod string_extend_chars;
88+
mod string_lit_chars_any;
8889
mod suspicious_command_arg_space;
8990
mod suspicious_map;
9091
mod suspicious_splitn;
@@ -114,7 +115,7 @@ use clippy_utils::consts::{constant, Constant};
114115
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
115116
use clippy_utils::msrvs::{self, Msrv};
116117
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
117-
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
118+
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
118119
use if_chain::if_chain;
119120
use rustc_hir as hir;
120121
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
@@ -3378,6 +3379,34 @@ declare_clippy_lint! {
33783379
"calling `Stdin::read_line`, then trying to parse it without first trimming"
33793380
}
33803381

3382+
declare_clippy_lint! {
3383+
/// ### What it does
3384+
/// Checks for `<string_lit>.chars().any(|i| i == c)`.
3385+
///
3386+
/// ### Why is this bad?
3387+
/// It's significantly slower than using a pattern instead, like
3388+
/// `matches!(c, '\\' | '.' | '+')`.
3389+
///
3390+
/// Despite this being faster, this is not `perf` as this is pretty common, and is a rather nice
3391+
/// way to check if a `char` is any in a set. In any case, this `restriction` lint is available
3392+
/// for situations where that additional performance is absolutely necessary.
3393+
///
3394+
/// ### Example
3395+
/// ```rust
3396+
/// # let c = 'c';
3397+
/// "\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
3398+
/// ```
3399+
/// Use instead:
3400+
/// ```rust
3401+
/// # let c = 'c';
3402+
/// matches!(c, '\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
3403+
/// ```
3404+
#[clippy::version = "1.72.0"]
3405+
pub STRING_LIT_CHARS_ANY,
3406+
restriction,
3407+
"checks for `<string_lit>.chars().any(|i| i == c)`"
3408+
}
3409+
33813410
pub struct Methods {
33823411
avoid_breaking_exported_api: bool,
33833412
msrv: Msrv,
@@ -3512,6 +3541,7 @@ impl_lint_pass!(Methods => [
35123541
UNNECESSARY_LITERAL_UNWRAP,
35133542
DRAIN_COLLECT,
35143543
MANUAL_TRY_FOLD,
3544+
STRING_LIT_CHARS_ANY,
35153545
]);
35163546

35173547
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3885,6 +3915,13 @@ impl Methods {
38853915
}
38863916
}
38873917
},
3918+
("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
3919+
&& let body = cx.tcx.hir().body(arg.body)
3920+
&& let [param] = body.params
3921+
&& let Some(("chars", recv, _, _, _)) = method_call(recv) =>
3922+
{
3923+
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
3924+
}
38883925
("nth", [n_arg]) => match method_call(recv) {
38893926
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
38903927
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),

‎clippy_lints/src/methods/string_lit_chars_any.rs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::msrvs::{Msrv, MATCHES_MACRO};
3+
use clippy_utils::source::snippet_opt;
4+
use clippy_utils::{is_from_proc_macro, is_trait_method, path_to_local};
5+
use itertools::Itertools;
6+
use rustc_ast::LitKind;
7+
use rustc_errors::Applicability;
8+
use rustc_hir::{BinOpKind, Expr, ExprKind, Param, PatKind};
9+
use rustc_lint::LateContext;
10+
use rustc_span::sym;
11+
12+
use super::STRING_LIT_CHARS_ANY;
13+
14+
pub(super) fn check<'tcx>(
15+
cx: &LateContext<'tcx>,
16+
expr: &'tcx Expr<'tcx>,
17+
recv: &Expr<'_>,
18+
param: &'tcx Param<'tcx>,
19+
body: &Expr<'_>,
20+
msrv: &Msrv,
21+
) {
22+
if msrv.meets(MATCHES_MACRO)
23+
&& is_trait_method(cx, expr, sym::Iterator)
24+
&& let PatKind::Binding(_, arg, _, _) = param.pat.kind
25+
&& let ExprKind::Lit(lit_kind) = recv.kind
26+
&& let LitKind::Str(val, _) = lit_kind.node
27+
&& let ExprKind::Binary(kind, lhs, rhs) = body.kind
28+
&& let BinOpKind::Eq = kind.node
29+
&& let Some(lhs_path) = path_to_local(lhs)
30+
&& let Some(rhs_path) = path_to_local(rhs)
31+
&& let scrutinee = if lhs_path == arg {
32+
rhs
33+
} else if rhs_path == arg {
34+
lhs
35+
} else {
36+
return;
37+
}
38+
&& !is_from_proc_macro(cx, expr)
39+
&& let Some(scrutinee_snip) = snippet_opt(cx, scrutinee.span)
40+
{
41+
// Normalize the char using `map` so `join` doesn't use `Display`, if we don't then
42+
// something like `r"\"` will become `'\'`, which is of course invalid
43+
let pat_snip = val.as_str().chars().map(|c| format!("{c:?}")).join(" | ");
44+
45+
span_lint_and_then(
46+
cx,
47+
STRING_LIT_CHARS_ANY,
48+
expr.span,
49+
"usage of `.chars().any(...)` to check if a char matches any from a string literal",
50+
|diag| {
51+
diag.span_suggestion_verbose(
52+
expr.span,
53+
"use `matches!(...)` instead",
54+
format!("matches!({scrutinee_snip}, {pat_snip})"),
55+
Applicability::MachineApplicable,
56+
);
57+
}
58+
);
59+
}
60+
}

‎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: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::string-lit-chars-any` implied by `-D warnings`
8+
help: use `matches!(...)` instead
9+
|
10+
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
11+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12+
13+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
14+
--> $DIR/string_lit_chars_any.rs:20:5
15+
|
16+
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| x == c);
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
|
19+
help: use `matches!(...)` instead
20+
|
21+
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
22+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23+
24+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
25+
--> $DIR/string_lit_chars_any.rs:21:5
26+
|
27+
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| c == x);
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
|
30+
help: use `matches!(...)` instead
31+
|
32+
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
33+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
34+
35+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
36+
--> $DIR/string_lit_chars_any.rs:22:5
37+
|
38+
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| c == x);
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
|
41+
help: use `matches!(...)` instead
42+
|
43+
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
44+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
45+
46+
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
47+
--> $DIR/string_lit_chars_any.rs:24:5
48+
|
49+
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| { x == c });
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
51+
|
52+
help: use `matches!(...)` instead
53+
|
54+
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
55+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
56+
57+
error: aborting due to 5 previous errors
58+

0 commit comments

Comments
 (0)