From 5fda6c130c395eece69bd392925085b82b388283 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Wed, 4 Jun 2025 05:07:10 +0800 Subject: [PATCH 1/2] Fix linting false positive when block used as val --- compiler/rustc_lint/src/lib.rs | 2 +- compiler/rustc_lint/src/unused.rs | 19 +++++++++++++++++-- .../unused/block-with-return-expr-as-value.rs | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/ui/lint/unused/block-with-return-expr-as-value.rs 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..b43070246a92f 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1340,7 +1340,12 @@ declare_lint! { "unnecessary braces around an expression" } -declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]); +#[derive(Default)] +pub(crate) struct UnusedBraces { + block_as_value: bool, +} + +impl_lint_pass!(UnusedBraces => [UNUSED_BRACES]); impl UnusedDelimLint for UnusedBraces { const DELIM_STR: &'static str = "braces"; @@ -1369,7 +1374,10 @@ 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; }`) + // - the block contains exactly one expression + // - do not lint `{ expr; }` + // - do not lint `{ return }` if block is used as value by other + // expressions, e.g. `return` and `match`, which may cause false positive. // - `followed_by_block` is true and the internal expr may contain a `{` // - the block is not multiline (do not lint multiline match arms) // ``` @@ -1399,6 +1407,7 @@ impl UnusedDelimLint for UnusedBraces { && value.attrs.is_empty() && !value.span.from_expansion() && !inner.span.from_expansion() + && !(self.block_as_value && matches!(expr.kind, ast::ExprKind::Ret(_))) { self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw) } @@ -1428,6 +1437,12 @@ impl EarlyLintPass for UnusedBraces { #[inline] fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { + use rustc_ast::ast::ExprKind::*; + self.block_as_value = match e.kind { + Ret(Some(ref expr)) | Match(ref expr, ..) if matches!(expr.kind, Block(..)) => true, + _ => false, + }; + ::check_expr(self, cx, e); if let ExprKind::Repeat(_, ref anon_const) = e.kind { diff --git a/tests/ui/lint/unused/block-with-return-expr-as-value.rs b/tests/ui/lint/unused/block-with-return-expr-as-value.rs new file mode 100644 index 0000000000000..accd7f62f5758 --- /dev/null +++ b/tests/ui/lint/unused/block-with-return-expr-as-value.rs @@ -0,0 +1,16 @@ +//@ check-pass +#[allow(unreachable_code)] +#[warn(unused_braces)] + +fn main() { + return { return }; + if return { return } { return } else { return } + match return { return } { + _ => { return } + } + return { return () }; + if return { return () } { return () } else { return () } + match return { return () } { + _ => { return () } + } +} From ae418c4a9508c657496e48dc55fcdcde4adc7ffe Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Thu, 5 Jun 2025 19:50:20 +0800 Subject: [PATCH 2/2] Respect parent expression block when linting --- compiler/rustc_lint/src/unused.rs | 63 +++++++++++++++---- ...s-respect-parent-block-issue-141783.fixed} | 9 +-- ...races-respect-parent-block-issue-141783.rs | 13 ++++ ...s-respect-parent-block-issue-141783.stderr | 19 ++++++ 4 files changed, 86 insertions(+), 18 deletions(-) rename tests/ui/lint/unused/{block-with-return-expr-as-value.rs => unused-braces-respect-parent-block-issue-141783.fixed} (52%) create mode 100644 tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.rs create mode 100644 tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.stderr diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index b43070246a92f..1b6b7441911be 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1342,11 +1342,32 @@ declare_lint! { #[derive(Default)] pub(crate) struct UnusedBraces { - block_as_value: bool, + 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"; @@ -1374,10 +1395,12 @@ 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; }` - // - do not lint `{ return }` if block is used as value by other - // expressions, e.g. `return` and `match`, which may cause false positive. + // - 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) // ``` @@ -1400,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())) @@ -1407,7 +1435,6 @@ impl UnusedDelimLint for UnusedBraces { && value.attrs.is_empty() && !value.span.from_expansion() && !inner.span.from_expansion() - && !(self.block_as_value && matches!(expr.kind, ast::ExprKind::Ret(_))) { self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw) } @@ -1437,14 +1464,12 @@ impl EarlyLintPass for UnusedBraces { #[inline] fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { - use rustc_ast::ast::ExprKind::*; - self.block_as_value = match e.kind { - Ret(Some(ref expr)) | Match(ref expr, ..) if matches!(expr.kind, Block(..)) => true, - _ => false, - }; - ::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, @@ -1458,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/block-with-return-expr-as-value.rs b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed similarity index 52% rename from tests/ui/lint/unused/block-with-return-expr-as-value.rs rename to tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed index accd7f62f5758..cc39b23f1e11a 100644 --- a/tests/ui/lint/unused/block-with-return-expr-as-value.rs +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed @@ -1,16 +1,13 @@ +//@ run-rustfix //@ check-pass #[allow(unreachable_code)] #[warn(unused_braces)] fn main() { - return { return }; + return return; //~ WARN: unnecessary braces if return { return } { return } else { return } match return { return } { _ => { return } } - return { return () }; - 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 +