about summary refs log tree commit diff
path: root/clippy_lints/src/string_patterns.rs
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2024-06-13 12:18:48 +0200
committerPhilipp Krones <hello@philkrones.com>2024-06-13 12:24:08 +0200
commitcc63143bbf5ae28daae127902090492560313ca1 (patch)
tree3cf8901da2767e0e330b32ef8a1f8a3bf75adc0c /clippy_lints/src/string_patterns.rs
parent35f54fd439432f56c749fbb34f1326e34ed1fc42 (diff)
parentaaade2d12d3cf78fd9eb24bf7973e86433d6373d (diff)
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'clippy_lints/src/string_patterns.rs')
-rw-r--r--clippy_lints/src/string_patterns.rs227
1 files changed, 227 insertions, 0 deletions
diff --git a/clippy_lints/src/string_patterns.rs b/clippy_lints/src/string_patterns.rs
new file mode 100644
index 00000000000..64b5b8f9f27
--- /dev/null
+++ b/clippy_lints/src/string_patterns.rs
@@ -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);
+        }
+    }
+}