mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-14 00:47:16 +00:00
Auto merge of #12849 - AurelienFT:manual-char-comparison-pattern, r=Alexendoo
Add lint to check manual pattern char comparison This PR adds a new lint asked in https://github.com/rust-lang/rust-clippy/issues/12490 This lint catches manual char comparison in pattern of string functions and propose to use `char` or array of `char` instead. As it's my first contribution i'm not feeling very safe about not matching too much or missing some cases. Thanks in advance for taking time to review and propose feedback ! changelog: new lint [`manual_pattern_char_comparison`]
This commit is contained in:
commit
740b72ef79
18 changed files with 451 additions and 163 deletions
|
@ -5532,6 +5532,7 @@ Released 2018-09-13
|
|||
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
|
||||
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
|
||||
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
|
||||
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
|
||||
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
|
||||
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
|
||||
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
|
||||
|
|
|
@ -448,7 +448,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO,
|
||||
crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO,
|
||||
crate::methods::SINGLE_CHAR_ADD_STR_INFO,
|
||||
crate::methods::SINGLE_CHAR_PATTERN_INFO,
|
||||
crate::methods::SKIP_WHILE_NEXT_INFO,
|
||||
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
|
||||
crate::methods::STRING_EXTEND_CHARS_INFO,
|
||||
|
@ -656,6 +655,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
|
|||
crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
|
||||
crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
|
||||
crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,
|
||||
crate::string_patterns::MANUAL_PATTERN_CHAR_COMPARISON_INFO,
|
||||
crate::string_patterns::SINGLE_CHAR_PATTERN_INFO,
|
||||
crate::strings::STRING_ADD_INFO,
|
||||
crate::strings::STRING_ADD_ASSIGN_INFO,
|
||||
crate::strings::STRING_FROM_UTF8_AS_BYTES_INFO,
|
||||
|
|
|
@ -103,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
|
|||
return;
|
||||
}
|
||||
|
||||
if is_whole && !sym_str.contains(|c| c == 'e' || c == 'E') {
|
||||
if is_whole && !sym_str.contains(['e', 'E']) {
|
||||
// Normalize the literal by stripping the fractional portion
|
||||
if sym_str.split('.').next().unwrap() != float_str {
|
||||
// If the type suffix is missing the suggestion would be
|
||||
|
|
|
@ -326,6 +326,7 @@ mod size_of_in_element_count;
|
|||
mod size_of_ref;
|
||||
mod slow_vector_initialization;
|
||||
mod std_instead_of_core;
|
||||
mod string_patterns;
|
||||
mod strings;
|
||||
mod strlen_on_c_strings;
|
||||
mod suspicious_operation_groupings;
|
||||
|
@ -1167,6 +1168,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
|
|||
..Default::default()
|
||||
})
|
||||
});
|
||||
store.register_late_pass(|_| Box::new(string_patterns::StringPatterns));
|
||||
// add lints here, do not remove this comment, it's used in `new_lint`
|
||||
}
|
||||
|
||||
|
|
|
@ -94,7 +94,6 @@ mod seek_from_current;
|
|||
mod seek_to_start_instead_of_rewind;
|
||||
mod single_char_add_str;
|
||||
mod single_char_insert_string;
|
||||
mod single_char_pattern;
|
||||
mod single_char_push_string;
|
||||
mod skip_while_next;
|
||||
mod stable_sort_primitive;
|
||||
|
@ -1141,38 +1140,6 @@ declare_clippy_lint! {
|
|||
"not returning type containing `Self` in a `new` method"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for string methods that receive a single-character
|
||||
/// `str` as an argument, e.g., `_.split("x")`.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// While this can make a perf difference on some systems,
|
||||
/// benchmarks have proven inconclusive. But at least using a
|
||||
/// char literal makes it clear that we are looking at a single
|
||||
/// character.
|
||||
///
|
||||
/// ### Known problems
|
||||
/// Does not catch multi-byte unicode characters. This is by
|
||||
/// design, on many machines, splitting by a non-ascii char is
|
||||
/// actually slower. Please do your own measurements instead of
|
||||
/// relying solely on the results of this lint.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust,ignore
|
||||
/// _.split("x");
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```rust,ignore
|
||||
/// _.split('x');
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub SINGLE_CHAR_PATTERN,
|
||||
pedantic,
|
||||
"using a single-character str where a char could be used, e.g., `_.split(\"x\")`"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for calling `.step_by(0)` on iterators which panics.
|
||||
|
@ -4169,7 +4136,6 @@ impl_lint_pass!(Methods => [
|
|||
FLAT_MAP_OPTION,
|
||||
INEFFICIENT_TO_STRING,
|
||||
NEW_RET_NO_SELF,
|
||||
SINGLE_CHAR_PATTERN,
|
||||
SINGLE_CHAR_ADD_STR,
|
||||
SEARCH_IS_SOME,
|
||||
FILTER_NEXT,
|
||||
|
@ -4324,7 +4290,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
|
|||
inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args);
|
||||
single_char_add_str::check(cx, expr, receiver, args);
|
||||
into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, receiver);
|
||||
single_char_pattern::check(cx, expr, method_call.ident.name, receiver, args);
|
||||
unnecessary_to_owned::check(cx, expr, method_call.ident.name, receiver, args, &self.msrv);
|
||||
},
|
||||
ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {
|
||||
|
|
|
@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
|
|||
lit.span,
|
||||
"calling `push` with '/' or '\\' (file system root) will overwrite the previous path definition",
|
||||
"try",
|
||||
format!("\"{}\"", pushed_path_lit.trim_start_matches(|c| c == '/' || c == '\\')),
|
||||
format!("\"{}\"", pushed_path_lit.trim_start_matches(['/', '\\'])),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
|
|
|
@ -1,6 +1,5 @@
|
|||
use super::utils::get_hint_if_single_char_arg;
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::snippet_with_applicability;
|
||||
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
|
||||
use rustc_ast::BorrowKind;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{self as hir, ExprKind};
|
||||
|
@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
|
|||
/// lint for length-1 `str`s as argument for `insert_str`
|
||||
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[1], &mut applicability, false) {
|
||||
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[1], &mut applicability, false) {
|
||||
let base_string_snippet =
|
||||
snippet_with_applicability(cx, receiver.span.source_callsite(), "_", &mut applicability);
|
||||
let pos_arg = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);
|
||||
|
|
|
@ -1,64 +0,0 @@
|
|||
use super::utils::get_hint_if_single_char_arg;
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir as hir;
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty;
|
||||
use rustc_span::symbol::Symbol;
|
||||
|
||||
use super::SINGLE_CHAR_PATTERN;
|
||||
|
||||
const PATTERN_METHODS: [(&str, usize); 22] = [
|
||||
("contains", 0),
|
||||
("starts_with", 0),
|
||||
("ends_with", 0),
|
||||
("find", 0),
|
||||
("rfind", 0),
|
||||
("split", 0),
|
||||
("split_inclusive", 0),
|
||||
("rsplit", 0),
|
||||
("split_terminator", 0),
|
||||
("rsplit_terminator", 0),
|
||||
("splitn", 1),
|
||||
("rsplitn", 1),
|
||||
("split_once", 0),
|
||||
("rsplit_once", 0),
|
||||
("matches", 0),
|
||||
("rmatches", 0),
|
||||
("match_indices", 0),
|
||||
("rmatch_indices", 0),
|
||||
("trim_start_matches", 0),
|
||||
("trim_end_matches", 0),
|
||||
("replace", 0),
|
||||
("replacen", 0),
|
||||
];
|
||||
|
||||
/// lint for length-1 `str`s for methods in `PATTERN_METHODS`
|
||||
pub(super) fn check(
|
||||
cx: &LateContext<'_>,
|
||||
_expr: &hir::Expr<'_>,
|
||||
method_name: Symbol,
|
||||
receiver: &hir::Expr<'_>,
|
||||
args: &[hir::Expr<'_>],
|
||||
) {
|
||||
for &(method, pos) in &PATTERN_METHODS {
|
||||
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty_adjusted(receiver).kind()
|
||||
&& ty.is_str()
|
||||
&& method_name.as_str() == method
|
||||
&& args.len() > pos
|
||||
&& let arg = &args[pos]
|
||||
&& let mut applicability = Applicability::MachineApplicable
|
||||
&& let Some(hint) = get_hint_if_single_char_arg(cx, arg, &mut applicability, true)
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
SINGLE_CHAR_PATTERN,
|
||||
arg.span,
|
||||
"single-character string constant used as pattern",
|
||||
"consider using a `char`",
|
||||
hint,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
|
@ -1,6 +1,5 @@
|
|||
use super::utils::get_hint_if_single_char_arg;
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::snippet_with_applicability;
|
||||
use clippy_utils::source::{snippet_with_applicability, str_literal_to_char_literal};
|
||||
use rustc_ast::BorrowKind;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{self as hir, ExprKind};
|
||||
|
@ -11,7 +10,7 @@ use super::SINGLE_CHAR_ADD_STR;
|
|||
/// lint for length-1 `str`s as argument for `push_str`
|
||||
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[0], &mut applicability, false) {
|
||||
if let Some(extension_string) = str_literal_to_char_literal(cx, &args[0], &mut applicability, false) {
|
||||
let base_string_snippet =
|
||||
snippet_with_applicability(cx, receiver.span.source_callsite(), "..", &mut applicability);
|
||||
let sugg = format!("{base_string_snippet}.push({extension_string})");
|
||||
|
|
|
@ -1,8 +1,5 @@
|
|||
use clippy_utils::source::snippet_with_applicability;
|
||||
use clippy_utils::ty::is_type_diagnostic_item;
|
||||
use clippy_utils::{get_parent_expr, path_to_local_id, usage};
|
||||
use rustc_ast::ast;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::intravisit::{walk_expr, Visitor};
|
||||
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, QPath, Stmt, StmtKind};
|
||||
use rustc_lint::LateContext;
|
||||
|
@ -49,48 +46,6 @@ pub(super) fn derefs_to_slice<'tcx>(
|
|||
}
|
||||
}
|
||||
|
||||
pub(super) fn get_hint_if_single_char_arg(
|
||||
cx: &LateContext<'_>,
|
||||
arg: &Expr<'_>,
|
||||
applicability: &mut Applicability,
|
||||
ascii_only: bool,
|
||||
) -> Option<String> {
|
||||
if let ExprKind::Lit(lit) = &arg.kind
|
||||
&& let ast::LitKind::Str(r, style) = lit.node
|
||||
&& let string = r.as_str()
|
||||
&& let len = if ascii_only {
|
||||
string.len()
|
||||
} else {
|
||||
string.chars().count()
|
||||
}
|
||||
&& len == 1
|
||||
{
|
||||
let snip = snippet_with_applicability(cx, arg.span, string, applicability);
|
||||
let ch = if let ast::StrStyle::Raw(nhash) = style {
|
||||
let nhash = nhash as usize;
|
||||
// for raw string: r##"a"##
|
||||
&snip[(nhash + 2)..(snip.len() - 1 - nhash)]
|
||||
} else {
|
||||
// for regular string: "a"
|
||||
&snip[1..(snip.len() - 1)]
|
||||
};
|
||||
|
||||
let hint = format!(
|
||||
"'{}'",
|
||||
match ch {
|
||||
"'" => "\\'",
|
||||
r"\" => "\\\\",
|
||||
"\\\"" => "\"", // no need to escape `"` in `'"'`
|
||||
_ => ch,
|
||||
}
|
||||
);
|
||||
|
||||
Some(hint)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// The core logic of `check_for_loop_iter` in `unnecessary_iter_cloned.rs`, this function wraps a
|
||||
/// use of `CloneOrCopyVisitor`.
|
||||
pub(super) fn clone_or_copy_needed<'tcx>(
|
||||
|
|
|
@ -6,7 +6,7 @@ use rustc_span::Span;
|
|||
use super::ZERO_PREFIXED_LITERAL;
|
||||
|
||||
pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str) {
|
||||
let trimmed_lit_snip = lit_snip.trim_start_matches(|c| c == '_' || c == '0');
|
||||
let trimmed_lit_snip = lit_snip.trim_start_matches(['_', '0']);
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
ZERO_PREFIXED_LITERAL,
|
||||
|
@ -20,7 +20,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str) {
|
|||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
// do not advise to use octal form if the literal cannot be expressed in base 8.
|
||||
if !lit_snip.contains(|c| c == '8' || c == '9') {
|
||||
if !lit_snip.contains(['8', '9']) {
|
||||
diag.span_suggestion(
|
||||
lit_span,
|
||||
"if you mean to use an octal constant, use `0o`",
|
||||
|
|
227
clippy_lints/src/string_patterns.rs
Normal file
227
clippy_lints/src/string_patterns.rs
Normal file
|
@ -0,0 +1,227 @@
|
|||
use std::ops::ControlFlow;
|
||||
|
||||
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
|
||||
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
|
||||
use clippy_utils::macros::matching_root_macro_call;
|
||||
use clippy_utils::path_to_local_id;
|
||||
use clippy_utils::source::{snippet, str_literal_to_char_literal};
|
||||
use clippy_utils::visitors::{for_each_expr, Descend};
|
||||
use itertools::Itertools;
|
||||
use rustc_ast::{BinOpKind, LitKind};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, PatKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::{sym, Span};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for manual `char` comparison in string patterns
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// This can be written more concisely using a `char` or an array of `char`.
|
||||
/// This is more readable and more optimized when comparing to only one `char`.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// "Hello World!".trim_end_matches(|c| c == '.' || c == ',' || c == '!' || c == '?');
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```no_run
|
||||
/// "Hello World!".trim_end_matches(['.', ',', '!', '?']);
|
||||
/// ```
|
||||
#[clippy::version = "1.80.0"]
|
||||
pub MANUAL_PATTERN_CHAR_COMPARISON,
|
||||
style,
|
||||
"manual char comparison in string patterns"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for string methods that receive a single-character
|
||||
/// `str` as an argument, e.g., `_.split("x")`.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// While this can make a perf difference on some systems,
|
||||
/// benchmarks have proven inconclusive. But at least using a
|
||||
/// char literal makes it clear that we are looking at a single
|
||||
/// character.
|
||||
///
|
||||
/// ### Known problems
|
||||
/// Does not catch multi-byte unicode characters. This is by
|
||||
/// design, on many machines, splitting by a non-ascii char is
|
||||
/// actually slower. Please do your own measurements instead of
|
||||
/// relying solely on the results of this lint.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust,ignore
|
||||
/// _.split("x");
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```rust,ignore
|
||||
/// _.split('x');
|
||||
/// ```
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub SINGLE_CHAR_PATTERN,
|
||||
pedantic,
|
||||
"using a single-character str where a char could be used, e.g., `_.split(\"x\")`"
|
||||
}
|
||||
|
||||
declare_lint_pass!(StringPatterns => [MANUAL_PATTERN_CHAR_COMPARISON, SINGLE_CHAR_PATTERN]);
|
||||
|
||||
const PATTERN_METHODS: [(&str, usize); 22] = [
|
||||
("contains", 0),
|
||||
("starts_with", 0),
|
||||
("ends_with", 0),
|
||||
("find", 0),
|
||||
("rfind", 0),
|
||||
("split", 0),
|
||||
("split_inclusive", 0),
|
||||
("rsplit", 0),
|
||||
("split_terminator", 0),
|
||||
("rsplit_terminator", 0),
|
||||
("splitn", 1),
|
||||
("rsplitn", 1),
|
||||
("split_once", 0),
|
||||
("rsplit_once", 0),
|
||||
("matches", 0),
|
||||
("rmatches", 0),
|
||||
("match_indices", 0),
|
||||
("rmatch_indices", 0),
|
||||
("trim_start_matches", 0),
|
||||
("trim_end_matches", 0),
|
||||
("replace", 0),
|
||||
("replacen", 0),
|
||||
];
|
||||
|
||||
fn check_single_char_pattern_lint(cx: &LateContext<'_>, arg: &Expr<'_>) {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
if let Some(hint) = str_literal_to_char_literal(cx, arg, &mut applicability, true) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
SINGLE_CHAR_PATTERN,
|
||||
arg.span,
|
||||
"single-character string constant used as pattern",
|
||||
"consider using a `char`",
|
||||
hint,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn get_char_span<'tcx>(cx: &'_ LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Span> {
|
||||
if cx.typeck_results().expr_ty_adjusted(expr).is_char()
|
||||
&& !expr.span.from_expansion()
|
||||
&& switch_to_eager_eval(cx, expr)
|
||||
{
|
||||
Some(expr.span)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn check_manual_pattern_char_comparison(cx: &LateContext<'_>, method_arg: &Expr<'_>) {
|
||||
if let ExprKind::Closure(closure) = method_arg.kind
|
||||
&& let body = cx.tcx.hir().body(closure.body)
|
||||
&& let Some(PatKind::Binding(_, binding, ..)) = body.params.first().map(|p| p.pat.kind)
|
||||
{
|
||||
let mut set_char_spans: Vec<Span> = Vec::new();
|
||||
|
||||
// We want to retrieve all the comparisons done.
|
||||
// They are ordered in a nested way and so we need to traverse the AST to collect them all.
|
||||
if for_each_expr(cx, body.value, |sub_expr| -> ControlFlow<(), Descend> {
|
||||
match sub_expr.kind {
|
||||
ExprKind::Binary(op, left, right) if op.node == BinOpKind::Eq => {
|
||||
if path_to_local_id(left, binding)
|
||||
&& let Some(span) = get_char_span(cx, right)
|
||||
{
|
||||
set_char_spans.push(span);
|
||||
ControlFlow::Continue(Descend::No)
|
||||
} else if path_to_local_id(right, binding)
|
||||
&& let Some(span) = get_char_span(cx, left)
|
||||
{
|
||||
set_char_spans.push(span);
|
||||
ControlFlow::Continue(Descend::No)
|
||||
} else {
|
||||
ControlFlow::Break(())
|
||||
}
|
||||
},
|
||||
ExprKind::Binary(op, _, _) if op.node == BinOpKind::Or => ControlFlow::Continue(Descend::Yes),
|
||||
ExprKind::Match(match_value, [arm, _], _) => {
|
||||
if matching_root_macro_call(cx, sub_expr.span, sym::matches_macro).is_none()
|
||||
|| arm.guard.is_some()
|
||||
|| !path_to_local_id(match_value, binding)
|
||||
{
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
if arm.pat.walk_short(|pat| match pat.kind {
|
||||
PatKind::Lit(expr) if let ExprKind::Lit(lit) = expr.kind => {
|
||||
if let LitKind::Char(_) = lit.node {
|
||||
set_char_spans.push(lit.span);
|
||||
}
|
||||
true
|
||||
},
|
||||
PatKind::Or(_) => true,
|
||||
_ => false,
|
||||
}) {
|
||||
ControlFlow::Continue(Descend::No)
|
||||
} else {
|
||||
ControlFlow::Break(())
|
||||
}
|
||||
},
|
||||
_ => ControlFlow::Break(()),
|
||||
}
|
||||
})
|
||||
.is_some()
|
||||
{
|
||||
return;
|
||||
}
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
MANUAL_PATTERN_CHAR_COMPARISON,
|
||||
method_arg.span,
|
||||
"this manual char comparison can be written more succinctly",
|
||||
|diag| {
|
||||
if let [set_char_span] = set_char_spans[..] {
|
||||
diag.span_suggestion(
|
||||
method_arg.span,
|
||||
"consider using a `char`",
|
||||
snippet(cx, set_char_span, "c"),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
} else {
|
||||
diag.span_suggestion(
|
||||
method_arg.span,
|
||||
"consider using an array of `char`",
|
||||
format!(
|
||||
"[{}]",
|
||||
set_char_spans.into_iter().map(|span| snippet(cx, span, "c")).join(", ")
|
||||
),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for StringPatterns {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
if !expr.span.from_expansion()
|
||||
&& let ExprKind::MethodCall(method, receiver, args, _) = expr.kind
|
||||
&& let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty_adjusted(receiver).kind()
|
||||
&& ty.is_str()
|
||||
&& let method_name = method.ident.name.as_str()
|
||||
&& let Some(&(_, pos)) = PATTERN_METHODS
|
||||
.iter()
|
||||
.find(|(array_method_name, _)| *array_method_name == method_name)
|
||||
&& let Some(arg) = args.get(pos)
|
||||
{
|
||||
check_single_char_pattern_lint(cx, arg);
|
||||
|
||||
check_manual_pattern_char_comparison(cx, arg);
|
||||
}
|
||||
}
|
||||
}
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
#![allow(clippy::module_name_repetitions)]
|
||||
|
||||
use rustc_ast::{LitKind, StrStyle};
|
||||
use rustc_data_structures::sync::Lrc;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource};
|
||||
|
@ -500,6 +501,50 @@ pub fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span {
|
|||
extended.with_lo(extended.lo() - BytePos(1))
|
||||
}
|
||||
|
||||
/// Converts `expr` to a `char` literal if it's a `str` literal containing a single
|
||||
/// character (or a single byte with `ascii_only`)
|
||||
pub fn str_literal_to_char_literal(
|
||||
cx: &LateContext<'_>,
|
||||
expr: &Expr<'_>,
|
||||
applicability: &mut Applicability,
|
||||
ascii_only: bool,
|
||||
) -> Option<String> {
|
||||
if let ExprKind::Lit(lit) = &expr.kind
|
||||
&& let LitKind::Str(r, style) = lit.node
|
||||
&& let string = r.as_str()
|
||||
&& let len = if ascii_only {
|
||||
string.len()
|
||||
} else {
|
||||
string.chars().count()
|
||||
}
|
||||
&& len == 1
|
||||
{
|
||||
let snip = snippet_with_applicability(cx, expr.span, string, applicability);
|
||||
let ch = if let StrStyle::Raw(nhash) = style {
|
||||
let nhash = nhash as usize;
|
||||
// for raw string: r##"a"##
|
||||
&snip[(nhash + 2)..(snip.len() - 1 - nhash)]
|
||||
} else {
|
||||
// for regular string: "a"
|
||||
&snip[1..(snip.len() - 1)]
|
||||
};
|
||||
|
||||
let hint = format!(
|
||||
"'{}'",
|
||||
match ch {
|
||||
"'" => "\\'",
|
||||
r"\" => "\\\\",
|
||||
"\\\"" => "\"", // no need to escape `"` in `'"'`
|
||||
_ => ch,
|
||||
}
|
||||
);
|
||||
|
||||
Some(hint)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use super::{reindent_multiline, without_block_comments};
|
||||
|
|
49
tests/ui/manual_pattern_char_comparison.fixed
Normal file
49
tests/ui/manual_pattern_char_comparison.fixed
Normal file
|
@ -0,0 +1,49 @@
|
|||
#![warn(clippy::manual_pattern_char_comparison)]
|
||||
|
||||
struct NotStr;
|
||||
|
||||
impl NotStr {
|
||||
fn find(&self, _: impl FnMut(char) -> bool) {}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let sentence = "Hello, world!";
|
||||
sentence.trim_end_matches(['.', ',', '!', '?']);
|
||||
sentence.split(['\n', 'X']);
|
||||
sentence.split(['\n', 'X']);
|
||||
sentence.splitn(3, 'X');
|
||||
sentence.splitn(3, |c: char| c.is_whitespace() || c == 'X');
|
||||
let char_compare = 'X';
|
||||
sentence.splitn(3, char_compare);
|
||||
sentence.split(['\n', 'X', 'Y']);
|
||||
sentence.splitn(3, 'X');
|
||||
sentence.splitn(3, ['X', 'W']);
|
||||
sentence.find('🎈');
|
||||
|
||||
let not_str = NotStr;
|
||||
not_str.find(|c: char| c == 'X');
|
||||
|
||||
"".find(|c| c == 'a' || c > 'z');
|
||||
|
||||
let x = true;
|
||||
"".find(|c| c == 'a' || x || c == 'b');
|
||||
|
||||
let d = 'd';
|
||||
"".find(|c| c == 'a' || d == 'b');
|
||||
|
||||
"".find(|c| match c {
|
||||
'a' | 'b' => true,
|
||||
_ => c.is_ascii(),
|
||||
});
|
||||
|
||||
"".find(|c| matches!(c, 'a' | 'b' if false));
|
||||
|
||||
"".find(|c| matches!(c, 'a' | '1'..'4'));
|
||||
"".find(|c| c == 'a' || matches!(c, '1'..'4'));
|
||||
macro_rules! m {
|
||||
($e:expr) => {
|
||||
$e == '?'
|
||||
};
|
||||
}
|
||||
"".find(|c| m!(c));
|
||||
}
|
49
tests/ui/manual_pattern_char_comparison.rs
Normal file
49
tests/ui/manual_pattern_char_comparison.rs
Normal file
|
@ -0,0 +1,49 @@
|
|||
#![warn(clippy::manual_pattern_char_comparison)]
|
||||
|
||||
struct NotStr;
|
||||
|
||||
impl NotStr {
|
||||
fn find(&self, _: impl FnMut(char) -> bool) {}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let sentence = "Hello, world!";
|
||||
sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?');
|
||||
sentence.split(|c: char| c == '\n' || c == 'X');
|
||||
sentence.split(|c| c == '\n' || c == 'X');
|
||||
sentence.splitn(3, |c: char| c == 'X');
|
||||
sentence.splitn(3, |c: char| c.is_whitespace() || c == 'X');
|
||||
let char_compare = 'X';
|
||||
sentence.splitn(3, |c: char| c == char_compare);
|
||||
sentence.split(|c: char| matches!(c, '\n' | 'X' | 'Y'));
|
||||
sentence.splitn(3, |c: char| matches!(c, 'X'));
|
||||
sentence.splitn(3, |c: char| matches!(c, 'X' | 'W'));
|
||||
sentence.find(|c| c == '🎈');
|
||||
|
||||
let not_str = NotStr;
|
||||
not_str.find(|c: char| c == 'X');
|
||||
|
||||
"".find(|c| c == 'a' || c > 'z');
|
||||
|
||||
let x = true;
|
||||
"".find(|c| c == 'a' || x || c == 'b');
|
||||
|
||||
let d = 'd';
|
||||
"".find(|c| c == 'a' || d == 'b');
|
||||
|
||||
"".find(|c| match c {
|
||||
'a' | 'b' => true,
|
||||
_ => c.is_ascii(),
|
||||
});
|
||||
|
||||
"".find(|c| matches!(c, 'a' | 'b' if false));
|
||||
|
||||
"".find(|c| matches!(c, 'a' | '1'..'4'));
|
||||
"".find(|c| c == 'a' || matches!(c, '1'..'4'));
|
||||
macro_rules! m {
|
||||
($e:expr) => {
|
||||
$e == '?'
|
||||
};
|
||||
}
|
||||
"".find(|c| m!(c));
|
||||
}
|
59
tests/ui/manual_pattern_char_comparison.stderr
Normal file
59
tests/ui/manual_pattern_char_comparison.stderr
Normal file
|
@ -0,0 +1,59 @@
|
|||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:11:31
|
||||
|
|
||||
LL | sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?');
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['.', ',', '!', '?']`
|
||||
|
|
||||
= note: `-D clippy::manual-pattern-char-comparison` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::manual_pattern_char_comparison)]`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:12:20
|
||||
|
|
||||
LL | sentence.split(|c: char| c == '\n' || c == 'X');
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['\n', 'X']`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:13:20
|
||||
|
|
||||
LL | sentence.split(|c| c == '\n' || c == 'X');
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['\n', 'X']`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:14:24
|
||||
|
|
||||
LL | sentence.splitn(3, |c: char| c == 'X');
|
||||
| ^^^^^^^^^^^^^^^^^^ help: consider using a `char`: `'X'`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:17:24
|
||||
|
|
||||
LL | sentence.splitn(3, |c: char| c == char_compare);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a `char`: `char_compare`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:18:20
|
||||
|
|
||||
LL | sentence.split(|c: char| matches!(c, '\n' | 'X' | 'Y'));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['\n', 'X', 'Y']`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:19:24
|
||||
|
|
||||
LL | sentence.splitn(3, |c: char| matches!(c, 'X'));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a `char`: `'X'`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:20:24
|
||||
|
|
||||
LL | sentence.splitn(3, |c: char| matches!(c, 'X' | 'W'));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['X', 'W']`
|
||||
|
||||
error: this manual char comparison can be written more succinctly
|
||||
--> tests/ui/manual_pattern_char_comparison.rs:21:19
|
||||
|
|
||||
LL | sentence.find(|c| c == '🎈');
|
||||
| ^^^^^^^^^^^^^ help: consider using a `char`: `'🎈'`
|
||||
|
||||
error: aborting due to 9 previous errors
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
//@aux-build:option_helpers.rs
|
||||
#![warn(clippy::search_is_some)]
|
||||
#![allow(clippy::manual_pattern_char_comparison)]
|
||||
#![allow(clippy::useless_vec)]
|
||||
#![allow(dead_code)]
|
||||
extern crate option_helpers;
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
error: called `is_some()` after searching an `Iterator` with `find`
|
||||
--> tests/ui/search_is_some.rs:15:13
|
||||
--> tests/ui/search_is_some.rs:16:13
|
||||
|
|
||||
LL | let _ = v.iter().find(|&x| {
|
||||
| _____________^
|
||||
|
@ -13,7 +13,7 @@ LL | | ).is_some();
|
|||
= help: to override `-D warnings` add `#[allow(clippy::search_is_some)]`
|
||||
|
||||
error: called `is_some()` after searching an `Iterator` with `position`
|
||||
--> tests/ui/search_is_some.rs:21:13
|
||||
--> tests/ui/search_is_some.rs:22:13
|
||||
|
|
||||
LL | let _ = v.iter().position(|&x| {
|
||||
| _____________^
|
||||
|
@ -25,7 +25,7 @@ LL | | ).is_some();
|
|||
= help: this is more succinctly expressed by calling `any()`
|
||||
|
||||
error: called `is_some()` after searching an `Iterator` with `rposition`
|
||||
--> tests/ui/search_is_some.rs:27:13
|
||||
--> tests/ui/search_is_some.rs:28:13
|
||||
|
|
||||
LL | let _ = v.iter().rposition(|&x| {
|
||||
| _____________^
|
||||
|
@ -37,13 +37,13 @@ LL | | ).is_some();
|
|||
= help: this is more succinctly expressed by calling `any()`
|
||||
|
||||
error: called `is_some()` after searching an `Iterator` with `find`
|
||||
--> tests/ui/search_is_some.rs:42:20
|
||||
--> tests/ui/search_is_some.rs:43:20
|
||||
|
|
||||
LL | let _ = (0..1).find(some_closure).is_some();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(some_closure)`
|
||||
|
||||
error: called `is_none()` after searching an `Iterator` with `find`
|
||||
--> tests/ui/search_is_some.rs:52:13
|
||||
--> tests/ui/search_is_some.rs:53:13
|
||||
|
|
||||
LL | let _ = v.iter().find(|&x| {
|
||||
| _____________^
|
||||
|
@ -55,7 +55,7 @@ LL | | ).is_none();
|
|||
= help: this is more succinctly expressed by calling `any()` with negation
|
||||
|
||||
error: called `is_none()` after searching an `Iterator` with `position`
|
||||
--> tests/ui/search_is_some.rs:58:13
|
||||
--> tests/ui/search_is_some.rs:59:13
|
||||
|
|
||||
LL | let _ = v.iter().position(|&x| {
|
||||
| _____________^
|
||||
|
@ -67,7 +67,7 @@ LL | | ).is_none();
|
|||
= help: this is more succinctly expressed by calling `any()` with negation
|
||||
|
||||
error: called `is_none()` after searching an `Iterator` with `rposition`
|
||||
--> tests/ui/search_is_some.rs:64:13
|
||||
--> tests/ui/search_is_some.rs:65:13
|
||||
|
|
||||
LL | let _ = v.iter().rposition(|&x| {
|
||||
| _____________^
|
||||
|
@ -79,7 +79,7 @@ LL | | ).is_none();
|
|||
= help: this is more succinctly expressed by calling `any()` with negation
|
||||
|
||||
error: called `is_none()` after searching an `Iterator` with `find`
|
||||
--> tests/ui/search_is_some.rs:79:13
|
||||
--> tests/ui/search_is_some.rs:80:13
|
||||
|
|
||||
LL | let _ = (0..1).find(some_closure).is_none();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!(0..1).any(some_closure)`
|
||||
|
|
Loading…
Reference in a new issue