Rollup merge of #142237 - benschulz:unused-parens-fn, r=fee1-dead

Detect more cases of unused_parens around types

With this change, more unused parentheses around bounds and types nested within bounds are detected.

1 files changed, 118 insertions, 10 deletions

diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs
index a206f71e153..0627f70507c 100644
--- a/compiler/rustc_lint/src/unused.rs
+++ b/compiler/rustc_lint/src/unused.rs
@@ -3,6 +3,7 @@ use std::iter;
 use rustc_ast::util::{classify, parser};
 use rustc_ast::{self as ast, ExprKind, HasAttrs as _, StmtKind};
 use rustc_attr_data_structures::{AttributeKind, find_attr};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{MultiSpan, pluralize};
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
@@ -10,6 +11,7 @@ use rustc_hir::{self as hir, LangItem};
 use rustc_infer::traits::util::elaborate;
 use rustc_middle::ty::{self, Ty, adjustment};
 use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass};
+use rustc_span::edition::Edition::Edition2015;
 use rustc_span::{BytePos, Span, Symbol, kw, sym};
 use tracing::instrument;
 
@@ -1034,6 +1036,31 @@ pub(crate) struct UnusedParens {
     /// `1 as (i32) < 2` parses to ExprKind::Lt
     /// `1 as i32 < 2` parses to i32::<2[missing angle bracket]
     parens_in_cast_in_lt: Vec<ast::NodeId>,
+    /// Ty nodes in this map are in TypeNoBounds position. Any bounds they
+    /// contain may be ambiguous w/r/t trailing `+` operators.
+    in_no_bounds_pos: FxHashMap<ast::NodeId, NoBoundsException>,
+}
+
+/// Whether parentheses may be omitted from a type without resulting in ambiguity.
+///
+/// ```
+/// type Example = Box<dyn Fn() -> &'static (dyn Send) + Sync>;
+/// ```
+///
+/// Here, `&'static (dyn Send) + Sync` is a `TypeNoBounds`. As such, it may not directly
+/// contain `ImplTraitType` or `TraitObjectType` which is why `(dyn Send)` is parenthesized.
+/// However, an exception is made for `ImplTraitTypeOneBound` and `TraitObjectTypeOneBound`.
+/// The following is accepted because there is no `+`.
+///
+/// ```
+/// type Example = Box<dyn Fn() -> &'static dyn Send>;
+/// ```
+enum NoBoundsException {
+    /// The type must be parenthesized.
+    None,
+    /// The type is the last bound of the containing type expression. If it has exactly one bound,
+    /// parentheses around the type are unnecessary.
+    OneBound,
 }
 
 impl_lint_pass!(UnusedParens => [UNUSED_PARENS]);
@@ -1277,23 +1304,100 @@ impl EarlyLintPass for UnusedParens {
                 );
             }
             ast::TyKind::Paren(r) => {
-                match &r.kind {
-                    ast::TyKind::TraitObject(..) => {}
-                    ast::TyKind::BareFn(b)
-                        if self.with_self_ty_parens && b.generic_params.len() > 0 => {}
-                    ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {}
-                    _ => {
-                        let spans = if !ty.span.from_expansion() {
+                let unused_parens = match &r.kind {
+                    ast::TyKind::ImplTrait(_, bounds) | ast::TyKind::TraitObject(bounds, _) => {
+                        match self.in_no_bounds_pos.get(&ty.id) {
+                            Some(NoBoundsException::None) => false,
+                            Some(NoBoundsException::OneBound) => bounds.len() <= 1,
+                            None => true,
+                        }
+                    }
+                    ast::TyKind::BareFn(b) => {
+                        !self.with_self_ty_parens || b.generic_params.is_empty()
+                    }
+                    _ => true,
+                };
+
+                if unused_parens {
+                    let spans = (!ty.span.from_expansion())
+                        .then(|| {
                             r.span
                                 .find_ancestor_inside(ty.span)
                                 .map(|r| (ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi())))
+                        })
+                        .flatten();
+
+                    self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false);
+                }
+
+                self.with_self_ty_parens = false;
+            }
+            ast::TyKind::Ref(_, mut_ty) | ast::TyKind::Ptr(mut_ty) => {
+                self.in_no_bounds_pos.insert(mut_ty.ty.id, NoBoundsException::OneBound);
+            }
+            ast::TyKind::TraitObject(bounds, _) | ast::TyKind::ImplTrait(_, bounds) => {
+                for i in 0..bounds.len() {
+                    let is_last = i == bounds.len() - 1;
+
+                    if let ast::GenericBound::Trait(poly_trait_ref) = &bounds[i] {
+                        let fn_with_explicit_ret_ty = if let [.., segment] =
+                            &*poly_trait_ref.trait_ref.path.segments
+                            && let Some(args) = segment.args.as_ref()
+                            && let ast::GenericArgs::Parenthesized(paren_args) = &**args
+                            && let ast::FnRetTy::Ty(ret_ty) = &paren_args.output
+                        {
+                            self.in_no_bounds_pos.insert(
+                                ret_ty.id,
+                                if is_last {
+                                    NoBoundsException::OneBound
+                                } else {
+                                    NoBoundsException::None
+                                },
+                            );
+
+                            true
                         } else {
-                            None
+                            false
                         };
-                        self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false);
+
+                        // In edition 2015, dyn is a contextual keyword and `dyn::foo::Bar` is
+                        // parsed as a path, so parens are necessary to disambiguate. See
+                        //  - tests/ui/lint/unused/unused-parens-trait-obj-e2015.rs and
+                        //  - https://doc.rust-lang.org/reference/types/trait-object.html#r-type.trait-object.syntax-edition2018
+                        let dyn2015_exception = cx.sess().psess.edition == Edition2015
+                            && matches!(ty.kind, ast::TyKind::TraitObject(..))
+                            && i == 0
+                            && poly_trait_ref
+                                .trait_ref
+                                .path
+                                .segments
+                                .first()
+                                .map(|s| s.ident.name == kw::PathRoot)
+                                .unwrap_or(false);
+
+                        if let ast::Parens::Yes = poly_trait_ref.parens
+                            && (is_last || !fn_with_explicit_ret_ty)
+                            && !dyn2015_exception
+                        {
+                            let s = poly_trait_ref.span;
+                            let spans = (!s.from_expansion()).then(|| {
+                                (
+                                    s.with_hi(s.lo() + rustc_span::BytePos(1)),
+                                    s.with_lo(s.hi() - rustc_span::BytePos(1)),
+                                )
+                            });
+
+                            self.emit_unused_delims(
+                                cx,
+                                poly_trait_ref.span,
+                                spans,
+                                "type",
+                                (false, false),
+                                false,
+                            );
+                        }
                     }
                 }
-                self.with_self_ty_parens = false;
             }
             _ => {}
         }
@@ -1303,6 +1407,10 @@ impl EarlyLintPass for UnusedParens {
         <Self as UnusedDelimLint>::check_item(self, cx, item)
     }
 
+    fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Item) {
+        self.in_no_bounds_pos.clear();
+    }
+
     fn enter_where_predicate(&mut self, _: &EarlyContext<'_>, pred: &ast::WherePredicate) {
         use rustc_ast::{WhereBoundPredicate, WherePredicateKind};
         if let WherePredicateKind::BoundPredicate(WhereBoundPredicate {