From: Cameron Steffen Date: Fri, 30 Jul 2021 22:12:11 +0000 (-0500) Subject: Handle let-else initializer edge case errors X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=29bc94ff0de00d79aa10c47603701592e1d3e340;p=rust.git Handle let-else initializer edge case errors --- diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 90786520fe8..6ea3db6d303 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/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, + } + } +} diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index a289b379fc8..f04ac8dd942 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/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 @@ fn from(ctx: UnusedDelimsCtx) -> &'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 @@ fn check_unused_delims_expr( right_pos: Option, ); - 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 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { 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 @@ fn check_unused_delims_expr( ) { 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 @@ fn check_unused_delims_expr( // 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) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index d083c379f77..068bd36af55 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -298,6 +298,8 @@ fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { 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 @@ fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { 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>> { let eq_consumed = match self.token.kind {