Auto merge of #10785 - Centri3:diverting_sub_expression, r=Jarcho

Fix `diverging_sub_expression` not checking body of block

Fixes #10776

This also adds a warning to the test `ui/never_loop.rs`, not sure if this is correct or not.

changelog: [`diverging_sub_expression`]: Fix false negatives with body of block

Author: bors

clippy_lints/src/mixed_read_write_in_expression.rs

@@ -114,7 +114,7 @@ struct DivergenceVisitor<'a, 'tcx> {
 impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
     fn maybe_walk_expr(&mut self, e: &'tcx Expr<'_>) {
         match e.kind {
-            ExprKind::Closure { .. } => {},
+            ExprKind::Closure(..) | ExprKind::If(..) | ExprKind::Loop(..) => {},
             ExprKind::Match(e, arms, _) => {
                 self.visit_expr(e);
                 for arm in arms {
@@ -128,6 +128,7 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
             _ => walk_expr(self, e),
         }
     }
+
     fn report_diverging_sub_expr(&mut self, e: &Expr<'_>) {
         span_lint(self.cx, DIVERGING_SUB_EXPRESSION, e.span, "sub-expression diverges");
     }
@@ -136,6 +137,15 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
 impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
     fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
         match e.kind {
+            // fix #10776
+            ExprKind::Block(block, ..) => match (block.stmts, block.expr) {
+                ([], Some(e)) => self.visit_expr(e),
+                ([stmt], None) => match stmt.kind {
+                    StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
+                    _ => {},
+                },
+                _ => {},
+            },
             ExprKind::Continue(_) | ExprKind::Break(_, _) | ExprKind::Ret(_) => self.report_diverging_sub_expr(e),
             ExprKind::Call(func, _) => {
                 let typ = self.cx.typeck_results().expr_ty(func);

tests/ui/diverging_sub_expression.rs

@@ -1,5 +1,6 @@
 #![warn(clippy::diverging_sub_expression)]
 #![allow(clippy::match_same_arms, clippy::overly_complex_bool_expr)]
+#![allow(clippy::nonminimal_bool)]
 #[allow(clippy::empty_loop)]
 fn diverge() -> ! {
     loop {}
@@ -21,6 +22,7 @@ fn main() {
 }
 
 #[allow(dead_code, unused_variables)]
+#[rustfmt::skip]
 fn foobar() {
     loop {
         let x = match 5 {
@@ -35,6 +37,20 @@ fn foobar() {
                 99 => return,
                 _ => true || panic!("boo"),
             },
+            // lint blocks as well
+            15 => true || { return; },
+            16 => false || { return; },
+            // ... and when it's a single expression
+            17 => true || { return },
+            18 => false || { return },
+            // ... but not when there's both an expression and a statement
+            19 => true || { _ = 1; return },
+            20 => false || { _ = 1; return },
+            // ... or multiple statements
+            21 => true || { _ = 1; return; },
+            22 => false || { _ = 1; return; },
+            23 => true || { return; true },
+            24 => true || { return; true },
             _ => true || break,
         };
     }

tests/ui/diverging_sub_expression.stderr

@@ -1,40 +1,72 @@
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:19:10
+  --> $DIR/diverging_sub_expression.rs:20:10
    |
 LL |     b || diverge();
    |          ^^^^^^^^^
    |
    = note: `-D clippy::diverging-sub-expression` implied by `-D warnings`
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:20:10
+  --> $DIR/diverging_sub_expression.rs:21:10
    |
 LL |     b || A.foo();
    |          ^^^^^^^
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:29:26
+  --> $DIR/diverging_sub_expression.rs:31:26
    |
 LL |             6 => true || return,
    |                          ^^^^^^
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:30:26
+  --> $DIR/diverging_sub_expression.rs:32:26
    |
 LL |             7 => true || continue,
    |                          ^^^^^^^^
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:33:26
+  --> $DIR/diverging_sub_expression.rs:35:26
    |
 LL |             3 => true || diverge(),
    |                          ^^^^^^^^^
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:38:26
+  --> $DIR/diverging_sub_expression.rs:38:30
+   |
+LL |                 _ => true || panic!("boo"),
+   |                              ^^^^^^^^^^^^^
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: sub-expression diverges
+  --> $DIR/diverging_sub_expression.rs:41:29
+   |
+LL |             15 => true || { return; },
+   |                             ^^^^^^
+
+error: sub-expression diverges
+  --> $DIR/diverging_sub_expression.rs:42:30
+   |
+LL |             16 => false || { return; },
+   |                              ^^^^^^
+
+error: sub-expression diverges
+  --> $DIR/diverging_sub_expression.rs:44:29
+   |
+LL |             17 => true || { return },
+   |                             ^^^^^^
+
+error: sub-expression diverges
+  --> $DIR/diverging_sub_expression.rs:45:30
+   |
+LL |             18 => false || { return },
+   |                              ^^^^^^
+
+error: sub-expression diverges
+  --> $DIR/diverging_sub_expression.rs:54:26
    |
 LL |             _ => true || break,
    |                          ^^^^^
 
-error: aborting due to 6 previous errors
+error: aborting due to 11 previous errors
 

tests/ui/never_loop.stderr

@@ -126,6 +126,14 @@ LL | |         }
 LL | |     }
    | |_____^
 
+error: sub-expression diverges
+  --> $DIR/never_loop.rs:247:17
+   |
+LL |                 break 'a;
+   |                 ^^^^^^^^
+   |
+   = note: `-D clippy::diverging-sub-expression` implied by `-D warnings`
+
 error: this loop never actually loops
   --> $DIR/never_loop.rs:278:13
    |
@@ -139,5 +147,5 @@ help: if you need the first element of the iterator, try writing
 LL |             if let Some(_) = (0..20).next() {
    |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-error: aborting due to 12 previous errors
+error: aborting due to 13 previous errors