add tests, lint on while let true and matches!(.., true)

Author: y21

clippy_lints/src/loops/while_let_on_iterator.rs

@@ -14,7 +14,7 @@ use rustc_span::symbol::sym;
 use rustc_span::Symbol;
 
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-    if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr)
+    if let Some(higher::WhileLet { if_then, let_pat, let_expr, .. }) = higher::WhileLet::hir(expr)
         // check for `Some(..)` pattern
         && let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind
         && is_res_lang_ctor(cx, cx.qpath_res(pat_path, let_pat.hir_id), LangItem::OptionSome)

clippy_lints/src/matches/mod.rs

@@ -469,15 +469,15 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// ### What it does
     /// Lint for redundant pattern matching over `Result`, `Option`,
-    /// `std::task::Poll` or `std::net::IpAddr`
+    /// `std::task::Poll`, `std::net::IpAddr` or `bool`s
     ///
     /// ### Why is this bad?
     /// It's more concise and clear to just use the proper
-    /// utility function
+    /// utility function or using the condition directly
     ///
     /// ### Known problems
-    /// This will change the drop order for the matched type. Both `if let` and
-    /// `while let` will drop the value at the end of the block, both `if` and `while` will drop the
+    /// For suggestions involving bindings in patterns, this will change the drop order for the matched type.
+    /// Both `if let` and `while let` will drop the value at the end of the block, both `if` and `while` will drop the
     /// value before entering the block. For most types this change will not matter, but for a few
     /// types this will not be an acceptable change (e.g. locks). See the
     /// [reference](https://doc.rust-lang.org/reference/destructors.html#drop-scopes) for more about
@@ -499,6 +499,10 @@ declare_clippy_lint! {
     ///     Ok(_) => true,
     ///     Err(_) => false,
     /// };
+    ///
+    /// let cond = true;
+    /// if let true = cond {}
+    /// matches!(cond, true);
     /// ```
     ///
     /// The more idiomatic use would be:
@@ -515,6 +519,10 @@ declare_clippy_lint! {
     /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
     /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
     /// Ok::<i32, i32>(42).is_ok();
+    ///
+    /// let cond = true;
+    /// if cond {}
+    /// cond;
     /// ```
     #[clippy::version = "1.31.0"]
     pub REDUNDANT_PATTERN_MATCHING,
@@ -1019,8 +1027,11 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
         let from_expansion = expr.span.from_expansion();
 
         if let ExprKind::Match(ex, arms, source) = expr.kind {
-            if is_direct_expn_of(expr.span, "matches").is_some() {
+            if is_direct_expn_of(expr.span, "matches").is_some()
+                && let [arm, _] = arms
+            {
                 redundant_pattern_match::check_match(cx, expr, ex, arms);
+                redundant_pattern_match::check_matches_true(cx, expr, arm, ex);
             }
 
             if source == MatchSource::Normal && !is_span_match(cx, expr.span) {

clippy_lints/src/matches/redundant_pattern_match.rs

@@ -1,7 +1,7 @@
 use super::REDUNDANT_PATTERN_MATCHING;
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, walk_span_to_context};
-use clippy_utils::sugg::Sugg;
+use clippy_utils::sugg::{make_unop, Sugg};
 use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
 use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr};
 use clippy_utils::{higher, is_expn_of, is_trait_method};
@@ -17,8 +17,15 @@ use std::fmt::Write;
 use std::ops::ControlFlow;
 
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-    if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
+    if let Some(higher::WhileLet {
+        let_pat,
+        let_expr,
+        let_span,
+        ..
+    }) = higher::WhileLet::hir(expr)
+    {
         find_method_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false);
+        find_if_let_true(cx, let_pat, let_expr, let_span);
     }
 }
 
@@ -34,26 +41,65 @@ pub(super) fn check_if_let<'tcx>(
     find_method_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else);
 }
 
+/// Looks for:
+/// * `matches!(expr, true)`
+pub fn check_matches_true<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    arm: &'tcx Arm<'_>,
+    scrutinee: &'tcx Expr<'_>,
+) {
+    find_match_true(
+        cx,
+        arm.pat,
+        scrutinee,
+        expr.span.source_callsite(),
+        "using `matches!` to pattern match a bool",
+    );
+}
+
+/// Looks for any of:
+/// * `if let true = ...`
+/// * `if let false = ...`
+/// * `while let true = ...`
 fn find_if_let_true<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, scrutinee: &'tcx Expr<'_>, let_span: Span) {
+    find_match_true(cx, pat, scrutinee, let_span, "using `if let` to pattern match a bool");
+}
+
+/// Common logic between `find_if_let_true` and `check_matches_true`
+fn find_match_true<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    scrutinee: &'tcx Expr<'_>,
+    span: Span,
+    message: &str,
+) {
     if let PatKind::Lit(lit) = pat.kind
         && let ExprKind::Lit(lit) = lit.kind
-        && let LitKind::Bool(is_true) = lit.node
+        && let LitKind::Bool(pat_is_true) = lit.node
     {
-        let mut snip = snippet(cx, scrutinee.span, "..").into_owned();
+        let mut applicability = Applicability::MachineApplicable;
+
+        let mut sugg = Sugg::hir_with_context(
+            cx,
+            scrutinee,
+            scrutinee.span.source_callsite().ctxt(),
+            "..",
+            &mut applicability,
+        );
 
-        if !is_true {
-            // Invert condition for `if let false = ...`
-            snip.insert(0, '!');
+        if !pat_is_true {
+            sugg = make_unop("!", sugg);
         }
 
         span_lint_and_sugg(
             cx,
             REDUNDANT_PATTERN_MATCHING,
-            let_span,
-            "using `if let` to pattern match a boolean",
-            "consider using a regular `if` expression",
-            snip,
-            Applicability::MachineApplicable,
+            span,
+            message,
+            "consider using the condition directly",
+            sugg.to_string(),
+            applicability,
         );
     }
 }

clippy_lints/src/utils/author.rs

@@ -350,6 +350,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
             let_pat,
             let_expr,
             if_then,
+            ..
         }) = higher::WhileLet::hir(expr.value)
         {
             bind!(self, let_pat, let_expr, if_then);

clippy_utils/src/higher.rs

@@ -362,6 +362,9 @@ pub struct WhileLet<'hir> {
     pub let_expr: &'hir Expr<'hir>,
     /// `while let` loop body
     pub if_then: &'hir Expr<'hir>,
+    /// `while let PAT = EXPR`
+    ///        ^^^^^^^^^^^^^^
+    pub let_span: Span,
 }
 
 impl<'hir> WhileLet<'hir> {
@@ -376,9 +379,10 @@ impl<'hir> WhileLet<'hir> {
                             ExprKind::If(
                                 Expr {
                                     kind:
-                                        ExprKind::Let(hir::Let {
+                                        ExprKind::Let(&hir::Let {
                                             pat: let_pat,
                                             init: let_expr,
+                                            span: let_span,
                                             ..
                                         }),
                                     ..
@@ -399,6 +403,7 @@ impl<'hir> WhileLet<'hir> {
                 let_pat,
                 let_expr,
                 if_then,
+                let_span,
             });
         }
         None

tests/ui/author/loop.rs

@@ -1,5 +1,9 @@
 #![feature(stmt_expr_attributes)]
-#![allow(clippy::never_loop, clippy::while_immutable_condition)]
+#![allow(
+    clippy::never_loop,
+    clippy::while_immutable_condition,
+    clippy::redundant_pattern_matching
+)]
 
 fn main() {
     #[clippy::author]

tests/ui/branches_sharing_code/shared_at_bottom.rs

@@ -1,6 +1,10 @@
 #![deny(clippy::if_same_then_else, clippy::branches_sharing_code)]
-#![allow(dead_code)]
-#![allow(clippy::equatable_if_let, clippy::uninlined_format_args)]
+#![allow(
+    clippy::equatable_if_let,
+    clippy::uninlined_format_args,
+    clippy::redundant_pattern_matching,
+    dead_code
+)]
 //@no-rustfix
 // This tests the branches_sharing_code lint at the end of blocks
 

tests/ui/branches_sharing_code/shared_at_bottom.stderr

@@ -1,5 +1,5 @@
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:31:5
+  --> $DIR/shared_at_bottom.rs:35:5
    |
 LL | /         let result = false;
 LL | |
@@ -26,7 +26,7 @@ LL ~     result;
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:51:5
+  --> $DIR/shared_at_bottom.rs:55:5
    |
 LL | /         println!("Same end of block");
 LL | |
@@ -40,7 +40,7 @@ LL +     println!("Same end of block");
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:69:5
+  --> $DIR/shared_at_bottom.rs:73:5
    |
 LL | /         println!(
 LL | |
@@ -61,7 +61,7 @@ LL +     );
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:82:9
+  --> $DIR/shared_at_bottom.rs:86:9
    |
 LL | /             println!("Hello World");
 LL | |
@@ -75,7 +75,7 @@ LL +         println!("Hello World");
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:99:5
+  --> $DIR/shared_at_bottom.rs:103:5
    |
 LL | /         let later_used_value = "A string value";
 LL | |
@@ -94,7 +94,7 @@ LL +     println!("{}", later_used_value);
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:113:5
+  --> $DIR/shared_at_bottom.rs:117:5
    |
 LL | /         let simple_examples = "I now identify as a &str :)";
 LL | |
@@ -112,7 +112,7 @@ LL +     println!("This is the new simple_example: {}", simple_examples);
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:179:5
+  --> $DIR/shared_at_bottom.rs:183:5
    |
 LL | /         x << 2
 LL | |
@@ -128,7 +128,7 @@ LL ~     x << 2;
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:188:5
+  --> $DIR/shared_at_bottom.rs:192:5
    |
 LL | /         x * 4
 LL | |
@@ -144,7 +144,7 @@ LL +     x * 4
    |
 
 error: all if blocks contain the same code at the end
-  --> $DIR/shared_at_bottom.rs:202:44
+  --> $DIR/shared_at_bottom.rs:206:44
    |
 LL |     if x == 17 { b = 1; a = 0x99; } else { a = 0x99; }
    |                                            ^^^^^^^^^^^

tests/ui/collapsible_if.fixed

@@ -3,7 +3,8 @@
     clippy::equatable_if_let,
     clippy::needless_if,
     clippy::nonminimal_bool,
-    clippy::eq_op
+    clippy::eq_op,
+    clippy::redundant_pattern_matching
 )]
 
 #[rustfmt::skip]

tests/ui/collapsible_if.rs

@@ -3,7 +3,8 @@
     clippy::equatable_if_let,
     clippy::needless_if,
     clippy::nonminimal_bool,
-    clippy::eq_op
+    clippy::eq_op,
+    clippy::redundant_pattern_matching
 )]
 
 #[rustfmt::skip]