diagnostic: suggest parens when users want logical ops, but get closures

4 files changed, 151 insertions, 3 deletions

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 7cb8c35b868..399596692d5 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -372,10 +372,17 @@ impl<'a> Parser<'a> {
                 self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span);
                 false
             }
-            (true, Some(AssocOp::LAnd)) => {
+            (true, Some(AssocOp::LAnd)) |
+            (true, Some(AssocOp::LOr)) |
+            (true, Some(AssocOp::BitOr)) => {
                 // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`. Separated from the
                 // above due to #74233.
                 // These cases are ambiguous and can't be identified in the parser alone.
+                //
+                // Bitwise AND is left out because guessing intent is hard. We can make
+                // suggestions based on the assumption that double-refs are rarely intentional,
+                // and closures are distinct enough that they don't get mixed up with their
+                // return value.
                 let sp = self.sess.source_map().start_point(self.token.span);
                 self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span);
                 false
@@ -1244,7 +1251,14 @@ impl<'a> Parser<'a> {
         } else if self.check(&token::OpenDelim(token::Brace)) {
             self.parse_block_expr(None, lo, BlockCheckMode::Default, attrs)
         } else if self.check(&token::BinOp(token::Or)) || self.check(&token::OrOr) {
-            self.parse_closure_expr(attrs)
+            self.parse_closure_expr(attrs).map_err(|mut err| {
+                // If the input is something like `if a { 1 } else { 2 } | if a { 3 } else { 4 }`
+                // then suggest parens around the lhs.
+                if let Some(sp) = self.sess.ambiguous_block_expr_parse.borrow().get(&lo) {
+                    self.sess.expr_parentheses_needed(&mut err, *sp);
+                }
+                err
+            })
         } else if self.check(&token::OpenDelim(token::Bracket)) {
             self.parse_array_or_repeat_expr(attrs, token::Bracket)
         } else if self.check_path() {
diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed
index 101959d6da0..36709eea17c 100644
--- a/src/test/ui/parser/expr-as-stmt.fixed
+++ b/src/test/ui/parser/expr-as-stmt.fixed
@@ -37,4 +37,31 @@ fn qux() -> u32 {
     //~^ ERROR mismatched types
 }
 
+fn space_cadet() -> bool {
+    ({ true }) | { true } //~ ERROR E0308
+    //~^ ERROR expected parameter name
+}
+
+fn revenge_from_mars() -> bool {
+    ({ true }) && { true } //~ ERROR E0308
+    //~^ ERROR mismatched types
+}
+
+fn attack_from_mars() -> bool {
+    ({ true }) || { true } //~ ERROR E0308
+    //~^ ERROR mismatched types
+}
+
+// This gets corrected by adding a semicolon, instead of parens.
+// It's placed here to help keep track of the way this diagnostic
+// needs to interact with type checking to avoid MachineApplicable
+// suggestions that actually break stuff.
+//
+// If you're wondering what happens if that `foo()` is a `true` like
+// all the ones above use? Nothing. It makes neither suggestion in
+// that case.
+fn asteroids() -> impl FnOnce() -> bool {
+    { foo(); } || { true } //~ ERROR E0308
+}
+
 fn main() {}
diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs
index 45c4f977502..92bb972b240 100644
--- a/src/test/ui/parser/expr-as-stmt.rs
+++ b/src/test/ui/parser/expr-as-stmt.rs
@@ -37,4 +37,31 @@ fn qux() -> u32 {
     //~^ ERROR mismatched types
 }
 
+fn space_cadet() -> bool {
+    { true } | { true } //~ ERROR E0308
+    //~^ ERROR expected parameter name
+}
+
+fn revenge_from_mars() -> bool {
+    { true } && { true } //~ ERROR E0308
+    //~^ ERROR mismatched types
+}
+
+fn attack_from_mars() -> bool {
+    { true } || { true } //~ ERROR E0308
+    //~^ ERROR mismatched types
+}
+
+// This gets corrected by adding a semicolon, instead of parens.
+// It's placed here to help keep track of the way this diagnostic
+// needs to interact with type checking to avoid MachineApplicable
+// suggestions that actually break stuff.
+//
+// If you're wondering what happens if that `foo()` is a `true` like
+// all the ones above use? Nothing. It makes neither suggestion in
+// that case.
+fn asteroids() -> impl FnOnce() -> bool {
+    { foo() } || { true } //~ ERROR E0308
+}
+
 fn main() {}
diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr
index cae775099e0..df0e4dcb16e 100644
--- a/src/test/ui/parser/expr-as-stmt.stderr
+++ b/src/test/ui/parser/expr-as-stmt.stderr
@@ -44,6 +44,25 @@ LL |         _ => 1,
 LL ~     }) > 0
    |
 
+error: expected parameter name, found `{`
+  --> $DIR/expr-as-stmt.rs:41:16
+   |
+LL |     { true } | { true }
+   |                ^ expected parameter name
+   |
+help: parentheses are required to parse this as an expression
+   |
+LL |     ({ true }) | { true }
+   |     +        +
+
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:64:7
+   |
+LL |     { foo() } || { true }
+   |       ^^^^^- help: consider using a semicolon here: `;`
+   |       |
+   |       expected `()`, found `i32`
+
 error[E0308]: mismatched types
   --> $DIR/expr-as-stmt.rs:8:6
    |
@@ -121,7 +140,68 @@ help: parentheses are required to parse this as an expression
 LL |     ({2}) - 2
    |     +   +
 
-error: aborting due to 11 previous errors
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:41:7
+   |
+LL |     { true } | { true }
+   |       ^^^^ expected `()`, found `bool`
+   |
+help: you might have meant to return this value
+   |
+LL |     { return true; } | { true }
+   |       ++++++     +
+
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:46:7
+   |
+LL |     { true } && { true }
+   |       ^^^^ expected `()`, found `bool`
+   |
+help: you might have meant to return this value
+   |
+LL |     { return true; } && { true }
+   |       ++++++     +
+
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:46:14
+   |
+LL | fn revenge_from_mars() -> bool {
+   |                           ---- expected `bool` because of return type
+LL |     { true } && { true }
+   |              ^^^^^^^^^^^ expected `bool`, found `&&bool`
+   |
+help: parentheses are required to parse this as an expression
+   |
+LL |     ({ true }) && { true }
+   |     +        +
+
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:51:7
+   |
+LL |     { true } || { true }
+   |       ^^^^ expected `()`, found `bool`
+   |
+help: you might have meant to return this value
+   |
+LL |     { return true; } || { true }
+   |       ++++++     +
+
+error[E0308]: mismatched types
+  --> $DIR/expr-as-stmt.rs:51:14
+   |
+LL | fn attack_from_mars() -> bool {
+   |                          ---- expected `bool` because of return type
+LL |     { true } || { true }
+   |              ^^^^^^^^^^^ expected `bool`, found closure
+   |
+   = note: expected type `bool`
+           found closure `[closure@$DIR/expr-as-stmt.rs:51:14: 51:25]`
+help: parentheses are required to parse this as an expression
+   |
+LL |     ({ true }) || { true }
+   |     +        +
+
+error: aborting due to 18 previous errors
 
 Some errors have detailed explanations: E0308, E0600, E0614.
 For more information about an error, try `rustc --explain E0308`.