Auto merge of #7308 - lengyijun:redundant_allocation_arc, r=xFrednet,flip1995

add Arc to `redundant_allocation`

 fixes #7303
changelog:  add Arc to `redundant_allocation`

Author: bors

clippy_lints/src/types/mod.rs

@@ -178,8 +178,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>>`, `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>`, `Box<Rc<T>>`, `Box<Arc<T>>`, `Box<Box<T>>`, add an unnecessary level of indirection.
     ///
     /// **Known problems:** None.
     ///

clippy_lints/src/types/redundant_allocation.rs

@@ -1,5 +1,5 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::{get_qpath_generic_tys, is_ty_param_diagnostic_item, is_ty_param_lang_item};
 use rustc_errors::Applicability;
 use rustc_hir::{self as hir, def_id::DefId, LangItem, QPath, TyKind};
@@ -9,74 +9,99 @@ use rustc_span::symbol::sym;
 use super::{utils, REDUNDANT_ALLOCATION};
 
 pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool {
-    if Some(def_id) == cx.tcx.lang_items().owned_box() {
-        if let Some(span) = utils::match_borrows_parameter(cx, qpath) {
-            let mut applicability = Applicability::MachineApplicable;
-            span_lint_and_sugg(
-                cx,
-                REDUNDANT_ALLOCATION,
-                hir_ty.span,
-                "usage of `Box<&T>`",
-                "try",
-                snippet_with_applicability(cx, span, "..", &mut applicability).to_string(),
-                applicability,
-            );
-            return true;
-        }
+    let outer_sym = if Some(def_id) == cx.tcx.lang_items().owned_box() {
+        "Box"
+    } else if cx.tcx.is_diagnostic_item(sym::Rc, def_id) {
+        "Rc"
+    } else if cx.tcx.is_diagnostic_item(sym::Arc, def_id) {
+        "Arc"
+    } else {
+        return false;
+    };
+
+    if let Some(span) = utils::match_borrows_parameter(cx, qpath) {
+        let mut applicability = Applicability::MaybeIncorrect;
+        let generic_snippet = snippet_with_applicability(cx, span, "..", &mut applicability);
+        span_lint_and_then(
+            cx,
+            REDUNDANT_ALLOCATION,
+            hir_ty.span,
+            &format!("usage of `{}<{}>`", outer_sym, generic_snippet),
+            |diag| {
+                diag.span_suggestion(hir_ty.span, "try", format!("{}", generic_snippet), applicability);
+                diag.note(&format!(
+                    "`{generic}` is already a pointer, `{outer}<{generic}>` allocates a pointer on the heap",
+                    outer = outer_sym,
+                    generic = generic_snippet
+                ));
+            },
+        );
+        return true;
     }
 
-    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,
-            );
-            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,
-            );
-            true
-        } else {
-            utils::match_borrows_parameter(cx, qpath).map_or(false, |span| {
-                let mut applicability = Applicability::MachineApplicable;
-                span_lint_and_sugg(
-                    cx,
-                    REDUNDANT_ALLOCATION,
+    let (inner_sym, ty) = if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) {
+        ("Box", ty)
+    } else if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Rc) {
+        ("Rc", ty)
+    } else if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Arc) {
+        ("Arc", ty)
+    } else {
+        return false;
+    };
+
+    let inner_qpath = match &ty.kind {
+        TyKind::Path(inner_qpath) => inner_qpath,
+        _ => return false,
+    };
+    let inner_span = match get_qpath_generic_tys(inner_qpath).next() {
+        Some(ty) => ty.span,
+        None => return false,
+    };
+    if inner_sym == outer_sym {
+        let mut applicability = Applicability::MaybeIncorrect;
+        let generic_snippet = snippet_with_applicability(cx, inner_span, "..", &mut applicability);
+        span_lint_and_then(
+            cx,
+            REDUNDANT_ALLOCATION,
+            hir_ty.span,
+            &format!("usage of `{}<{}<{}>>`", outer_sym, inner_sym, generic_snippet),
+            |diag| {
+                diag.span_suggestion(
                     hir_ty.span,
-                    "usage of `Rc<&T>`",
                     "try",
-                    snippet_with_applicability(cx, span, "..", &mut applicability).to_string(),
+                    format!("{}<{}>", outer_sym, generic_snippet),
                     applicability,
                 );
-                true
-            })
-        }
+                diag.note(&format!(
+                    "`{inner}<{generic}>` is already on the heap, `{outer}<{inner}<{generic}>>` makes an extra allocation",
+                    outer = outer_sym,
+                    inner = inner_sym,
+                    generic = generic_snippet
+                ));
+            },
+        );
     } else {
-        false
+        let generic_snippet = snippet(cx, inner_span, "..");
+        span_lint_and_then(
+            cx,
+            REDUNDANT_ALLOCATION,
+            hir_ty.span,
+            &format!("usage of `{}<{}<{}>>`", outer_sym, inner_sym, generic_snippet),
+            |diag| {
+                diag.note(&format!(
+                    "`{inner}<{generic}>` is already on the heap, `{outer}<{inner}<{generic}>>` makes an extra allocation",
+                    outer = outer_sym,
+                    inner = inner_sym,
+                    generic = generic_snippet
+                ));
+                diag.help(&format!(
+                    "consider using just `{outer}<{generic}>` or `{inner}<{generic}>`",
+                    outer = outer_sym,
+                    inner = inner_sym,
+                    generic = generic_snippet
+                ));
+            },
+        );
     }
+    true
 }

tests/ui/redundant_allocation.fixed

@@ -1,10 +1,7 @@
-// run-rustfix
 #![warn(clippy::all)]
 #![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
 #![allow(clippy::blacklisted_name, unused_variables, dead_code)]
-
-use std::boxed::Box;
-use std::rc::Rc;
+#![allow(unused_imports)]
 
 pub struct MyStruct {}
 
@@ -17,32 +14,67 @@ pub enum MyEnum {
     Two,
 }
 
-// Rc<&T>
+mod outer_box {
+    use crate::MyEnum;
+    use crate::MyStruct;
+    use crate::SubT;
+    use std::boxed::Box;
+    use std::rc::Rc;
+    use std::sync::Arc;
 
-pub fn test1<T>(foo: Rc<&T>) {}
+    pub fn box_test6<T>(foo: Box<Rc<T>>) {}
 
-pub fn test2(foo: Rc<&MyStruct>) {}
+    pub fn box_test7<T>(foo: Box<Arc<T>>) {}
 
-pub fn test3(foo: Rc<&MyEnum>) {}
+    pub fn box_test8() -> Box<Rc<SubT<usize>>> {
+        unimplemented!();
+    }
 
-pub fn test4_neg(foo: Rc<SubT<&usize>>) {}
+    pub fn box_test9<T>(foo: Box<Arc<T>>) -> Box<Arc<SubT<T>>> {
+        unimplemented!();
+    }
+}
 
-// Rc<Rc<T>>
+mod outer_rc {
+    use crate::MyEnum;
+    use crate::MyStruct;
+    use crate::SubT;
+    use std::boxed::Box;
+    use std::rc::Rc;
+    use std::sync::Arc;
 
-pub fn test5(a: Rc<Rc<bool>>) {}
+    pub fn rc_test5(a: Rc<Box<bool>>) {}
 
-// Rc<Box<T>>
+    pub fn rc_test7(a: Rc<Arc<bool>>) {}
 
-pub fn test6(a: Rc<Box<bool>>) {}
+    pub fn rc_test8() -> Rc<Box<SubT<usize>>> {
+        unimplemented!();
+    }
 
-// Box<&T>
+    pub fn rc_test9<T>(foo: Rc<Arc<T>>) -> Rc<Arc<SubT<T>>> {
+        unimplemented!();
+    }
+}
 
-pub fn test7<T>(foo: Box<&T>) {}
+mod outer_arc {
+    use crate::MyEnum;
+    use crate::MyStruct;
+    use crate::SubT;
+    use std::boxed::Box;
+    use std::rc::Rc;
+    use std::sync::Arc;
 
-pub fn test8(foo: Box<&MyStruct>) {}
+    pub fn arc_test5(a: Arc<Box<bool>>) {}
 
-pub fn test9(foo: Box<&MyEnum>) {}
+    pub fn arc_test6(a: Arc<Rc<bool>>) {}
 
-pub fn test10_neg(foo: Box<SubT<&usize>>) {}
+    pub fn arc_test8() -> Arc<Box<SubT<usize>>> {
+        unimplemented!();
+    }
+
+    pub fn arc_test9<T>(foo: Arc<Rc<T>>) -> Arc<Rc<SubT<T>>> {
+        unimplemented!();
+    }
+}
 
 fn main() {}