Skip to content

Commit ec57155

Browse files
borsbors
committed
Auto merge of rust-lang#11052 - Centri3:string_lit_chars_any, r=Jarcho
New lint [`string_lit_chars_any`] Closes rust-lang#10389 This lint can probably be deprecated if/when rustc optimizes `.chars().any(...)`. changelog: New lint [`string_lit_chars_any`]
2 parents d4a6634 + 9d08502 commit ec57155

7 files changed

+256
-1
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5256,6 +5256,7 @@ Released 2018-09-13
52565256
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
52575257
[`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes
52585258
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
5259+
[`string_lit_chars_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_chars_any
52595260
[`string_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_slice
52605261
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
52615262
[`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
@@ -405,6 +405,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
405405
crate::methods::SKIP_WHILE_NEXT_INFO,
406406
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
407407
crate::methods::STRING_EXTEND_CHARS_INFO,
408+
crate::methods::STRING_LIT_CHARS_ANY_INFO,
408409
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
409410
crate::methods::SUSPICIOUS_MAP_INFO,
410411
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
@@ -86,6 +86,7 @@ mod skip_while_next;
8686
mod stable_sort_primitive;
8787
mod str_splitn;
8888
mod string_extend_chars;
89+
mod string_lit_chars_any;
8990
mod suspicious_command_arg_space;
9091
mod suspicious_map;
9192
mod suspicious_splitn;
@@ -115,7 +116,7 @@ use clippy_utils::consts::{constant, Constant};
115116
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
116117
use clippy_utils::msrvs::{self, Msrv};
117118
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
118-
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
119+
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
119120
use if_chain::if_chain;
120121
use rustc_hir as hir;
121122
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
@@ -3379,6 +3380,34 @@ declare_clippy_lint! {
33793380
"calling `Stdin::read_line`, then trying to parse it without first trimming"
33803381
}
33813382

3383+
declare_clippy_lint! {
3384+
/// ### What it does
3385+
/// Checks for `<string_lit>.chars().any(|i| i == c)`.
3386+
///
3387+
/// ### Why is this bad?
3388+
/// It's significantly slower than using a pattern instead, like
3389+
/// `matches!(c, '\\' | '.' | '+')`.
3390+
///
3391+
/// Despite this being faster, this is not `perf` as this is pretty common, and is a rather nice
3392+
/// way to check if a `char` is any in a set. In any case, this `restriction` lint is available
3393+
/// for situations where that additional performance is absolutely necessary.
3394+
///
3395+
/// ### Example
3396+
/// ```rust
3397+
/// # let c = 'c';
3398+
/// "\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
3399+
/// ```
3400+
/// Use instead:
3401+
/// ```rust
3402+
/// # let c = 'c';
3403+
/// matches!(c, '\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
3404+
/// ```
3405+
#[clippy::version = "1.72.0"]
3406+
pub STRING_LIT_CHARS_ANY,
3407+
restriction,
3408+
"checks for `<string_lit>.chars().any(|i| i == c)`"
3409+
}
3410+
33823411
declare_clippy_lint! {
33833412
/// ### What it does
33843413
/// Checks for usage of `.map(|_| format!(..)).collect::<String>()`.
@@ -3549,6 +3578,7 @@ impl_lint_pass!(Methods => [
35493578
DRAIN_COLLECT,
35503579
MANUAL_TRY_FOLD,
35513580
FORMAT_COLLECT,
3581+
STRING_LIT_CHARS_ANY,
35523582
]);
35533583

35543584
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3923,6 +3953,13 @@ impl Methods {
39233953
}
39243954
}
39253955
},
3956+
("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
3957+
&& let body = cx.tcx.hir().body(arg.body)
3958+
&& let [param] = body.params
3959+
&& let Some(("chars", recv, _, _, _)) = method_call(recv) =>
3960+
{
3961+
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
3962+
}
39263963
("nth", [n_arg]) => match method_call(recv) {
39273964
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
39283965
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),

‎clippy_lints/src/methods/string_lit_chars_any.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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 = match (lhs_path == arg, rhs_path == arg) {
32+
(true, false) => rhs,
33+
(false, true) => lhs,
34+
_ => return,
35+
}
36+
&& !is_from_proc_macro(cx, expr)
37+
&& let Some(scrutinee_snip) = snippet_opt(cx, scrutinee.span)
38+
{
39+
// Normalize the char using `map` so `join` doesn't use `Display`, if we don't then
40+
// something like `r"\"` will become `'\'`, which is of course invalid
41+
let pat_snip = val.as_str().chars().map(|c| format!("{c:?}")).join(" | ");
42+
43+
span_lint_and_then(
44+
cx,
45+
STRING_LIT_CHARS_ANY,
46+
expr.span,
47+
"usage of `.chars().any(...)` to check if a char matches any from a string literal",
48+
|diag| {
49+
diag.span_suggestion_verbose(
50+
expr.span,
51+
"use `matches!(...)` instead",
52+
format!("matches!({scrutinee_snip}, {pat_snip})"),
53+
Applicability::MachineApplicable,
54+
);
55+
}
56+
);
57+
}
58+
}

‎tests/ui/string_lit_chars_any.fixed

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::eq_op, 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+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == x);
36+
"\\.+*?()|[]{}^$#&-~".chars().any(|_x| c == c);
37+
matches!(
38+
c,
39+
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
40+
);
41+
external! {
42+
let c = 'c';
43+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
44+
}
45+
with_span! {
46+
span
47+
let c = 'c';
48+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
49+
}
50+
}

‎tests/ui/string_lit_chars_any.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::eq_op, 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+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == x);
36+
"\\.+*?()|[]{}^$#&-~".chars().any(|_x| c == c);
37+
matches!(
38+
c,
39+
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
40+
);
41+
external! {
42+
let c = 'c';
43+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
44+
}
45+
with_span! {
46+
span
47+
let c = 'c';
48+
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
49+
}
50+
}

‎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)