Auto merge of #146044 - estebank:issue-88727, r=chenyukang

Suggest parentheses when `match` or `if` expression in binop is parsed as statement

```
error[E0308]: mismatched types
  --> $DIR/expr-as-stmt.rs:81:5
   |
LL |     match () { _ => true } && match () { _ => true };
   |     ^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `bool`
   |
help: parentheses are required to parse this as an expression
   |
LL |     (match () { _ => true }) && match () { _ => true };
   |     +                      +
```

Address the common case from #88727. The original parse error is still outstanding, but the cases brought up in the thread are resolved.

Author: bors

compiler/rustc_hir_typeck/src/coercion.rs

@@ -1897,7 +1897,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                     fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
                 }
             }
-        };
+        }
 
         // If this is due to an explicit `return`, suggest adding a return type.
         if let Some((fn_id, fn_decl)) = fcx.get_fn_decl(block_or_return_id)

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -1912,7 +1912,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             hir::StmtKind::Expr(ref expr) => {
                 // Check with expected type of `()`.
                 self.check_expr_has_type_or_error(expr, self.tcx.types.unit, |err| {
-                    if expr.can_have_side_effects() {
+                    if self.is_next_stmt_expr_continuation(stmt.hir_id)
+                        && let hir::ExprKind::Match(..) | hir::ExprKind::If(..) = expr.kind
+                    {
+                        // We have something like `match () { _ => true } && true`. Suggest
+                        // wrapping in parentheses. We find the statement or expression
+                        // following the `match` (`&& true`) and see if it is something that
+                        // can reasonably be interpreted as a binop following an expression.
+                        err.multipart_suggestion(
+                            "parentheses are required to parse this as an expression",
+                            vec![
+                                (expr.span.shrink_to_lo(), "(".to_string()),
+                                (expr.span.shrink_to_hi(), ")".to_string()),
+                            ],
+                            Applicability::MachineApplicable,
+                        );
+                    } else if expr.can_have_side_effects() {
                         self.suggest_semicolon_at_end(expr.span, err);
                     }
                 });

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1,3 +1,4 @@
+// ignore-tidy-filelength
 use core::cmp::min;
 use core::iter;
 
@@ -766,56 +767,121 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         needs_block: bool,
         parent_is_closure: bool,
     ) {
-        if expected.is_unit() {
-            // `BlockTailExpression` only relevant if the tail expr would be
-            // useful on its own.
-            match expression.kind {
-                ExprKind::Call(..)
-                | ExprKind::MethodCall(..)
-                | ExprKind::Loop(..)
-                | ExprKind::If(..)
-                | ExprKind::Match(..)
-                | ExprKind::Block(..)
-                    if expression.can_have_side_effects()
-                        // If the expression is from an external macro, then do not suggest
-                        // adding a semicolon, because there's nowhere to put it.
-                        // See issue #81943.
-                        && !expression.span.in_external_macro(self.tcx.sess.source_map()) =>
+        if !expected.is_unit() {
+            return;
+        }
+        // `BlockTailExpression` only relevant if the tail expr would be
+        // useful on its own.
+        match expression.kind {
+            ExprKind::Call(..)
+            | ExprKind::MethodCall(..)
+            | ExprKind::Loop(..)
+            | ExprKind::If(..)
+            | ExprKind::Match(..)
+            | ExprKind::Block(..)
+                if expression.can_have_side_effects()
+                    // If the expression is from an external macro, then do not suggest
+                    // adding a semicolon, because there's nowhere to put it.
+                    // See issue #81943.
+                    && !expression.span.in_external_macro(self.tcx.sess.source_map()) =>
+            {
+                if needs_block {
+                    err.multipart_suggestion(
+                        "consider using a semicolon here",
+                        vec![
+                            (expression.span.shrink_to_lo(), "{ ".to_owned()),
+                            (expression.span.shrink_to_hi(), "; }".to_owned()),
+                        ],
+                        Applicability::MachineApplicable,
+                    );
+                } else if let hir::Node::Block(block) = self.tcx.parent_hir_node(expression.hir_id)
+                    && let hir::Node::Expr(expr) = self.tcx.parent_hir_node(block.hir_id)
+                    && let hir::Node::Expr(if_expr) = self.tcx.parent_hir_node(expr.hir_id)
+                    && let hir::ExprKind::If(_cond, _then, Some(_else)) = if_expr.kind
+                    && let hir::Node::Stmt(stmt) = self.tcx.parent_hir_node(if_expr.hir_id)
+                    && let hir::StmtKind::Expr(_) = stmt.kind
+                    && self.is_next_stmt_expr_continuation(stmt.hir_id)
                 {
-                    if needs_block {
-                        err.multipart_suggestion(
-                            "consider using a semicolon here",
-                            vec![
-                                (expression.span.shrink_to_lo(), "{ ".to_owned()),
-                                (expression.span.shrink_to_hi(), "; }".to_owned()),
-                            ],
-                            Applicability::MachineApplicable,
-                        );
-                    } else {
-                        err.span_suggestion(
-                            expression.span.shrink_to_hi(),
-                            "consider using a semicolon here",
-                            ";",
-                            Applicability::MachineApplicable,
-                        );
-                    }
+                    err.multipart_suggestion(
+                        "parentheses are required to parse this as an expression",
+                        vec![
+                            (stmt.span.shrink_to_lo(), "(".to_string()),
+                            (stmt.span.shrink_to_hi(), ")".to_string()),
+                        ],
+                        Applicability::MachineApplicable,
+                    );
+                } else {
+                    err.span_suggestion(
+                        expression.span.shrink_to_hi(),
+                        "consider using a semicolon here",
+                        ";",
+                        Applicability::MachineApplicable,
+                    );
                 }
-                ExprKind::Path(..) | ExprKind::Lit(_)
-                    if parent_is_closure
-                        && !expression.span.in_external_macro(self.tcx.sess.source_map()) =>
+            }
+            ExprKind::Path(..) | ExprKind::Lit(_)
+                if parent_is_closure
+                    && !expression.span.in_external_macro(self.tcx.sess.source_map()) =>
+            {
+                err.span_suggestion_verbose(
+                    expression.span.shrink_to_lo(),
+                    "consider ignoring the value",
+                    "_ = ",
+                    Applicability::MachineApplicable,
+                );
+            }
+            _ => {
+                if let hir::Node::Block(block) = self.tcx.parent_hir_node(expression.hir_id)
+                    && let hir::Node::Expr(expr) = self.tcx.parent_hir_node(block.hir_id)
+                    && let hir::Node::Expr(if_expr) = self.tcx.parent_hir_node(expr.hir_id)
+                    && let hir::ExprKind::If(_cond, _then, Some(_else)) = if_expr.kind
+                    && let hir::Node::Stmt(stmt) = self.tcx.parent_hir_node(if_expr.hir_id)
+                    && let hir::StmtKind::Expr(_) = stmt.kind
+                    && self.is_next_stmt_expr_continuation(stmt.hir_id)
                 {
-                    err.span_suggestion_verbose(
-                        expression.span.shrink_to_lo(),
-                        "consider ignoring the value",
-                        "_ = ",
+                    // The error is pointing at an arm of an if-expression, and we want to get the
+                    // `Span` of the whole if-expression for the suggestion. This only works for a
+                    // single level of nesting, which is fine.
+                    // We have something like `if true { false } else { true } && true`. Suggest
+                    // wrapping in parentheses. We find the statement or expression following the
+                    // `if` (`&& true`) and see if it is something that can reasonably be
+                    // interpreted as a binop following an expression.
+                    err.multipart_suggestion(
+                        "parentheses are required to parse this as an expression",
+                        vec![
+                            (stmt.span.shrink_to_lo(), "(".to_string()),
+                            (stmt.span.shrink_to_hi(), ")".to_string()),
+                        ],
                         Applicability::MachineApplicable,
                     );
                 }
-                _ => (),
             }
         }
     }
 
+    pub(crate) fn is_next_stmt_expr_continuation(&self, hir_id: HirId) -> bool {
+        if let hir::Node::Block(b) = self.tcx.parent_hir_node(hir_id)
+            && let mut stmts = b.stmts.iter().skip_while(|s| s.hir_id != hir_id)
+            && let Some(_) = stmts.next() // The statement the statement that was passed in
+            && let Some(next) = match (stmts.next(), b.expr) { // The following statement
+                (Some(next), _) => match next.kind {
+                    hir::StmtKind::Expr(next) | hir::StmtKind::Semi(next) => Some(next),
+                    _ => None,
+                },
+                (None, Some(next)) => Some(next),
+                _ => None,
+            }
+            && let hir::ExprKind::AddrOf(..) // prev_stmt && next
+                | hir::ExprKind::Unary(..) // prev_stmt * next
+                | hir::ExprKind::Err(_) = next.kind
+        // prev_stmt + next
+        {
+            true
+        } else {
+            false
+        }
+    }
+
     /// A possible error is to forget to add a return type that is needed:
     ///
     /// ```compile_fail,E0308

tests/ui/parser/expr-as-stmt-2.rs

@@ -7,4 +7,25 @@ fn foo(a: Option<u32>, b: Option<u32>) -> bool {
     if let Some(y) = a { true } else { false }
 }
 
-fn main() {}
+fn bar() -> bool {
+    false
+}
+
+fn main() {
+    if true { true } else { false } && true;
+    //~^ ERROR mismatched types
+    //~| ERROR mismatched types
+    if true { true } else { false } && if true { true } else { false };
+    //~^ ERROR mismatched types
+    //~| ERROR mismatched types
+    if true { true } else { false } if true { true } else { false };
+    //~^ ERROR mismatched types
+    //~| ERROR mismatched types
+    if true { bar() } else { bar() } && if true { bar() } else { bar() };
+    //~^ ERROR mismatched types
+    //~| ERROR mismatched types
+    if true { bar() } else { bar() } if true { bar() } else { bar() };
+    //~^ ERROR mismatched types
+    //~| ERROR mismatched types
+    let _ = if true { true } else { false } && true; // ok
+}