From ba7374e517415696c383a6c7b79214d2168dff21 Mon Sep 17 00:00:00 2001
From: Fabian Wolff <fabian.wolff@alumni.ethz.ch>
Date: Wed, 1 Dec 2021 22:36:50 +0100
Subject: [PATCH] Improve diagnostic for missing half of binary operator in
 `if` condition

---
 compiler/rustc_parse/src/parser/expr.rs     | 51 +++++++++++++++++----
 src/test/ui/expr/if/if-without-block.stderr |  6 ++-
 src/test/ui/parser/issue-91421.rs           | 10 ++++
 src/test/ui/parser/issue-91421.stderr       | 21 +++++++++
 4 files changed, 77 insertions(+), 11 deletions(-)
 create mode 100644 src/test/ui/parser/issue-91421.rs
 create mode 100644 src/test/ui/parser/issue-91421.stderr

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index f7ee874c831..1dbd7bad0f0 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -1988,25 +1988,34 @@ impl<'a> Parser<'a> {
         let lo = self.prev_token.span;
         let cond = self.parse_cond_expr()?;
 
+        let missing_then_block_binop_span = || {
+            match cond.kind {
+                ExprKind::Binary(Spanned { span: binop_span, .. }, _, ref right)
+                    if let ExprKind::Block(..) = right.kind => Some(binop_span),
+                _ => None
+            }
+        };
+
         // Verify that the parsed `if` condition makes sense as a condition. If it is a block, then
         // verify that the last statement is either an implicit return (no `;`) or an explicit
         // return. This won't catch blocks with an explicit `return`, but that would be caught by
         // the dead code lint.
-        let thn = if self.eat_keyword(kw::Else) || !cond.returns() {
-            self.error_missing_if_cond(lo, cond.span)
+        let thn = if self.token.is_keyword(kw::Else) || !cond.returns() {
+            if let Some(binop_span) = missing_then_block_binop_span() {
+                self.error_missing_if_then_block(lo, None, Some(binop_span)).emit();
+                self.mk_block_err(cond.span)
+            } else {
+                self.error_missing_if_cond(lo, cond.span)
+            }
         } else {
             let attrs = self.parse_outer_attributes()?.take_for_recovery(); // For recovery.
             let not_block = self.token != token::OpenDelim(token::Brace);
-            let block = self.parse_block().map_err(|mut err| {
+            let block = self.parse_block().map_err(|err| {
                 if not_block {
-                    err.span_label(lo, "this `if` expression has a condition, but no block");
-                    if let ExprKind::Binary(_, _, ref right) = cond.kind {
-                        if let ExprKind::Block(_, _) = right.kind {
-                            err.help("maybe you forgot the right operand of the condition?");
-                        }
-                    }
+                    self.error_missing_if_then_block(lo, Some(err), missing_then_block_binop_span())
+                } else {
+                    err
                 }
-                err
             })?;
             self.error_on_if_block_attrs(lo, false, block.span, &attrs);
             block
@@ -2015,6 +2024,28 @@ impl<'a> Parser<'a> {
         Ok(self.mk_expr(lo.to(self.prev_token.span), ExprKind::If(cond, thn, els), attrs))
     }
 
+    fn error_missing_if_then_block(
+        &self,
+        if_span: Span,
+        err: Option<DiagnosticBuilder<'a>>,
+        binop_span: Option<Span>,
+    ) -> DiagnosticBuilder<'a> {
+        let msg = "this `if` expression has a condition, but no block";
+
+        let mut err = if let Some(mut err) = err {
+            err.span_label(if_span, msg);
+            err
+        } else {
+            self.struct_span_err(if_span, msg)
+        };
+
+        if let Some(binop_span) = binop_span {
+            err.span_help(binop_span, "maybe you forgot the right operand of the condition?");
+        }
+
+        err
+    }
+
     fn error_missing_if_cond(&self, lo: Span, span: Span) -> P<ast::Block> {
         let sp = self.sess.source_map().next_point(lo);
         self.struct_span_err(sp, "missing condition for `if` expression")
diff --git a/src/test/ui/expr/if/if-without-block.stderr b/src/test/ui/expr/if/if-without-block.stderr
index ee2bb62e2bb..d3f6ca07617 100644
--- a/src/test/ui/expr/if/if-without-block.stderr
+++ b/src/test/ui/expr/if/if-without-block.stderr
@@ -7,7 +7,11 @@ LL |     if 5 == {
 LL | }
    | ^ expected `{`
    |
-   = help: maybe you forgot the right operand of the condition?
+help: maybe you forgot the right operand of the condition?
+  --> $DIR/if-without-block.rs:3:10
+   |
+LL |     if 5 == {
+   |          ^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/issue-91421.rs b/src/test/ui/parser/issue-91421.rs
new file mode 100644
index 00000000000..9959df56638
--- /dev/null
+++ b/src/test/ui/parser/issue-91421.rs
@@ -0,0 +1,10 @@
+// Regression test for issue #91421.
+
+fn main() {
+    let value = if true && {
+    //~^ ERROR: this `if` expression has a condition, but no block
+    //~| HELP: maybe you forgot the right operand of the condition?
+        3
+        //~^ ERROR: mismatched types [E0308]
+    } else { 4 };
+}
diff --git a/src/test/ui/parser/issue-91421.stderr b/src/test/ui/parser/issue-91421.stderr
new file mode 100644
index 00000000000..04284d5e3b2
--- /dev/null
+++ b/src/test/ui/parser/issue-91421.stderr
@@ -0,0 +1,21 @@
+error: this `if` expression has a condition, but no block
+  --> $DIR/issue-91421.rs:4:17
+   |
+LL |     let value = if true && {
+   |                 ^^
+   |
+help: maybe you forgot the right operand of the condition?
+  --> $DIR/issue-91421.rs:4:25
+   |
+LL |     let value = if true && {
+   |                         ^^
+
+error[E0308]: mismatched types
+  --> $DIR/issue-91421.rs:7:9
+   |
+LL |         3
+   |         ^ expected `bool`, found integer
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0308`.