Auto merge of rust-lang#11852 - rust-lang:single-char-pattern-ascii-only, r=xFrednet

reduce `single_char_pattern` to only lint on ascii chars

This should mostly fix the `single_char_pattern` lint, because with a single byte, the optimizer will usually see through the char-to-string-expansion and single loop iteration. This fixes rust-lang#11675 and rust-lang#8111.

Update: As per the meeting on November 28th, 2023, we voted to also downgrade the lint to pedantic.

---

changelog: downgrade [`single_char_pattern`] to `pedantic`

Author: bors

clippy_lints/src/methods/mod.rs

@@ -1145,11 +1145,16 @@ declare_clippy_lint! {
     /// `str` as an argument, e.g., `_.split("x")`.
     ///
     /// ### Why is this bad?
-    /// Performing these methods using a `char` is faster than
-    /// using a `str`.
+    /// 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.
+    /// 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
@@ -1162,7 +1167,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "pre 1.29.0"]
     pub SINGLE_CHAR_PATTERN,
-    perf,
+    pedantic,
     "using a single-character str where a char could be used, e.g., `_.split(\"x\")`"
 }
 

clippy_lints/src/methods/single_char_insert_string.rs

@@ -10,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) {
+    if let Some(extension_string) = get_hint_if_single_char_arg(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_pattern.rs

@@ -8,7 +8,7 @@ use rustc_span::symbol::Symbol;
 
 use super::SINGLE_CHAR_PATTERN;
 
-const PATTERN_METHODS: [(&str, usize); 24] = [
+const PATTERN_METHODS: [(&str, usize); 22] = [
     ("contains", 0),
     ("starts_with", 0),
     ("ends_with", 0),
@@ -27,8 +27,6 @@ const PATTERN_METHODS: [(&str, usize); 24] = [
     ("rmatches", 0),
     ("match_indices", 0),
     ("rmatch_indices", 0),
-    ("strip_prefix", 0),
-    ("strip_suffix", 0),
     ("trim_start_matches", 0),
     ("trim_end_matches", 0),
     ("replace", 0),
@@ -50,7 +48,7 @@ pub(super) fn check(
             && 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)
+            && let Some(hint) = get_hint_if_single_char_arg(cx, arg, &mut applicability, true)
         {
             span_lint_and_sugg(
                 cx,

clippy_lints/src/methods/single_char_push_string.rs

@@ -10,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) {
+    if let Some(extension_string) = get_hint_if_single_char_arg(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

@@ -52,11 +52,17 @@ 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()
-        && string.chars().count() == 1
+        && 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 {

tests/ui/single_char_pattern.fixed

@@ -1,5 +1,5 @@
 #![allow(clippy::needless_raw_strings, clippy::needless_raw_string_hashes, unused_must_use)]
-
+#![warn(clippy::single_char_pattern)]
 use std::collections::HashSet;
 
 fn main() {
@@ -10,9 +10,9 @@ fn main() {
 
     let y = "x";
     x.split(y);
-    x.split('ß');
-    x.split('ℝ');
-    x.split('💣');
+    x.split("ß");
+    x.split("ℝ");
+    x.split("💣");
     // Can't use this lint for unicode code points which don't fit in a char
     x.split("❤️");
     x.split_inclusive('x');
@@ -34,8 +34,6 @@ fn main() {
     x.rmatch_indices('x');
     x.trim_start_matches('x');
     x.trim_end_matches('x');
-    x.strip_prefix('x');
-    x.strip_suffix('x');
     x.replace('x', "y");
     x.replacen('x', "y", 3);
     // Make sure we escape characters correctly.
@@ -64,4 +62,8 @@ fn main() {
     // Must escape backslash in raw strings when converting to char #8060
     x.split('\\');
     x.split('\\');
+
+    // should not warn, the char versions are actually slower in some cases
+    x.strip_prefix("x");
+    x.strip_suffix("x");
 }

tests/ui/single_char_pattern.rs

@@ -1,5 +1,5 @@
 #![allow(clippy::needless_raw_strings, clippy::needless_raw_string_hashes, unused_must_use)]
-
+#![warn(clippy::single_char_pattern)]
 use std::collections::HashSet;
 
 fn main() {
@@ -34,8 +34,6 @@ fn main() {
     x.rmatch_indices("x");
     x.trim_start_matches("x");
     x.trim_end_matches("x");
-    x.strip_prefix("x");
-    x.strip_suffix("x");
     x.replace("x", "y");
     x.replacen("x", "y", 3);
     // Make sure we escape characters correctly.
@@ -64,4 +62,8 @@ fn main() {
     // Must escape backslash in raw strings when converting to char #8060
     x.split(r#"\"#);
     x.split(r"\");
+
+    // should not warn, the char versions are actually slower in some cases
+    x.strip_prefix("x");
+    x.strip_suffix("x");
 }

tests/ui/single_char_pattern.stderr

@@ -7,24 +7,6 @@ LL |     x.split("x");
    = note: `-D clippy::single-char-pattern` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::single_char_pattern)]`
 
-error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:13:13
-   |
-LL |     x.split("ß");
-   |             ^^^ help: consider using a `char`: `'ß'`
-
-error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:14:13
-   |
-LL |     x.split("ℝ");
-   |             ^^^ help: consider using a `char`: `'ℝ'`
-
-error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:15:13
-   |
-LL |     x.split("💣");
-   |             ^^^^ help: consider using a `char`: `'💣'`
-
 error: single-character string constant used as pattern
   --> tests/ui/single_char_pattern.rs:18:23
    |
@@ -140,106 +122,94 @@ LL |     x.trim_end_matches("x");
    |                        ^^^ help: consider using a `char`: `'x'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:37:20
-   |
-LL |     x.strip_prefix("x");
-   |                    ^^^ help: consider using a `char`: `'x'`
-
-error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:38:20
-   |
-LL |     x.strip_suffix("x");
-   |                    ^^^ help: consider using a `char`: `'x'`
-
-error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:39:15
+  --> tests/ui/single_char_pattern.rs:37:15
    |
 LL |     x.replace("x", "y");
    |               ^^^ help: consider using a `char`: `'x'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:40:16
+  --> tests/ui/single_char_pattern.rs:38:16
    |
 LL |     x.replacen("x", "y", 3);
    |                ^^^ help: consider using a `char`: `'x'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:42:13
+  --> tests/ui/single_char_pattern.rs:40:13
    |
 LL |     x.split("\n");
    |             ^^^^ help: consider using a `char`: `'\n'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:43:13
+  --> tests/ui/single_char_pattern.rs:41:13
    |
 LL |     x.split("'");
    |             ^^^ help: consider using a `char`: `'\''`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:44:13
+  --> tests/ui/single_char_pattern.rs:42:13
    |
 LL |     x.split("\'");
    |             ^^^^ help: consider using a `char`: `'\''`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:46:13
+  --> tests/ui/single_char_pattern.rs:44:13
    |
 LL |     x.split("\"");
    |             ^^^^ help: consider using a `char`: `'"'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:51:31
+  --> tests/ui/single_char_pattern.rs:49:31
    |
 LL |     x.replace(';', ",").split(","); // issue #2978
    |                               ^^^ help: consider using a `char`: `','`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:52:19
+  --> tests/ui/single_char_pattern.rs:50:19
    |
 LL |     x.starts_with("\x03"); // issue #2996
    |                   ^^^^^^ help: consider using a `char`: `'\x03'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:59:13
+  --> tests/ui/single_char_pattern.rs:57:13
    |
 LL |     x.split(r"a");
    |             ^^^^ help: consider using a `char`: `'a'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:60:13
+  --> tests/ui/single_char_pattern.rs:58:13
    |
 LL |     x.split(r#"a"#);
    |             ^^^^^^ help: consider using a `char`: `'a'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:61:13
+  --> tests/ui/single_char_pattern.rs:59:13
    |
 LL |     x.split(r###"a"###);
    |             ^^^^^^^^^^ help: consider using a `char`: `'a'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:62:13
+  --> tests/ui/single_char_pattern.rs:60:13
    |
 LL |     x.split(r###"'"###);
    |             ^^^^^^^^^^ help: consider using a `char`: `'\''`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:63:13
+  --> tests/ui/single_char_pattern.rs:61:13
    |
 LL |     x.split(r###"#"###);
    |             ^^^^^^^^^^ help: consider using a `char`: `'#'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:65:13
+  --> tests/ui/single_char_pattern.rs:63:13
    |
 LL |     x.split(r#"\"#);
    |             ^^^^^^ help: consider using a `char`: `'\\'`
 
 error: single-character string constant used as pattern
-  --> tests/ui/single_char_pattern.rs:66:13
+  --> tests/ui/single_char_pattern.rs:64:13
    |
 LL |     x.split(r"\");
    |             ^^^^ help: consider using a `char`: `'\\'`
 
-error: aborting due to 40 previous errors
+error: aborting due to 35 previous errors