skip necessary paren expr during unused_paren check

Author: bvanjoi

compiler/rustc_ast/src/mut_visit.rs

@@ -807,7 +807,7 @@ fn visit_lazy_tts_opt_mut<T: MutVisitor>(vis: &mut T, lazy_tts: Option<&mut Lazy
     }
 }
 
-fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
+pub fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
     visit_lazy_tts_opt_mut(vis, lazy_tts.as_mut());
 }
 

compiler/rustc_expand/src/base.rs

@@ -1073,6 +1073,8 @@ pub trait ResolverExpand {
         trait_def_id: DefId,
         impl_def_id: LocalDefId,
     ) -> Result<Vec<(Ident, Option<Ident>)>, Indeterminate>;
+
+    fn add_necessary_parens(&mut self, node_span: Span);
 }
 
 pub trait LintStoreExpand {

compiler/rustc_expand/src/expand.rs

@@ -1,4 +1,4 @@
-use std::ops::Deref;
+use std::ops::{Deref, DerefMut};
 use std::path::PathBuf;
 use std::rc::Rc;
 use std::{iter, mem};
@@ -594,6 +594,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 cx: self.cx,
                 invocations: Vec::new(),
                 monotonic: self.monotonic,
+                expr_under_let_init_else: false,
             };
             fragment.mut_visit_with(&mut collector);
             fragment.add_placeholders(extra_placeholders);
@@ -1771,6 +1772,7 @@ struct InvocationCollector<'a, 'b> {
     cx: &'a mut ExtCtxt<'b>,
     invocations: Vec<(Invocation, Option<Lrc<SyntaxExtension>>)>,
     monotonic: bool,
+    expr_under_let_init_else: bool,
 }
 
 impl<'a, 'b> InvocationCollector<'a, 'b> {
@@ -2167,6 +2169,35 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         self.flat_map_node(node)
     }
 
+    fn visit_local(&mut self, local: &mut P<rustc_ast::Local>) {
+        let ast::Local { id, pat, ty, kind, span, colon_sp, attrs, tokens } = local.deref_mut();
+        self.visit_id(id);
+        for attr in attrs.iter_mut() {
+            self.visit_attribute(attr);
+        }
+        self.visit_pat(pat);
+        if let Some(ty) = ty {
+            self.visit_ty(ty);
+        }
+        match kind {
+            ast::LocalKind::Decl => {}
+            ast::LocalKind::Init(init) => {
+                self.visit_expr(init);
+            }
+            ast::LocalKind::InitElse(init, els) => {
+                self.expr_under_let_init_else = true;
+                self.visit_expr(init);
+                self.expr_under_let_init_else = false;
+                self.visit_block(els);
+            }
+        }
+        visit_lazy_tts(self, tokens);
+        if let Some(sp) = colon_sp {
+            self.visit_span(sp);
+        }
+        self.visit_span(span);
+    }
+
     fn visit_crate(&mut self, node: &mut ast::Crate) {
         self.visit_node(node)
     }
@@ -2180,6 +2211,13 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
     }
 
     fn visit_expr(&mut self, node: &mut P<ast::Expr>) {
+        if self.expr_under_let_init_else
+            && let ast::ExprKind::Paren(paren) = &node.kind
+            && let ast::ExprKind::MacCall(mac) = &paren.kind
+            && mac.args.delim == Delimiter::Brace
+        {
+            self.cx.resolver.add_necessary_parens(node.span);
+        }
         // FIXME: Feature gating is performed inconsistently between `Expr` and `OptExpr`.
         if let Some(attr) = node.attrs.first() {
             self.cfg().maybe_emit_expr_attr_err(attr);

compiler/rustc_interface/src/passes.rs

@@ -81,6 +81,7 @@ fn pre_expansion_lint<'a>(
                 lint_store,
                 registered_tools,
                 None,
+                None,
                 rustc_lint::BuiltinCombinedPreExpansionLintPass::new(),
                 check_node,
             );
@@ -315,6 +316,7 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) {
         lint_store,
         tcx.registered_tools(()),
         Some(lint_buffer),
+        Some(&resolver.necessary_parens),
         rustc_lint::BuiltinCombinedEarlyLintPass::new(),
         (&**krate, &*krate.attrs),
     )

compiler/rustc_lint/src/context.rs

@@ -6,7 +6,7 @@
 use std::cell::Cell;
 use std::{iter, slice};
 
-use rustc_data_structures::fx::FxIndexMap;
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_data_structures::sync;
 use rustc_data_structures::unord::UnordMap;
 use rustc_errors::{Diag, LintDiagnostic, MultiSpan};
@@ -509,6 +509,7 @@ pub struct LateContext<'tcx> {
 pub struct EarlyContext<'a> {
     pub builder: LintLevelsBuilder<'a, crate::levels::TopDown>,
     pub buffered: LintBuffer,
+    pub necessary_parens: Option<&'a FxHashSet<Span>>,
 }
 
 impl EarlyContext<'_> {
@@ -636,6 +637,7 @@ impl<'a> EarlyContext<'a> {
         lint_store: &'a LintStore,
         registered_tools: &'a RegisteredTools,
         buffered: LintBuffer,
+        necessary_parens: Option<&'a FxHashSet<Span>>,
     ) -> EarlyContext<'a> {
         EarlyContext {
             builder: LintLevelsBuilder::new(
@@ -646,6 +648,7 @@ impl<'a> EarlyContext<'a> {
                 registered_tools,
             ),
             buffered,
+            necessary_parens,
         }
     }
 }

compiler/rustc_lint/src/early.rs

@@ -7,6 +7,7 @@
 use rustc_ast::ptr::P;
 use rustc_ast::visit::{self as ast_visit, Visitor, walk_list};
 use rustc_ast::{self as ast, HasAttrs};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_feature::Features;
 use rustc_middle::ty::RegisteredTools;
@@ -364,6 +365,7 @@ pub fn check_ast_node<'a>(
     lint_store: &LintStore,
     registered_tools: &RegisteredTools,
     lint_buffer: Option<LintBuffer>,
+    necessary_parens: Option<&'a FxHashSet<Span>>,
     builtin_lints: impl EarlyLintPass + 'static,
     check_node: impl EarlyCheckNode<'a>,
 ) {
@@ -374,6 +376,7 @@ pub fn check_ast_node<'a>(
         lint_store,
         registered_tools,
         lint_buffer.unwrap_or_default(),
+        necessary_parens,
     );
 
     // Note: `passes` is often empty. In that case, it's faster to run

compiler/rustc_lint/src/unused.rs

@@ -1060,7 +1060,8 @@ impl UnusedDelimLint for UnusedParens {
     ) {
         match value.kind {
             ast::ExprKind::Paren(ref inner) => {
-                if !Self::is_expr_delims_necessary(inner, ctx, followed_by_block)
+                if cx.necessary_parens.map_or(true, |set| !set.contains(&value.span))
+                    && !Self::is_expr_delims_necessary(inner, ctx, followed_by_block)
                     && value.attrs.is_empty()
                     && !value.span.from_expansion()
                     && (ctx != UnusedDelimsCtx::LetScrutineeExpr

compiler/rustc_middle/src/ty/mod.rs

@@ -213,6 +213,7 @@ pub struct ResolverAstLowering {
 
     /// Information about functions signatures for delegation items expansion
     pub delegation_fn_sigs: LocalDefIdMap<DelegationFnSig>,
+    pub necessary_parens: FxHashSet<Span>,
 }
 
 #[derive(Debug)]

compiler/rustc_resolve/src/lib.rs

@@ -1198,6 +1198,12 @@ pub struct Resolver<'ra, 'tcx> {
     /// This is the `Span` where an `extern crate foo;` suggestion would be inserted, if `foo`
     /// could be a crate that wasn't imported. For diagnostics use only.
     current_crate_outer_attr_insert_span: Span,
+
+    /// It is used for the `unused_paren` lint because the lint checks after expansion.
+    /// This helps us record nodes that are wrapped by necessary parentheses.
+    ///
+    /// The reason for using `Span` is that the id has not been generated yet.
+    pub necessary_parens: FxHashSet<Span>,
 }
 
 /// This provides memory for the rest of the crate. The `'ra` lifetime that is
@@ -1543,6 +1549,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             impl_unexpanded_invocations: Default::default(),
             impl_binding_keys: Default::default(),
             current_crate_outer_attr_insert_span,
+            necessary_parens: Default::default(),
         };
 
         let root_parent_scope = ParentScope::module(graph_root, &resolver);
@@ -1656,6 +1663,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             lifetime_elision_allowed: self.lifetime_elision_allowed,
             lint_buffer: Steal::new(self.lint_buffer),
             delegation_fn_sigs: self.delegation_fn_sigs,
+            necessary_parens: self.necessary_parens,
         };
         ResolverOutputs { global_ctxt, ast_lowering }
     }

compiler/rustc_resolve/src/macros.rs

@@ -534,6 +534,10 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
         });
         Ok(idents)
     }
+
+    fn add_necessary_parens(&mut self, span: Span) {
+        self.necessary_parens.insert(span);
+    }
 }
 
 impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

tests/ui/lint/unused-parens-for-macro-call-with-brace.fixed

@@ -0,0 +1,24 @@
+//@ run-rustfix
+
+#![deny(unused_parens)]
+
+fn main() {
+    macro_rules! x {
+        () => { None::<i32> };
+    }
+
+    let Some(_) = (x!{}) else { return }; // no error
+    let Some(_) = (x!{}) else { return };
+    //~^ ERROR: unnecessary parentheses around assigned value
+    let Some(_) = (x!{}) else { return };
+    //~^ ERROR: unnecessary parentheses around pattern
+    let _ = x!{};
+    let _ = x!{};
+    //~^ ERROR: unnecessary parentheses around assigned value
+    if let Some(_) = x!{} {};
+    if let Some(_) = x!{} {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+    while let Some(_) = x!{} {};
+    while let Some(_) = x!{} {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+}

tests/ui/lint/unused-parens-for-macro-call-with-brace.rs

@@ -0,0 +1,24 @@
+//@ run-rustfix
+
+#![deny(unused_parens)]
+
+fn main() {
+    macro_rules! x {
+        () => { None::<i32> };
+    }
+
+    let Some(_) = (x!{}) else { return }; // no error
+    let Some(_) = ((x!{})) else { return };
+    //~^ ERROR: unnecessary parentheses around assigned value
+    let Some((_)) = (x!{}) else { return };
+    //~^ ERROR: unnecessary parentheses around pattern
+    let _ = x!{};
+    let _ = (x!{});
+    //~^ ERROR: unnecessary parentheses around assigned value
+    if let Some(_) = x!{} {};
+    if let Some(_) = (x!{}) {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+    while let Some(_) = x!{} {};
+    while let Some(_) = (x!{}) {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+}

tests/ui/lint/unused-parens-for-macro-call-with-brace.stderr

@@ -0,0 +1,67 @@
+error: unnecessary parentheses around assigned value
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:11:19
+   |
+LL |     let Some(_) = ((x!{})) else { return };
+   |                   ^      ^
+   |
+note: the lint level is defined here
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:3:9
+   |
+LL | #![deny(unused_parens)]
+   |         ^^^^^^^^^^^^^
+help: remove these parentheses
+   |
+LL -     let Some(_) = ((x!{})) else { return };
+LL +     let Some(_) = (x!{}) else { return };
+   |
+
+error: unnecessary parentheses around pattern
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:13:14
+   |
+LL |     let Some((_)) = (x!{}) else { return };
+   |              ^ ^
+   |
+help: remove these parentheses
+   |
+LL -     let Some((_)) = (x!{}) else { return };
+LL +     let Some(_) = (x!{}) else { return };
+   |
+
+error: unnecessary parentheses around assigned value
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:16:13
+   |
+LL |     let _ = (x!{});
+   |             ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     let _ = (x!{});
+LL +     let _ = x!{};
+   |
+
+error: unnecessary parentheses around `let` scrutinee expression
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:19:22
+   |
+LL |     if let Some(_) = (x!{}) {};
+   |                      ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     if let Some(_) = (x!{}) {};
+LL +     if let Some(_) = x!{} {};
+   |
+
+error: unnecessary parentheses around `let` scrutinee expression
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:22:25
+   |
+LL |     while let Some(_) = (x!{}) {};
+   |                         ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     while let Some(_) = (x!{}) {};
+LL +     while let Some(_) = x!{} {};
+   |
+
+error: aborting due to 5 previous errors
+