diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 9bb53fea54a18..8a54cb0d15b1e 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -166,7 +166,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 874c435402919..d4b3bcec2f80c 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -656,6 +656,25 @@ trait UnusedDelimLint { is_kw: bool, ); + #[inline] + fn impacts_followed_by_block(e: &ast::Expr) -> bool { + return match e.kind { + ExprKind::Block(..) | ExprKind::Call(..) | ExprKind::MethodCall(..) => 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, + }; + } + fn is_expr_delims_necessary( inner: &ast::Expr, ctx: UnusedDelimsCtx, @@ -1475,7 +1494,15 @@ declare_lint! { "unnecessary braces around an expression" } -declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]); +#[derive(Default)] +pub(crate) struct UnusedBraces { + // Used for tracking parent expressions that would immediately followed + // by block. Storing false here indicates that expression itself is Block + // expression. This is meant for to prevent report false positive cases. + followed_by_block: Vec, +} + +impl_lint_pass!(UnusedBraces => [UNUSED_BRACES]); impl UnusedDelimLint for UnusedBraces { const DELIM_STR: &'static str = "braces"; @@ -1505,6 +1532,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) // ``` @@ -1524,9 +1556,18 @@ impl UnusedDelimLint for UnusedBraces { // let _: A<{produces_literal!()}>; // ``` // FIXME(const_generics): handle paths when #67075 is fixed. + // if ctx == UnusedDelimsCtx::FunctionArg { + // println!("{:?}", self.followed_by_block.last().copied().unwrap_or(false)) + // } if let [stmt] = inner.stmts.as_slice() && let ast::StmtKind::Expr(ref expr) = stmt.kind && !Self::is_expr_delims_necessary(expr, ctx, followed_by_block) + && ((ctx == UnusedDelimsCtx::FunctionArg || ctx == UnusedDelimsCtx::MethodArg) + || !Self::is_expr_delims_necessary( + expr, + ctx, + self.followed_by_block.last().copied().unwrap_or(false), + )) && (ctx != UnusedDelimsCtx::AnonConst || (matches!(expr.kind, ast::ExprKind::Lit(_)) && !expr.span.from_expansion())) @@ -1564,6 +1605,10 @@ impl EarlyLintPass for UnusedBraces { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { ::check_expr(self, cx, e); + if ::impacts_followed_by_block(e) { + self.followed_by_block.push(::is_followed_by_block(e)); + } + if let ExprKind::Repeat(_, ref anon_const) = e.kind { self.check_unused_delims_expr( cx, @@ -1577,6 +1622,18 @@ impl EarlyLintPass for UnusedBraces { } } + fn check_expr_post(&mut self, _cx: &EarlyContext<'_>, e: &ast::Expr) { + if ::impacts_followed_by_block(e) { + let followed_by_block = + self.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..0243e5473ab73 --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.fixed @@ -0,0 +1,20 @@ +//@ run-rustfix +//@ check-pass +#[allow(unreachable_code)] +#[allow(unused)] +#[warn(unused_braces)] +#[warn(unused_parens)] + +fn main() { + return return; //~ WARN: unnecessary braces + if f(return) {} else {} //~ WARN: unnecessary braces + if return { return } { return } else { return } + match return { return } { + _ => { return } + } + while return { return } {} +} + +fn f(_a: ()) -> bool { + true +} 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..a38514ddd5853 --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.rs @@ -0,0 +1,20 @@ +//@ run-rustfix +//@ check-pass +#[allow(unreachable_code)] +#[allow(unused)] +#[warn(unused_braces)] +#[warn(unused_parens)] + +fn main() { + return { return }; //~ WARN: unnecessary braces + if f({ return }) {} else {} //~ WARN: unnecessary braces + if return { return } { return } else { return } + match return { return } { + _ => { return } + } + while return { return } {} +} + +fn f(_a: ()) -> bool { + true +} 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..87c4824fc14a0 --- /dev/null +++ b/tests/ui/lint/unused/unused-braces-respect-parent-block-issue-141783.stderr @@ -0,0 +1,31 @@ +warning: unnecessary braces around `return` value + --> $DIR/unused-braces-respect-parent-block-issue-141783.rs:9:12 + | +LL | return { return }; + | ^^ ^^ + | +note: the lint level is defined here + --> $DIR/unused-braces-respect-parent-block-issue-141783.rs:5:8 + | +LL | #[warn(unused_braces)] + | ^^^^^^^^^^^^^ +help: remove these braces + | +LL - return { return }; +LL + return return; + | + +warning: unnecessary braces around function argument + --> $DIR/unused-braces-respect-parent-block-issue-141783.rs:10:10 + | +LL | if f({ return }) {} else {} + | ^^ ^^ + | +help: remove these braces + | +LL - if f({ return }) {} else {} +LL + if f(return) {} else {} + | + +warning: 2 warnings emitted +