Skip to content

New lint [string_lit_chars_any] #11052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5256,6 +5256,7 @@ Released 2018-09-13
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
[`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
[`string_lit_chars_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_chars_any
[`string_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_slice
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
[`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::SKIP_WHILE_NEXT_INFO,
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
crate::methods::STRING_EXTEND_CHARS_INFO,
crate::methods::STRING_LIT_CHARS_ANY_INFO,
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
crate::methods::SUSPICIOUS_MAP_INFO,
crate::methods::SUSPICIOUS_SPLITN_INFO,
Expand Down
39 changes: 38 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ mod skip_while_next;
mod stable_sort_primitive;
mod str_splitn;
mod string_extend_chars;
mod string_lit_chars_any;
mod suspicious_command_arg_space;
mod suspicious_map;
mod suspicious_splitn;
Expand Down Expand Up @@ -115,7 +116,7 @@ use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
use if_chain::if_chain;
use rustc_hir as hir;
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
Expand Down Expand Up @@ -3379,6 +3380,34 @@ declare_clippy_lint! {
"calling `Stdin::read_line`, then trying to parse it without first trimming"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for `<string_lit>.chars().any(|i| i == c)`.
///
/// ### Why is this bad?
/// It's significantly slower than using a pattern instead, like
/// `matches!(c, '\\' | '.' | '+')`.
///
/// Despite this being faster, this is not `perf` as this is pretty common, and is a rather nice
/// way to check if a `char` is any in a set. In any case, this `restriction` lint is available
/// for situations where that additional performance is absolutely necessary.
///
/// ### Example
/// ```rust
/// # let c = 'c';
/// "\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
/// ```
/// Use instead:
/// ```rust
/// # let c = 'c';
/// matches!(c, '\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
/// ```
#[clippy::version = "1.72.0"]
pub STRING_LIT_CHARS_ANY,
restriction,
"checks for `<string_lit>.chars().any(|i| i == c)`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.map(|_| format!(..)).collect::<String>()`.
Expand Down Expand Up @@ -3549,6 +3578,7 @@ impl_lint_pass!(Methods => [
DRAIN_COLLECT,
MANUAL_TRY_FOLD,
FORMAT_COLLECT,
STRING_LIT_CHARS_ANY,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -3923,6 +3953,13 @@ impl Methods {
}
}
},
("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
&& let body = cx.tcx.hir().body(arg.body)
&& let [param] = body.params
&& let Some(("chars", recv, _, _, _)) = method_call(recv) =>
{
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
}
("nth", [n_arg]) => match method_call(recv) {
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
Expand Down
58 changes: 58 additions & 0 deletions clippy_lints/src/methods/string_lit_chars_any.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{Msrv, MATCHES_MACRO};
use clippy_utils::source::snippet_opt;
use clippy_utils::{is_from_proc_macro, is_trait_method, path_to_local};
use itertools::Itertools;
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, Param, PatKind};
use rustc_lint::LateContext;
use rustc_span::sym;

use super::STRING_LIT_CHARS_ANY;

pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
recv: &Expr<'_>,
param: &'tcx Param<'tcx>,
body: &Expr<'_>,
msrv: &Msrv,
) {
if msrv.meets(MATCHES_MACRO)
&& is_trait_method(cx, expr, sym::Iterator)
&& let PatKind::Binding(_, arg, _, _) = param.pat.kind
&& let ExprKind::Lit(lit_kind) = recv.kind
&& let LitKind::Str(val, _) = lit_kind.node
&& let ExprKind::Binary(kind, lhs, rhs) = body.kind
&& let BinOpKind::Eq = kind.node
&& let Some(lhs_path) = path_to_local(lhs)
&& let Some(rhs_path) = path_to_local(rhs)
&& let scrutinee = match (lhs_path == arg, rhs_path == arg) {
(true, false) => rhs,
(false, true) => lhs,
_ => return,
}
&& !is_from_proc_macro(cx, expr)
&& let Some(scrutinee_snip) = snippet_opt(cx, scrutinee.span)
{
// Normalize the char using `map` so `join` doesn't use `Display`, if we don't then
// something like `r"\"` will become `'\'`, which is of course invalid
let pat_snip = val.as_str().chars().map(|c| format!("{c:?}")).join(" | ");

span_lint_and_then(
cx,
STRING_LIT_CHARS_ANY,
expr.span,
"usage of `.chars().any(...)` to check if a char matches any from a string literal",
|diag| {
diag.span_suggestion_verbose(
expr.span,
"use `matches!(...)` instead",
format!("matches!({scrutinee_snip}, {pat_snip})"),
Applicability::MachineApplicable,
);
}
);
}
}
50 changes: 50 additions & 0 deletions tests/ui/string_lit_chars_any.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//@run-rustfix
//@aux-build:proc_macros.rs:proc-macro
#![allow(clippy::eq_op, clippy::needless_raw_string_hashes, clippy::no_effect, unused)]
#![warn(clippy::string_lit_chars_any)]

#[macro_use]
extern crate proc_macros;

struct NotStringLit;

impl NotStringLit {
fn chars(&self) -> impl Iterator<Item = char> {
"c".chars()
}
}

fn main() {
let c = 'c';
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
#[rustfmt::skip]
matches!(c, '\\' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
// Do not lint
NotStringLit.chars().any(|x| x == c);
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
let c = 'c';
x == c
});
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
1;
x == c
});
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == x);
"\\.+*?()|[]{}^$#&-~".chars().any(|_x| c == c);
matches!(
c,
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
);
external! {
let c = 'c';
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
}
with_span! {
span
let c = 'c';
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
}
}
50 changes: 50 additions & 0 deletions tests/ui/string_lit_chars_any.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//@run-rustfix
//@aux-build:proc_macros.rs:proc-macro
#![allow(clippy::eq_op, clippy::needless_raw_string_hashes, clippy::no_effect, unused)]
#![warn(clippy::string_lit_chars_any)]

#[macro_use]
extern crate proc_macros;

struct NotStringLit;

impl NotStringLit {
fn chars(&self) -> impl Iterator<Item = char> {
"c".chars()
}
}

fn main() {
let c = 'c';
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
r#"\.+*?()|[]{}^$#&-~"#.chars().any(|x| x == c);
"\\.+*?()|[]{}^$#&-~".chars().any(|x| c == x);
r#"\.+*?()|[]{}^$#&-~"#.chars().any(|x| c == x);
#[rustfmt::skip]
"\\.+*?()|[]{}^$#&-~".chars().any(|x| { x == c });
// Do not lint
NotStringLit.chars().any(|x| x == c);
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
let c = 'c';
x == c
});
"\\.+*?()|[]{}^$#&-~".chars().any(|x| {
1;
x == c
});
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == x);
"\\.+*?()|[]{}^$#&-~".chars().any(|_x| c == c);
matches!(
c,
'\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~'
);
external! {
let c = 'c';
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
}
with_span! {
span
let c = 'c';
"\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
}
}
58 changes: 58 additions & 0 deletions tests/ui/string_lit_chars_any.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
error: usage of `.chars().any(...)` to check if a char matches any from a string literal
--> $DIR/string_lit_chars_any.rs:19:5
|
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::string-lit-chars-any` implied by `-D warnings`
help: use `matches!(...)` instead
|
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: usage of `.chars().any(...)` to check if a char matches any from a string literal
--> $DIR/string_lit_chars_any.rs:20:5
|
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| x == c);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `matches!(...)` instead
|
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: usage of `.chars().any(...)` to check if a char matches any from a string literal
--> $DIR/string_lit_chars_any.rs:21:5
|
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| c == x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `matches!(...)` instead
|
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: usage of `.chars().any(...)` to check if a char matches any from a string literal
--> $DIR/string_lit_chars_any.rs:22:5
|
LL | r#"/.+*?()|[]{}^$#&-~"#.chars().any(|x| c == x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `matches!(...)` instead
|
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: usage of `.chars().any(...)` to check if a char matches any from a string literal
--> $DIR/string_lit_chars_any.rs:24:5
|
LL | "//.+*?()|[]{}^$#&-~".chars().any(|x| { x == c });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use `matches!(...)` instead
|
LL | matches!(c, '//' | '.' | '+' | '*' | '?' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 5 previous errors