diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 0a52e42e44287..dee1cad2f867a 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -165,7 +165,7 @@ early_lint_methods!( pub BuiltinCombinedEarlyLintPass, [ UnusedParens: UnusedParens::default(), - UnusedBraces: UnusedBraces, + UnusedBraces: UnusedBraces::default(), UnusedImportBraces: UnusedImportBraces, UnsafeCode: UnsafeCode, SpecialModuleName: SpecialModuleName, diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 1620f425794f2..1b6b7441911be 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1340,7 +1340,33 @@ declare_lint! { "unnecessary braces around an expression" } -declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]); +#[derive(Default)] +pub(crate) struct UnusedBraces { + parent_followed_by_block: Vec, +} + +impl_lint_pass!(UnusedBraces => [UNUSED_BRACES]); + +impl UnusedBraces { + #[inline] + fn should_mark_block(e: &ast::Expr) -> bool { + return match e.kind { + ExprKind::Block(..) => true, + _ => Self::is_followed_by_block(e), + }; + } + + #[inline] + fn is_followed_by_block(e: &ast::Expr) -> bool { + return match e.kind { + ExprKind::If(ref cond, ..) if !matches!(cond.kind, ExprKind::Let(..)) => true, + ExprKind::While(ref cond, ..) if !matches!(cond.kind, ExprKind::Let(..)) => true, + ExprKind::ForLoop { .. } => true, + ExprKind::Match(_, _, ast::MatchKind::Prefix) => true, + _ => false, + }; + } +} impl UnusedDelimLint for UnusedBraces { const DELIM_STR: &'static str = "braces"; @@ -1370,6 +1396,11 @@ impl UnusedDelimLint for UnusedBraces { // - the block does not have a label // - the block is not `unsafe` // - the block contains exactly one expression (do not lint `{ expr; }`) + // - however, this does not lint if block is immediately followed by parent + // expression's block, e.g. `if` and `match`, which may cause false positive. + // ``` + // if return { return } {} else {} + // ``` // - `followed_by_block` is true and the internal expr may contain a `{` // - the block is not multiline (do not lint multiline match arms) // ``` @@ -1392,6 +1423,11 @@ impl UnusedDelimLint for UnusedBraces { if let [stmt] = inner.stmts.as_slice() { if let ast::StmtKind::Expr(ref expr) = stmt.kind { if !Self::is_expr_delims_necessary(expr, ctx, followed_by_block) + && !Self::is_expr_delims_necessary( + expr, + ctx, + self.parent_followed_by_block.last().copied().unwrap_or(false), + ) && (ctx != UnusedDelimsCtx::AnonConst || (matches!(expr.kind, ast::ExprKind::Lit(_)) && !expr.span.from_expansion())) @@ -1430,6 +1466,10 @@ impl EarlyLintPass for UnusedBraces { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { ::check_expr(self, cx, e); + if Self::should_mark_block(e) { + self.parent_followed_by_block.push(Self::is_followed_by_block(e)); + } + if let ExprKind::Repeat(_, ref anon_const) = e.kind { self.check_unused_delims_expr( cx, @@ -1443,6 +1483,20 @@ impl EarlyLintPass for UnusedBraces { } } + fn check_expr_post(&mut self, _cx: &EarlyContext<'_>, e: &ast::Expr) { + if Self::should_mark_block(e) { + let followed_by_block = self + .parent_followed_by_block + .pop() + .expect("check_expr and check_expr_post must balance"); + assert_eq!( + followed_by_block, + Self::is_followed_by_block(e), + "check_expr, check_ty, and check_expr_post are called, in that order, by the visitor" + ); + } + } + fn check_generic_arg(&mut self, cx: &EarlyContext<'_>, arg: &ast::GenericArg) { if let ast::GenericArg::Const(ct) = arg { self.check_unused_delims_expr( diff --git a/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed new file mode 100644 index 0000000000000..cc39b23f1e11a --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed @@ -0,0 +1,13 @@ +//@ run-rustfix +//@ check-pass +#[allow(unreachable_code)] +#[warn(unused_braces)] + +fn main() { + return return; //~ WARN: unnecessary braces + if return { return } { return } else { return } + match return { return } { + _ => { return } + } + while return { return } {} +} diff --git a/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.rs b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.rs new file mode 100644 index 0000000000000..e0a123499b6a8 --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.rs @@ -0,0 +1,13 @@ +//@ run-rustfix +//@ check-pass +#[allow(unreachable_code)] +#[warn(unused_braces)] + +fn main() { + return { return }; //~ WARN: unnecessary braces + if return { return } { return } else { return } + match return { return } { + _ => { return } + } + while return { return } {} +} diff --git a/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.stderr b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.stderr new file mode 100644 index 0000000000000..61d2a2980da1e --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.stderr @@ -0,0 +1,19 @@ +warning: unnecessary braces around `return` value + --> $DIR/unused-braces-respect-parent-block-issue-141783.rs:7:12 + | +LL | return { return }; + | ^^ ^^ + | +note: the lint level is defined here + --> $DIR/unused-braces-respect-parent-block-issue-141783.rs:4:8 + | +LL | #[warn(unused_braces)] + | ^^^^^^^^^^^^^ +help: remove these braces + | +LL - return { return }; +LL + return return; + | + +warning: 1 warning emitted +