Fix unused_braces linting

Author: ChAoSUnItY

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,

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<bool>,
+}
+
+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) {
         <Self as UnusedDelimLint>::check_expr(self, cx, e);
 
+        if <Self as UnusedDelimLint>::impacts_followed_by_block(e) {
+            self.followed_by_block.push(<Self as UnusedDelimLint>::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 <Self as UnusedDelimLint>::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(

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
+}

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
+}

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
+