Handle let-else initializer edge case errors

Author: camsteffen

compiler/rustc_ast/src/util/classify.rs

@@ -23,3 +23,30 @@ pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool {
             | ast::ExprKind::TryBlock(..)
     )
 }
+
+/// If an expression ends with `}`, returns the innermost expression ending in the `}`
+pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
+    use ast::ExprKind::*;
+
+    loop {
+        match &expr.kind {
+            AddrOf(_, _, e)
+            | Assign(_, e, _)
+            | AssignOp(_, _, e)
+            | Binary(_, _, e)
+            | Box(e)
+            | Break(_, Some(e))
+            | Closure(.., e, _)
+            | Let(_, e, _)
+            | Range(_, Some(e), _)
+            | Ret(Some(e))
+            | Unary(_, e)
+            | Yield(Some(e)) => {
+                expr = e;
+            }
+            Async(..) | Block(..) | ForLoop(..) | If(..) | Loop(..) | Match(..) | Struct(..)
+            | TryBlock(..) | While(..) => break Some(expr),
+            _ => break None,
+        }
+    }
+}

compiler/rustc_lint/src/unused.rs

@@ -1,7 +1,7 @@
 use crate::Lint;
 use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
 use rustc_ast as ast;
-use rustc_ast::util::parser;
+use rustc_ast::util::{classify, parser};
 use rustc_ast::{ExprKind, StmtKind};
 use rustc_ast_pretty::pprust;
 use rustc_errors::{pluralize, Applicability};
@@ -382,6 +382,7 @@ enum UnusedDelimsCtx {
     FunctionArg,
     MethodArg,
     AssignedValue,
+    AssignedValueLetElse,
     IfCond,
     WhileCond,
     ForIterExpr,
@@ -398,7 +399,9 @@ impl From<UnusedDelimsCtx> for &'static str {
         match ctx {
             UnusedDelimsCtx::FunctionArg => "function argument",
             UnusedDelimsCtx::MethodArg => "method argument",
-            UnusedDelimsCtx::AssignedValue => "assigned value",
+            UnusedDelimsCtx::AssignedValue | UnusedDelimsCtx::AssignedValueLetElse => {
+                "assigned value"
+            }
             UnusedDelimsCtx::IfCond => "`if` condition",
             UnusedDelimsCtx::WhileCond => "`while` condition",
             UnusedDelimsCtx::ForIterExpr => "`for` iterator expression",
@@ -441,14 +444,26 @@ trait UnusedDelimLint {
         right_pos: Option<BytePos>,
     );
 
-    fn is_expr_delims_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool {
+    fn is_expr_delims_necessary(
+        inner: &ast::Expr,
+        followed_by_block: bool,
+        followed_by_else: bool,
+    ) -> bool {
+        if followed_by_else {
+            match inner.kind {
+                ast::ExprKind::Binary(op, ..) if op.node.lazy() => return true,
+                _ if classify::expr_trailing_brace(inner).is_some() => return true,
+                _ => {}
+            }
+        }
+
         // Prevent false-positives in cases like `fn x() -> u8 { ({ 0 } + 1) }`
         let lhs_needs_parens = {
             let mut innermost = inner;
             loop {
                 if let ExprKind::Binary(_, lhs, _rhs) = &innermost.kind {
                     innermost = lhs;
-                    if !rustc_ast::util::classify::expr_requires_semi_to_be_stmt(innermost) {
+                    if !classify::expr_requires_semi_to_be_stmt(innermost) {
                         break true;
                     }
                 } else {
@@ -618,15 +633,12 @@ trait UnusedDelimLint {
     fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
         match s.kind {
             StmtKind::Local(ref local) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => {
-                if let Some(value) = local.kind.init() {
-                    self.check_unused_delims_expr(
-                        cx,
-                        &value,
-                        UnusedDelimsCtx::AssignedValue,
-                        false,
-                        None,
-                        None,
-                    );
+                if let Some((init, els)) = local.kind.init_else_opt() {
+                    let ctx = match els {
+                        None => UnusedDelimsCtx::AssignedValue,
+                        Some(_) => UnusedDelimsCtx::AssignedValueLetElse,
+                    };
+                    self.check_unused_delims_expr(cx, init, ctx, false, None, None);
                 }
             }
             StmtKind::Expr(ref expr) => {
@@ -702,7 +714,8 @@ impl UnusedDelimLint for UnusedParens {
     ) {
         match value.kind {
             ast::ExprKind::Paren(ref inner) => {
-                if !Self::is_expr_delims_necessary(inner, followed_by_block)
+                let followed_by_else = ctx == UnusedDelimsCtx::AssignedValueLetElse;
+                if !Self::is_expr_delims_necessary(inner, followed_by_block, followed_by_else)
                     && value.attrs.is_empty()
                     && !value.span.from_expansion()
                     && (ctx != UnusedDelimsCtx::LetScrutineeExpr
@@ -941,7 +954,7 @@ impl UnusedDelimLint for UnusedBraces {
                 // FIXME(const_generics): handle paths when #67075 is fixed.
                 if let [stmt] = inner.stmts.as_slice() {
                     if let ast::StmtKind::Expr(ref expr) = stmt.kind {
-                        if !Self::is_expr_delims_necessary(expr, followed_by_block)
+                        if !Self::is_expr_delims_necessary(expr, followed_by_block, false)
                             && (ctx != UnusedDelimsCtx::AnonConst
                                 || matches!(expr.kind, ast::ExprKind::Lit(_)))
                             && !cx.sess().source_map().is_multiline(value.span)

compiler/rustc_parse/src/parser/stmt.rs

@@ -298,6 +298,8 @@ impl<'a> Parser<'a> {
             Some(init) => {
                 if self.eat_keyword(kw::Else) {
                     let els = self.parse_block()?;
+                    self.check_let_else_init_bool_expr(&init);
+                    self.check_let_else_init_trailing_brace(&init);
                     LocalKind::InitElse(init, els)
                 } else {
                     LocalKind::Init(init)
@@ -308,6 +310,50 @@ impl<'a> Parser<'a> {
         Ok(P(ast::Local { ty, pat, kind, id: DUMMY_NODE_ID, span: lo.to(hi), attrs, tokens: None }))
     }
 
+    fn check_let_else_init_bool_expr(&self, init: &ast::Expr) {
+        if let ast::ExprKind::Binary(op, ..) = init.kind {
+            if op.node.lazy() {
+                let suggs = vec![
+                    (init.span.shrink_to_lo(), "(".to_string()),
+                    (init.span.shrink_to_hi(), ")".to_string()),
+                ];
+                self.struct_span_err(
+                    init.span,
+                    &format!(
+                        "a `{}` expression cannot be directly assigned in `let...else`",
+                        op.node.to_string()
+                    ),
+                )
+                .multipart_suggestion(
+                    "wrap the expression in parenthesis",
+                    suggs,
+                    Applicability::MachineApplicable,
+                )
+                .emit();
+            }
+        }
+    }
+
+    fn check_let_else_init_trailing_brace(&self, init: &ast::Expr) {
+        if let Some(trailing) = classify::expr_trailing_brace(init) {
+            let err_span = trailing.span.with_lo(trailing.span.hi() - BytePos(1));
+            let suggs = vec![
+                (trailing.span.shrink_to_lo(), "(".to_string()),
+                (trailing.span.shrink_to_hi(), ")".to_string()),
+            ];
+            self.struct_span_err(
+                err_span,
+                "right curly brace `}` before `else` in a `let...else` statement not allowed",
+            )
+            .multipart_suggestion(
+                "try wrapping the expression in parenthesis",
+                suggs,
+                Applicability::MachineApplicable,
+            )
+            .emit();
+        }
+    }
+
     /// Parses the RHS of a local variable declaration (e.g., `= 14;`).
     fn parse_initializer(&mut self, eq_optional: bool) -> PResult<'a, Option<P<Expr>>> {
         let eq_consumed = match self.token.kind {