Detect more cases of unused_parens around types

Author: Benjamin Schulz

compiler/rustc_lint/src/unused.rs

@@ -3,13 +3,15 @@ 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;
 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;
 
@@ -1032,6 +1034,14 @@ 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>,
+}
+
+enum NoBoundsException {
+    None,
+    OneBound,
 }
 
 impl_lint_pass!(UnusedParens => [UNUSED_PARENS]);
@@ -1276,10 +1286,12 @@ impl EarlyLintPass for UnusedParens {
             }
             ast::TyKind::Paren(r) => {
                 match &r.kind {
-                    ast::TyKind::TraitObject(..) => {}
+                    ast::TyKind::ImplTrait(_, bounds) | ast::TyKind::TraitObject(bounds, _)
+                        if self.in_no_bounds_pos.get(&ty.id).is_some_and(|exception| {
+                            matches!(exception, NoBoundsException::None) || bounds.len() > 1
+                        }) => {}
                     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() {
                             r.span
@@ -1293,6 +1305,74 @@ impl EarlyLintPass for UnusedParens {
                 }
                 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 last = i == bounds.len() - 1;
+
+                    if let ast::GenericBound::Trait(poly_trait_ref) = &bounds[i] {
+                        let parenthesized = cx
+                            .sess()
+                            .source_map()
+                            .span_to_snippet(poly_trait_ref.span)
+                            .map(|snip| snip.starts_with('(') && snip.ends_with(')'))
+                            .unwrap_or(false);
+
+                        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 last {
+                                    NoBoundsException::OneBound
+                                } else {
+                                    NoBoundsException::None
+                                },
+                            );
+
+                            true
+                        } else {
+                            false
+                        };
+
+                        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 parenthesized && (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,
+                            );
+                        }
+                    }
+                }
+            }
             _ => {}
         }
     }
@@ -1301,6 +1381,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 {

library/core/src/error.rs

@@ -347,7 +347,7 @@ impl dyn Error {
     /// let b = B(Some(Box::new(A)));
     ///
     /// // let err : Box<Error> = b.into(); // or
-    /// let err = &b as &(dyn Error);
+    /// let err = &b as &dyn Error;
     ///
     /// let mut iter = err.sources();
     ///

tests/ui/lint/lint-unnecessary-parens.fixed

@@ -1,5 +1,6 @@
 //@ run-rustfix
 
+#![feature(impl_trait_in_fn_trait_return)]
 #![deny(unused_parens)]
 #![allow(while_true)] // for rustfix
 
@@ -16,11 +17,11 @@ fn bar(y: bool) -> X {
     return X { y }; //~ ERROR unnecessary parentheses around `return` value
 }
 
-pub fn unused_parens_around_return_type() -> u32 { //~ ERROR unnecessary parentheses around type
+pub fn around_return_type() -> u32 { //~ ERROR unnecessary parentheses around type
     panic!()
 }
 
-pub fn unused_parens_around_block_return() -> u32 {
+pub fn around_block_return() -> u32 {
     let _foo = {
         5 //~ ERROR unnecessary parentheses around block return value
     };
@@ -31,10 +32,90 @@ pub trait Trait {
     fn test(&self);
 }
 
-pub fn passes_unused_parens_lint() -> &'static (dyn Trait) {
+pub fn around_multi_bound_ref() -> &'static (dyn Trait + Send) {
     panic!()
 }
 
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_ref() -> &'static dyn Trait {
+    panic!()
+}
+
+pub fn around_multi_bound_ptr() -> *const (dyn Trait + Send) {
+    panic!()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_ptr() -> *const dyn Trait {
+    panic!()
+}
+
+pub fn around_multi_bound_dyn_fn_output() -> &'static dyn FnOnce() -> (impl Send + Sync) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_dyn_fn_output() -> &'static dyn FnOnce() -> impl Send {
+    &|| ()
+}
+
+pub fn around_dyn_fn_output_given_more_bounds() -> &'static (dyn FnOnce() -> (impl Send) + Sync) {
+    &|| ()
+}
+
+pub fn around_multi_bound_impl_fn_output() -> impl FnOnce() -> (impl Send + Sync) {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_impl_fn_output() -> impl FnOnce() -> impl Send {
+    || ()
+}
+
+pub fn around_impl_fn_output_given_more_bounds() -> impl FnOnce() -> (impl Send) + Sync {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_dyn_bound() -> &'static dyn FnOnce() {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_impl_trait_bound() -> impl FnOnce() {
+    || ()
+}
+
+// these parens aren't strictly required but they help disambiguate => no lint
+pub fn around_fn_bound_with_explicit_ret_ty() -> impl (Fn() -> ()) + Send {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_fn_bound_with_implicit_ret_ty() -> impl Fn() + Send {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_last_fn_bound_with_explicit_ret_ty() -> impl Send + Fn() -> () {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound1() -> &'static (dyn Send + Sync) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound2() -> &'static (dyn Send + Sync) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound3() -> &'static (dyn Send + ::std::marker::Sync) {
+    &|| ()
+}
+
 pub fn parens_with_keyword(e: &[()]) -> i32 {
     if true {} //~ ERROR unnecessary parentheses around `if`
     while true {} //~ ERROR unnecessary parentheses around `while`

tests/ui/lint/lint-unnecessary-parens.rs

@@ -1,5 +1,6 @@
 //@ run-rustfix
 
+#![feature(impl_trait_in_fn_trait_return)]
 #![deny(unused_parens)]
 #![allow(while_true)] // for rustfix
 
@@ -16,11 +17,11 @@ fn bar(y: bool) -> X {
     return (X { y }); //~ ERROR unnecessary parentheses around `return` value
 }
 
-pub fn unused_parens_around_return_type() -> (u32) { //~ ERROR unnecessary parentheses around type
+pub fn around_return_type() -> (u32) { //~ ERROR unnecessary parentheses around type
     panic!()
 }
 
-pub fn unused_parens_around_block_return() -> u32 {
+pub fn around_block_return() -> u32 {
     let _foo = {
         (5) //~ ERROR unnecessary parentheses around block return value
     };
@@ -31,10 +32,90 @@ pub trait Trait {
     fn test(&self);
 }
 
-pub fn passes_unused_parens_lint() -> &'static (dyn Trait) {
+pub fn around_multi_bound_ref() -> &'static (dyn Trait + Send) {
     panic!()
 }
 
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_ref() -> &'static (dyn Trait) {
+    panic!()
+}
+
+pub fn around_multi_bound_ptr() -> *const (dyn Trait + Send) {
+    panic!()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_ptr() -> *const (dyn Trait) {
+    panic!()
+}
+
+pub fn around_multi_bound_dyn_fn_output() -> &'static dyn FnOnce() -> (impl Send + Sync) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_dyn_fn_output() -> &'static dyn FnOnce() -> (impl Send) {
+    &|| ()
+}
+
+pub fn around_dyn_fn_output_given_more_bounds() -> &'static (dyn FnOnce() -> (impl Send) + Sync) {
+    &|| ()
+}
+
+pub fn around_multi_bound_impl_fn_output() -> impl FnOnce() -> (impl Send + Sync) {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_single_bound_impl_fn_output() -> impl FnOnce() -> (impl Send) {
+    || ()
+}
+
+pub fn around_impl_fn_output_given_more_bounds() -> impl FnOnce() -> (impl Send) + Sync {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_dyn_bound() -> &'static dyn (FnOnce()) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_impl_trait_bound() -> impl (FnOnce()) {
+    || ()
+}
+
+// these parens aren't strictly required but they help disambiguate => no lint
+pub fn around_fn_bound_with_explicit_ret_ty() -> impl (Fn() -> ()) + Send {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_fn_bound_with_implicit_ret_ty() -> impl (Fn()) + Send {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_last_fn_bound_with_explicit_ret_ty() -> impl Send + (Fn() -> ()) {
+    || ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound1() -> &'static (dyn (Send) + Sync) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound2() -> &'static (dyn Send + (Sync)) {
+    &|| ()
+}
+
+//~v ERROR unnecessary parentheses around type
+pub fn around_regular_bound3() -> &'static (dyn Send + (::std::marker::Sync)) {
+    &|| ()
+}
+
 pub fn parens_with_keyword(e: &[()]) -> i32 {
     if(true) {} //~ ERROR unnecessary parentheses around `if`
     while(true) {} //~ ERROR unnecessary parentheses around `while`