Auto merge of #12849 - AurelienFT:manual-char-comparison-pattern, r=A…

…lexendoo

Add lint to check manual pattern char comparison

This PR adds a new lint asked in #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.md

@@ -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

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/float_literal.rs

@@ -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

clippy_lints/src/lib.rs

@@ -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`
 }
 

clippy_lints/src/methods/mod.rs

@@ -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 => {

clippy_lints/src/methods/path_buf_push_overwrite.rs

@@ -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,
         );
     }

clippy_lints/src/methods/single_char_insert_string.rs

@@ -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);

clippy_lints/src/methods/single_char_push_string.rs

@@ -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})");

clippy_lints/src/methods/utils.rs

@@ -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>(

clippy_lints/src/misc_early/zero_prefixed_literal.rs

@@ -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`",