redundant_allocation: check Rc and Arc in same block

rust-lang · Jun 7, 2021 · ad71930 · ad71930
1 parent 29d917a
commit ad71930
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 112 deletions.
diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs
@@ -177,8 +177,8 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// **What it does:** Checks for use of redundant allocations anywhere in the code.
     ///
-    /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Box<T>>`, Arc<&T>`, `Arc<Arc<T>>`,
-    /// `Arc<Box<T>>`, `Box<&T>` add an unnecessary level of indirection.
+    /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, Arc<&T>`, `Arc<Rc<T>>`,
+    /// `Arc<Arc<T>>`, `Arc<Box<T>>`, `Box<&T>` add an unnecessary level of indirection.
     ///
     /// **Known problems:** None.
     ///

diff --git a/clippy_lints/src/types/redundant_allocation.rs b/clippy_lints/src/types/redundant_allocation.rs
@@ -25,71 +25,40 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_
         }
     }
 
-    if cx.tcx.is_diagnostic_item(sym::Rc, def_id) {
-        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Rc) {
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                REDUNDANT_ALLOCATION,
-                hir_ty.span,
-                "usage of `Rc<Rc<T>>`",
-                "try",
-                snippet_with_applicability(cx, ty.span, "..", &mut applicability).to_string(),
-                applicability,
-            );
-            return true;
-        } else if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) {
-            let qpath = match &ty.kind {
-                TyKind::Path(qpath) => qpath,
-                _ => return false,
-            };
-            let inner_span = match get_qpath_generic_tys(qpath).next() {
-                Some(ty) => ty.span,
-                None => return false,
-            };
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                REDUNDANT_ALLOCATION,
-                hir_ty.span,
-                "usage of `Rc<Box<T>>`",
-                "try",
-                format!(
-                    "Rc<{}>",
-                    snippet_with_applicability(cx, inner_span, "..", &mut applicability)
-                ),
-                applicability,
-            );
-            return true;
-        }
-        return utils::match_borrows_parameter(cx, qpath).map_or(false, |span| {
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                REDUNDANT_ALLOCATION,
-                hir_ty.span,
-                "usage of `Rc<&T>`",
-                "try",
-                snippet_with_applicability(cx, span, "..", &mut applicability).to_string(),
-                applicability,
-            );
-            true
-        });
-    }
+    let sym_outer = if cx.tcx.is_diagnostic_item(sym::Rc, def_id) {
+        Some(sym::Rc)
+    } else if cx.tcx.is_diagnostic_item(sym::Arc, def_id) {
+        Some(sym::Arc)
+    } else {
+        None
+    };
 
-    if cx.tcx.is_diagnostic_item(sym::Arc, def_id) {
-        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Arc) {
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                REDUNDANT_ALLOCATION,
-                hir_ty.span,
-                "usage of `Arc<Arc<T>>`",
-                "try",
-                snippet_with_applicability(cx, ty.span, "..", &mut applicability).to_string(),
-                applicability,
-            );
-            return true;
+    if let Some(sym_outer) = sym_outer {
+        let ty = is_ty_param_diagnostic_item(cx, qpath, sym::Rc)
+            .map_or_else(|| is_ty_param_diagnostic_item(cx, qpath, sym::Arc), |ty| Some(ty));
+
+        if let Some(ty) = ty {
+            if let TyKind::Path(ref qpath_inner) = ty.kind {
+                let inner_span = match get_qpath_generic_tys(qpath_inner).next() {
+                    Some(ty) => ty.span,
+                    None => return false,
+                };
+                let mut applicability = Applicability::MachineApplicable;
+                span_lint_and_sugg(
+                    cx,
+                    REDUNDANT_ALLOCATION,
+                    hir_ty.span,
+                    &format!("usage of `{}<Rc<T>>`", sym_outer),
+                    "try",
+                    format!(
+                        "{}<{}>",
+                        sym_outer,
+                        snippet_with_applicability(cx, inner_span, "..", &mut applicability)
+                    ),
+                    applicability,
+                );
+                return true;
+            }
         } else if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) {
             let qpath = match &ty.kind {
                 TyKind::Path(qpath) => qpath,
@@ -104,10 +73,11 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_
                 cx,
                 REDUNDANT_ALLOCATION,
                 hir_ty.span,
-                "usage of `Arc<Box<T>>`",
+                &format!("usage of `{}<Box<T>>`", sym_outer),
                 "try",
                 format!(
-                    "Arc<{}>",
+                    "{}<{}>",
+                    sym_outer,
                     snippet_with_applicability(cx, inner_span, "..", &mut applicability)
                 ),
                 applicability,
@@ -120,13 +90,14 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_
                 cx,
                 REDUNDANT_ALLOCATION,
                 hir_ty.span,
-                "usage of `Arc<&T>`",
+                &format!("usage of `{}<&T>`", sym_outer),
                 "try",
                 snippet_with_applicability(cx, span, "..", &mut applicability).to_string(),
                 applicability,
             );
             true
         });
     }
+
     false
 }
diff --git a/tests/ui/redundant_allocation.fixed b/tests/ui/redundant_allocation.fixed
@@ -32,36 +32,44 @@ pub fn test4_neg(foo: Rc<SubT<&usize>>) {}
 
 pub fn test5(a: Rc<bool>) {}
 
-// Rc<Box<T>>
+// Rc<Arc<T>>
 
 pub fn test6(a: Rc<bool>) {}
 
+// Rc<Box<T>>
+
+pub fn test7(a: Rc<bool>) {}
+
 // Box<&T>
 
-pub fn test7<T>(foo: &T) {}
+pub fn test8<T>(foo: &T) {}
 
-pub fn test8(foo: &MyStruct) {}
+pub fn test9(foo: &MyStruct) {}
 
-pub fn test9(foo: &MyEnum) {}
+pub fn test10(foo: &MyEnum) {}
 
-pub fn test10_neg(foo: Box<SubT<&usize>>) {}
+pub fn test11_neg(foo: Box<SubT<&usize>>) {}
 
 // Arc<&T>
 
-pub fn test11<T>(foo: &T) {}
+pub fn test12<T>(foo: &T) {}
 
-pub fn test12(foo: &MyStruct) {}
+pub fn test13(foo: &MyStruct) {}
 
-pub fn test13(foo: &MyEnum) {}
+pub fn test14(foo: &MyEnum) {}
 
-pub fn test14_neg(foo: Arc<SubT<&usize>>) {}
+pub fn test15_neg(foo: Arc<SubT<&usize>>) {}
+
+// Arc<Rc<T>>
+
+pub fn test16(a: Arc<bool>) {}
 
 // Arc<Arc<T>>
 
-pub fn test15(a: Arc<bool>) {}
+pub fn test17(a: Arc<bool>) {}
 
 // Arc<Box<T>>
 
-pub fn test16(a: Arc<bool>) {}
+pub fn test18(a: Arc<bool>) {}
 
 fn main() {}
diff --git a/tests/ui/redundant_allocation.rs b/tests/ui/redundant_allocation.rs
@@ -32,36 +32,44 @@ pub fn test4_neg(foo: Rc<SubT<&usize>>) {}
 
 pub fn test5(a: Rc<Rc<bool>>) {}
 
+// Rc<Arc<T>>
+
+pub fn test6(a: Rc<Arc<bool>>) {}
+
 // Rc<Box<T>>
 
-pub fn test6(a: Rc<Box<bool>>) {}
+pub fn test7(a: Rc<Box<bool>>) {}
 
 // Box<&T>
 
-pub fn test7<T>(foo: Box<&T>) {}
+pub fn test8<T>(foo: Box<&T>) {}
 
-pub fn test8(foo: Box<&MyStruct>) {}
+pub fn test9(foo: Box<&MyStruct>) {}
 
-pub fn test9(foo: Box<&MyEnum>) {}
+pub fn test10(foo: Box<&MyEnum>) {}
 
-pub fn test10_neg(foo: Box<SubT<&usize>>) {}
+pub fn test11_neg(foo: Box<SubT<&usize>>) {}
 
 // Arc<&T>
 
-pub fn test11<T>(foo: Arc<&T>) {}
+pub fn test12<T>(foo: Arc<&T>) {}
+
+pub fn test13(foo: Arc<&MyStruct>) {}
+
+pub fn test14(foo: Arc<&MyEnum>) {}
 
-pub fn test12(foo: Arc<&MyStruct>) {}
+pub fn test15_neg(foo: Arc<SubT<&usize>>) {}
 
-pub fn test13(foo: Arc<&MyEnum>) {}
+// Arc<Rc<T>>
 
-pub fn test14_neg(foo: Arc<SubT<&usize>>) {}
+pub fn test16(a: Arc<Rc<bool>>) {}
 
 // Arc<Arc<T>>
 
-pub fn test15(a: Arc<Arc<bool>>) {}
+pub fn test17(a: Arc<Arc<bool>>) {}
 
 // Arc<Box<T>>
 
-pub fn test16(a: Arc<Box<bool>>) {}
+pub fn test18(a: Arc<Box<bool>>) {}
 
 fn main() {}
diff --git a/tests/ui/redundant_allocation.stderr b/tests/ui/redundant_allocation.stderr
@@ -24,59 +24,71 @@ error: usage of `Rc<Rc<T>>`
 LL | pub fn test5(a: Rc<Rc<bool>>) {}
    |                 ^^^^^^^^^^^^ help: try: `Rc<bool>`
 
-error: usage of `Rc<Box<T>>`
+error: usage of `Rc<Rc<T>>`
   --> $DIR/redundant_allocation.rs:37:17
    |
-LL | pub fn test6(a: Rc<Box<bool>>) {}
+LL | pub fn test6(a: Rc<Arc<bool>>) {}
+   |                 ^^^^^^^^^^^^^ help: try: `Rc<bool>`
+
+error: usage of `Rc<Box<T>>`
+  --> $DIR/redundant_allocation.rs:41:17
+   |
+LL | pub fn test7(a: Rc<Box<bool>>) {}
    |                 ^^^^^^^^^^^^^ help: try: `Rc<bool>`
 
 error: usage of `Box<&T>`
-  --> $DIR/redundant_allocation.rs:41:22
+  --> $DIR/redundant_allocation.rs:45:22
    |
-LL | pub fn test7<T>(foo: Box<&T>) {}
+LL | pub fn test8<T>(foo: Box<&T>) {}
    |                      ^^^^^^^ help: try: `&T`
 
 error: usage of `Box<&T>`
-  --> $DIR/redundant_allocation.rs:43:19
+  --> $DIR/redundant_allocation.rs:47:19
    |
-LL | pub fn test8(foo: Box<&MyStruct>) {}
+LL | pub fn test9(foo: Box<&MyStruct>) {}
    |                   ^^^^^^^^^^^^^^ help: try: `&MyStruct`
 
 error: usage of `Box<&T>`
-  --> $DIR/redundant_allocation.rs:45:19
+  --> $DIR/redundant_allocation.rs:49:20
    |
-LL | pub fn test9(foo: Box<&MyEnum>) {}
-   |                   ^^^^^^^^^^^^ help: try: `&MyEnum`
+LL | pub fn test10(foo: Box<&MyEnum>) {}
+   |                    ^^^^^^^^^^^^ help: try: `&MyEnum`
 
 error: usage of `Arc<&T>`
-  --> $DIR/redundant_allocation.rs:51:23
+  --> $DIR/redundant_allocation.rs:55:23
    |
-LL | pub fn test11<T>(foo: Arc<&T>) {}
+LL | pub fn test12<T>(foo: Arc<&T>) {}
    |                       ^^^^^^^ help: try: `&T`
 
 error: usage of `Arc<&T>`
-  --> $DIR/redundant_allocation.rs:53:20
+  --> $DIR/redundant_allocation.rs:57:20
    |
-LL | pub fn test12(foo: Arc<&MyStruct>) {}
+LL | pub fn test13(foo: Arc<&MyStruct>) {}
    |                    ^^^^^^^^^^^^^^ help: try: `&MyStruct`
 
 error: usage of `Arc<&T>`
-  --> $DIR/redundant_allocation.rs:55:20
+  --> $DIR/redundant_allocation.rs:59:20
    |
-LL | pub fn test13(foo: Arc<&MyEnum>) {}
+LL | pub fn test14(foo: Arc<&MyEnum>) {}
    |                    ^^^^^^^^^^^^ help: try: `&MyEnum`
 
-error: usage of `Arc<Arc<T>>`
-  --> $DIR/redundant_allocation.rs:61:18
+error: usage of `Arc<Rc<T>>`
+  --> $DIR/redundant_allocation.rs:65:18
    |
-LL | pub fn test15(a: Arc<Arc<bool>>) {}
+LL | pub fn test16(a: Arc<Rc<bool>>) {}
+   |                  ^^^^^^^^^^^^^ help: try: `Arc<bool>`
+
+error: usage of `Arc<Rc<T>>`
+  --> $DIR/redundant_allocation.rs:69:18
+   |
+LL | pub fn test17(a: Arc<Arc<bool>>) {}
    |                  ^^^^^^^^^^^^^^ help: try: `Arc<bool>`
 
 error: usage of `Arc<Box<T>>`
-  --> $DIR/redundant_allocation.rs:65:18
+  --> $DIR/redundant_allocation.rs:73:18
    |
-LL | pub fn test16(a: Arc<Box<bool>>) {}
+LL | pub fn test18(a: Arc<Box<bool>>) {}
    |                  ^^^^^^^^^^^^^^ help: try: `Arc<bool>`
 
-error: aborting due to 13 previous errors
+error: aborting due to 15 previous errors