fix: match_single_binding suggests wrongly inside binary expr

Author: profetia

clippy_lints/src/matches/match_single_binding.rs

@@ -2,10 +2,8 @@ use std::ops::ControlFlow;
 
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::macros::HirNode;
-use clippy_utils::source::{
-    RelativeIndent, indent_of, reindent_multiline_relative, snippet, snippet_block_with_context, snippet_with_context,
-};
-use clippy_utils::{is_refutable, peel_blocks};
+use clippy_utils::source::{indent_of, reindent_multiline, snippet, snippet_block_with_context, snippet_with_context};
+use clippy_utils::{is_expr_identity_of_pat, is_refutable, peel_blocks};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
@@ -86,6 +84,18 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
                         snippet_with_context(cx, pat_span, ctxt, "..", &mut app).0
                     ),
                 ),
+                None if is_expr_identity_of_pat(cx, arms[0].pat, ex, false) => {
+                    span_lint_and_sugg(
+                        cx,
+                        MATCH_SINGLE_BINDING,
+                        expr.span,
+                        "this match could be replaced by its body itself",
+                        "consider using the match body instead",
+                        snippet_body,
+                        Applicability::MachineApplicable,
+                    );
+                    return;
+                },
                 None => {
                     let sugg = sugg_with_curlies(
                         cx,
@@ -302,7 +312,7 @@ fn expr_in_nested_block(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
 fn expr_must_have_curlies(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
     let parent = cx.tcx.parent_hir_node(match_expr.hir_id);
     if let Node::Expr(Expr {
-        kind: ExprKind::Closure { .. },
+        kind: ExprKind::Closure(..) | ExprKind::Binary(..),
         ..
     })
     | Node::AnonConst(..) = parent
@@ -319,15 +329,23 @@ fn expr_must_have_curlies(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
     false
 }
 
+fn indent_of_nth_line(snippet: &str, nth: usize) -> Option<usize> {
+    snippet
+        .lines()
+        .nth(nth)
+        .and_then(|s| s.find(|c: char| !c.is_whitespace()))
+}
+
 fn reindent_snippet_if_in_block(snippet_body: &str, has_assignment: bool) -> String {
     if has_assignment || !snippet_body.starts_with('{') {
-        return reindent_multiline_relative(snippet_body, true, RelativeIndent::Add(0));
+        return reindent_multiline(snippet_body, true, indent_of_nth_line(snippet_body, 1));
     }
 
-    reindent_multiline_relative(
-        snippet_body.trim_start_matches('{').trim_end_matches('}').trim(),
+    let snippet_body = snippet_body.trim_start_matches('{').trim_end_matches('}').trim();
+    reindent_multiline(
+        snippet_body,
         false,
-        RelativeIndent::Sub(4),
+        indent_of_nth_line(snippet_body, 0).map(|indent| indent.saturating_sub(4)),
     )
 }
 

clippy_utils/src/lib.rs

@@ -1900,39 +1900,6 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 ///
 /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
 fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
-    fn check_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
-        if cx
-            .typeck_results()
-            .pat_binding_modes()
-            .get(pat.hir_id)
-            .is_some_and(|mode| matches!(mode.0, ByRef::Yes(_)))
-        {
-            // If the parameter is `(x, y)` of type `&(T, T)`, or `[x, y]` of type `&[T; 2]`, then
-            // due to match ergonomics, the inner patterns become references. Don't consider this
-            // the identity function as that changes types.
-            return false;
-        }
-
-        match (pat.kind, expr.kind) {
-            (PatKind::Binding(_, id, _, _), _) => {
-                path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()
-            },
-            (PatKind::Tuple(pats, dotdot), ExprKind::Tup(tup))
-                if dotdot.as_opt_usize().is_none() && pats.len() == tup.len() =>
-            {
-                pats.iter().zip(tup).all(|(pat, expr)| check_pat(cx, pat, expr))
-            },
-            (PatKind::Slice(before, slice, after), ExprKind::Array(arr))
-                if slice.is_none() && before.len() + after.len() == arr.len() =>
-            {
-                (before.iter().chain(after))
-                    .zip(arr)
-                    .all(|(pat, expr)| check_pat(cx, pat, expr))
-            },
-            _ => false,
-        }
-    }
-
     let [param] = func.params else {
         return false;
     };
@@ -1965,11 +1932,56 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
                     return false;
                 }
             },
-            _ => return check_pat(cx, param.pat, expr),
+            _ => return is_expr_identity_of_pat(cx, param.pat, expr, true),
         }
     }
 }
 
+/// Checks if the given expression is an identity representation of the given pattern:
+/// * `x` is the identity representation of `x`
+/// * `(x, y)` is the identity representation of `(x, y)`
+/// * `[x, y]` is the identity representation of `[x, y]`
+///
+/// Note that `by_hir` is used to determine bindings are checked by their `HirId` or by their name.
+/// This can be useful when checking patterns in `let` bindings or `match` arms.
+pub fn is_expr_identity_of_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>, by_hir: bool) -> bool {
+    if cx
+        .typeck_results()
+        .pat_binding_modes()
+        .get(pat.hir_id)
+        .is_some_and(|mode| matches!(mode.0, ByRef::Yes(_)))
+    {
+        // If the parameter is `(x, y)` of type `&(T, T)`, or `[x, y]` of type `&[T; 2]`, then
+        // due to match ergonomics, the inner patterns become references. Don't consider this
+        // the identity function as that changes types.
+        return false;
+    }
+
+    match (pat.kind, expr.kind) {
+        (PatKind::Binding(_, id, _, _), _) if by_hir => {
+            path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()
+        },
+        (PatKind::Binding(_, _, ident, _), ExprKind::Path(QPath::Resolved(_, path))) => {
+            matches!(path.segments, [ segment] if segment.ident.name == ident.name)
+        },
+        (PatKind::Tuple(pats, dotdot), ExprKind::Tup(tup))
+            if dotdot.as_opt_usize().is_none() && pats.len() == tup.len() =>
+        {
+            pats.iter()
+                .zip(tup)
+                .all(|(pat, expr)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
+        },
+        (PatKind::Slice(before, slice, after), ExprKind::Array(arr))
+            if slice.is_none() && before.len() + after.len() == arr.len() =>
+        {
+            (before.iter().chain(after))
+                .zip(arr)
+                .all(|(pat, expr)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
+        },
+        _ => false,
+    }
+}
+
 /// This is the same as [`is_expr_identity_function`], but does not consider closures
 /// with type annotations for its bindings (or similar) as identity functions:
 /// * `|x: u8| x`

clippy_utils/src/source.rs

@@ -18,7 +18,7 @@ use rustc_span::{
 };
 use std::borrow::Cow;
 use std::fmt;
-use std::ops::{Add, Deref, Index, Range};
+use std::ops::{Deref, Index, Range};
 
 pub trait HasSession {
     fn sess(&self) -> &Session;
@@ -476,38 +476,6 @@ pub fn position_before_rarrow(s: &str) -> Option<usize> {
     })
 }
 
-pub enum RelativeIndent {
-    Add(usize),
-    Sub(usize),
-}
-
-impl Add<RelativeIndent> for usize {
-    type Output = usize;
-
-    fn add(self, rhs: RelativeIndent) -> Self::Output {
-        match rhs {
-            RelativeIndent::Add(n) => self + n,
-            RelativeIndent::Sub(n) => self.saturating_sub(n),
-        }
-    }
-}
-
-/// Reindents a multiline string with possibility of ignoring the first line and relative
-/// indentation.
-pub fn reindent_multiline_relative(s: &str, ignore_first: bool, relative_indent: RelativeIndent) -> String {
-    fn indent_of_string(s: &str) -> usize {
-        s.find(|c: char| !c.is_whitespace()).unwrap_or(0)
-    }
-
-    let mut indent = 0;
-    if let Some(line) = s.lines().nth(usize::from(ignore_first)) {
-        let line_indent = indent_of_string(line);
-        indent = line_indent + relative_indent;
-    }
-
-    reindent_multiline(s, ignore_first, Some(indent))
-}
-
 /// Reindent a multiline string with possibility of ignoring the first line.
 pub fn reindent_multiline(s: &str, ignore_first: bool, indent: Option<usize>) -> String {
     let s_space = reindent_multiline_inner(s, ignore_first, indent, ' ');

tests/ui/match_single_binding.fixed

@@ -257,3 +257,12 @@ mod issue15018 {
         }
     }
 }
+
+#[allow(clippy::short_circuit_statement)]
+fn issue15269(a: usize, b: usize, c: usize) -> bool {
+    a < b
+        && b < c;
+
+    a < b
+        && b < c
+}

tests/ui/match_single_binding.rs

@@ -328,3 +328,18 @@ mod issue15018 {
         }
     }
 }
+
+#[allow(clippy::short_circuit_statement)]
+fn issue15269(a: usize, b: usize, c: usize) -> bool {
+    a < b
+        && match b {
+            //~^ match_single_binding
+            b => b < c,
+        };
+
+    a < b
+        && match (a, b) {
+            //~^ match_single_binding
+            (a, b) => b < c,
+        }
+}

tests/ui/match_single_binding.stderr

@@ -505,5 +505,25 @@ LL ~             let (x, y, z) = (a, b, c);
 LL +             println!("{} {} {}", x, y, z);
    |
 
-error: aborting due to 35 previous errors
+error: this match could be replaced by its body itself
+  --> tests/ui/match_single_binding.rs:335:12
+   |
+LL |           && match b {
+   |  ____________^
+LL | |
+LL | |             b => b < c,
+LL | |         };
+   | |_________^ help: consider using the match body instead: `b < c`
+
+error: this match could be replaced by its body itself
+  --> tests/ui/match_single_binding.rs:341:12
+   |
+LL |           && match (a, b) {
+   |  ____________^
+LL | |
+LL | |             (a, b) => b < c,
+LL | |         }
+   | |_________^ help: consider using the match body instead: `b < c`
+
+error: aborting due to 37 previous errors