Stop inserting redundant parenthesis around desugared match expressions (#16102)

Currently the `sugg` utility treats all `ExprKind::Match` expressions as
potentially needing brackets, and therefore many lints will add
parenthesis around them. However this includes desugared match
expressions like the `?` and `.await` operators.

In this PR I have updated the utility to only treat match expressions
which include a code block as needing parenthesis, as the other types
have similar precedence rules and expectations to things like member
access and I think can be treated like as such.

While this change is small on paper it touches a large amount of code
due to changing a cross cutting concern, I am happy to add additional
tests if we think it is needed, but I wanted to get a feel for if this
is even a sensible change to be doing and what the expectations were
around the level of testing needed before investing more time into it.

Regarding not putting a specific lint in the changelog, determining all
the lints this could possibly effect would be possible but take some
time, and I wonder if it would be a bit too noisy in the changelog. Open
to suggestions about how best to address that.

fixes #16045

changelog: stop inserting unnecessary brackets around `x?` and `x.await`
expressions in suggestions

Author: Alexendoo

clippy_utils/src/sugg.rs

@@ -8,7 +8,7 @@ use rustc_ast::util::parser::AssocOp;
 use rustc_ast::{UnOp, ast};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
-use rustc_hir::{self as hir, Closure, ExprKind, HirId, MutTy, Node, TyKind};
+use rustc_hir::{self as hir, Closure, ExprKind, HirId, MatchSource, MutTy, Node, TyKind};
 use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
 use rustc_lint::{EarlyContext, LateContext, LintContext};
 use rustc_middle::hir::place::ProjectionKind;
@@ -146,7 +146,9 @@ impl<'a> Sugg<'a> {
             | ExprKind::Let(..)
             | ExprKind::Closure { .. }
             | ExprKind::Unary(..)
-            | ExprKind::Match(..) => Sugg::MaybeParen(get_snippet(expr.span)),
+            | ExprKind::Match(_, _,
+                MatchSource::Normal | MatchSource::Postfix | MatchSource::ForLoopDesugar
+            ) => Sugg::MaybeParen(get_snippet(expr.span)),
             ExprKind::Continue(..)
             | ExprKind::Yield(..)
             | ExprKind::Array(..)
@@ -169,7 +171,10 @@ impl<'a> Sugg<'a> {
             | ExprKind::Tup(..)
             | ExprKind::Use(..)
             | ExprKind::Err(_)
-            | ExprKind::UnsafeBinderCast(..) => Sugg::NonParen(get_snippet(expr.span)),
+            | ExprKind::UnsafeBinderCast(..)
+            | ExprKind::Match(_, _,
+                MatchSource::AwaitDesugar | MatchSource::TryDesugar(_) | MatchSource::FormatArgs
+            ) => Sugg::NonParen(get_snippet(expr.span)),
             ExprKind::DropTemps(inner) => Self::hir_from_snippet(cx, inner, get_snippet),
             ExprKind::Assign(lhs, rhs, _) => {
                 Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span))

tests/ui/cast.rs

@@ -582,3 +582,13 @@ mod issue14150 {
         //~^ cast_possible_wrap
     }
 }
+
+fn issue16045() {
+    fn f() -> Result<(), ()> {
+        let val = Ok::<_, ()>(0u8);
+        _ = val? as i8;
+        //~^ cast_possible_wrap
+
+        Ok(())
+    }
+}

tests/ui/cast.stderr

@@ -764,5 +764,11 @@ error: casting `u8` to `i8` may wrap around the value
 LL |         _ = 1u8 as i8;
    |             ^^^^^^^^^
 
-error: aborting due to 94 previous errors
+error: casting `u8` to `i8` may wrap around the value
+  --> tests/ui/cast.rs:589:13
+   |
+LL |         _ = val? as i8;
+   |             ^^^^^^^^^^ help: if this is intentional, use `cast_signed()` instead: `val?.cast_signed()`
+
+error: aborting due to 95 previous errors
 

tests/ui/redundant_pattern_matching_option.fixed

@@ -166,3 +166,32 @@ fn issue13902() {
         //~^ redundant_pattern_matching
     }
 }
+
+fn issue16045() {
+    fn f() -> Result<(), ()> {
+        let x = Ok::<_, ()>(Some(123));
+        if x?.is_some() {
+            //~^ redundant_pattern_matching
+        }
+
+        Ok(())
+    }
+
+    async fn g() {
+        struct F {
+            x: Option<u32>,
+        }
+
+        impl Future for F {
+            type Output = Option<u32>;
+
+            fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> std::task::Poll<Self::Output> {
+                std::task::Poll::Ready(self.x)
+            }
+        }
+        let x = F { x: Some(123) };
+        if x.await.is_some() {
+            //~^ redundant_pattern_matching
+        }
+    }
+}

tests/ui/redundant_pattern_matching_option.rs

@@ -202,3 +202,32 @@ fn issue13902() {
         //~^ redundant_pattern_matching
     }
 }
+
+fn issue16045() {
+    fn f() -> Result<(), ()> {
+        let x = Ok::<_, ()>(Some(123));
+        if let Some(_) = x? {
+            //~^ redundant_pattern_matching
+        }
+
+        Ok(())
+    }
+
+    async fn g() {
+        struct F {
+            x: Option<u32>,
+        }
+
+        impl Future for F {
+            type Output = Option<u32>;
+
+            fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> std::task::Poll<Self::Output> {
+                std::task::Poll::Ready(self.x)
+            }
+        }
+        let x = F { x: Some(123) };
+        if let Some(_) = x.await {
+            //~^ redundant_pattern_matching
+        }
+    }
+}

tests/ui/redundant_pattern_matching_option.stderr

@@ -224,5 +224,17 @@ error: redundant pattern matching, consider using `is_none()`
 LL |         let _ = matches!(*p, None);
    |                 ^^^^^^^^^^^^^^^^^^ help: try: `(*p).is_none()`
 
-error: aborting due to 31 previous errors
+error: redundant pattern matching, consider using `is_some()`
+  --> tests/ui/redundant_pattern_matching_option.rs:209:16
+   |
+LL |         if let Some(_) = x? {
+   |         -------^^^^^^^----- help: try: `if x?.is_some()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> tests/ui/redundant_pattern_matching_option.rs:229:16
+   |
+LL |         if let Some(_) = x.await {
+   |         -------^^^^^^^---------- help: try: `if x.await.is_some()`
+
+error: aborting due to 33 previous errors