new lint for clone() on an Arc or Rc

CHANGELOG.md

@@ -5439,6 +5439,7 @@ Released 2018-09-13
 [`checked_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions
 [`clear_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#clear_with_drain
 [`clone_double_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_double_ref
+[`clone_on_arc_or_rc`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_arc_or_rc
 [`clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
 [`clone_on_ref_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr
 [`cloned_instead_of_copied`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied

clippy_lints/src/declared_lints.rs

@@ -368,6 +368,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::CHARS_NEXT_CMP_INFO,
     crate::methods::CLEAR_WITH_DRAIN_INFO,
     crate::methods::CLONED_INSTEAD_OF_COPIED_INFO,
+    crate::methods::CLONE_ON_ARC_OR_RC_INFO,
     crate::methods::CLONE_ON_COPY_INFO,
     crate::methods::CLONE_ON_REF_PTR_INFO,
     crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,

clippy_lints/src/methods/clone_on_arc_or_rc.rs

@@ -0,0 +1,64 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_no_std_crate;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet;
+use clippy_utils::ty::get_type_diagnostic_name;
+use rustc_ast::ast::UnOp;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::{Symbol, sym};
+
+use super::CLONE_ON_ARC_OR_RC;
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    method_name: Symbol,
+    receiver: &Expr<'_>,
+    args: &[Expr<'_>],
+    msrv: &Msrv,
+) {
+    if !msrv.meets(msrvs::ARC_RC_UNWRAP_OR_CLONE) {
+        return;
+    }
+
+    if method_name == sym::clone
+        && args.is_empty()
+        && let ExprKind::Unary(UnOp::Deref, recv) = receiver.kind
+        && let Some(arc_or_rc_path) = is_arc_or_rc(cx, recv)
+    {
+        span_lint_and_sugg(
+            cx,
+            CLONE_ON_ARC_OR_RC,
+            expr.span,
+            "conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone",
+            "try",
+            format!(
+                "{arc_or_rc_path}::unwrap_or_clone({snip})",
+                snip = snippet(cx, recv.span, "..")
+            ),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+fn is_arc_or_rc(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<&'static str> {
+    let is_no_std = is_no_std_crate(cx);
+
+    match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(expr)) {
+        Some(sym::Arc) => {
+            if is_no_std {
+                return Some("alloc::sync::Arc");
+            }
+            Some("std::sync::Arc")
+        },
+        Some(sym::Rc) => {
+            if is_no_std {
+                return Some("alloc::rc::Rc");
+            }
+            Some("std::rc::Rc")
+        },
+        _ => None,
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -10,6 +10,7 @@ mod chars_last_cmp_with_unwrap;
 mod chars_next_cmp;
 mod chars_next_cmp_with_unwrap;
 mod clear_with_drain;
+mod clone_on_arc_or_rc;
 mod clone_on_copy;
 mod clone_on_ref_ptr;
 mod cloned_instead_of_copied;
@@ -4284,6 +4285,37 @@ declare_clippy_lint! {
     "map of a trivial closure (not dependent on parameter) over a range"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usage of `(*foo).clone()` where `foo` is an owned `std::sync::Arc` or `std::rc::Rc`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// This `clone()` call can be replaced with `Arc::unwrap_or_clone()` or `Rc::unwrap_or_clone()`, which reduce unnecessary clones.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use std::rc::Rc;
+    /// use std::sync::Arc;
+    ///
+    /// let foo: Arc<String> = Arc::new("foo".into());
+    /// let bar: String = (*foo).clone();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// use std::rc::Rc;
+    /// use std::sync::Arc;
+    ///
+    /// let foo: Arc<String> = Arc::new("foo".into());
+    /// let bar: String = Arc::unwrap_or_clone(foo);
+    /// ```
+    #[clippy::version = "1.85.0"]
+    pub CLONE_ON_ARC_OR_RC,
+    perf,
+    "calling `(*foo).clone()` where `foo` is an owned `std::sync::Arc` or `std::rc::Rc`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4449,6 +4481,7 @@ impl_lint_pass!(Methods => [
     MAP_ALL_ANY_IDENTITY,
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     UNNECESSARY_MAP_OR,
+    CLONE_ON_ARC_OR_RC,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4492,6 +4525,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 );
                 clone_on_copy::check(cx, expr, method_call.ident.name, receiver, args);
                 clone_on_ref_ptr::check(cx, expr, method_call.ident.name, receiver, args);
+                clone_on_arc_or_rc::check(cx, expr, method_call.ident.name, receiver, args, &self.msrv);
                 inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args);
                 single_char_add_str::check(cx, expr, receiver, args);
                 into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, receiver);

clippy_utils/src/msrvs.rs

@@ -23,7 +23,7 @@ msrv_aliases! {
     1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE }
     1,80,0 { BOX_INTO_ITER }
     1,77,0 { C_STR_LITERALS }
-    1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
+    1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT, ARC_RC_UNWRAP_OR_CLONE }
     1,74,0 { REPR_RUST }
     1,73,0 { MANUAL_DIV_CEIL }
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }

tests/ui/clone_on_arc_or_rc.fixed

@@ -0,0 +1,36 @@
+#![warn(clippy::clone_on_arc_or_rc)]
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+fn main() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = std::sync::Arc::unwrap_or_clone(arc);
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = std::rc::Rc::unwrap_or_clone(rc);
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let arc: Arc<String> = Arc::new("foo bar".into());
+    let _: Vec<_> = std::sync::Arc::unwrap_or_clone(arc).split(" ").collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<Vec<u32>> = Rc::new(vec![1, 2, 3]);
+    let _: Vec<_> = std::rc::Rc::unwrap_or_clone(rc).iter().map(|x| x + 1).collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let _: String = std::sync::Arc::unwrap_or_clone(Arc::<String>::new("foo".into()));
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+}
+
+#[clippy::msrv = "1.75"]
+fn msrv_check() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = (*arc).clone();
+    // Should not lint
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = (*rc).clone();
+    // Should not lint
+}

tests/ui/clone_on_arc_or_rc.rs

@@ -0,0 +1,36 @@
+#![warn(clippy::clone_on_arc_or_rc)]
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+fn main() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = (*arc).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = (*rc).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let arc: Arc<String> = Arc::new("foo bar".into());
+    let _: Vec<_> = (*arc).clone().split(" ").collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<Vec<u32>> = Rc::new(vec![1, 2, 3]);
+    let _: Vec<_> = (*rc).clone().iter().map(|x| x + 1).collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let _: String = (*Arc::<String>::new("foo".into())).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+}
+
+#[clippy::msrv = "1.75"]
+fn msrv_check() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = (*arc).clone();
+    // Should not lint
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = (*rc).clone();
+    // Should not lint
+}

tests/ui/clone_on_arc_or_rc.stderr

@@ -0,0 +1,35 @@
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc.rs:8:21
+   |
+LL |     let _: String = (*arc).clone();
+   |                     ^^^^^^^^^^^^^^ help: try: `std::sync::Arc::unwrap_or_clone(arc)`
+   |
+   = note: `-D clippy::clone-on-arc-or-rc` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::clone_on_arc_or_rc)]`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc.rs:12:21
+   |
+LL |     let _: String = (*rc).clone();
+   |                     ^^^^^^^^^^^^^ help: try: `std::rc::Rc::unwrap_or_clone(rc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc.rs:16:21
+   |
+LL |     let _: Vec<_> = (*arc).clone().split(" ").collect();
+   |                     ^^^^^^^^^^^^^^ help: try: `std::sync::Arc::unwrap_or_clone(arc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc.rs:20:21
+   |
+LL |     let _: Vec<_> = (*rc).clone().iter().map(|x| x + 1).collect();
+   |                     ^^^^^^^^^^^^^ help: try: `std::rc::Rc::unwrap_or_clone(rc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc.rs:23:21
+   |
+LL |     let _: String = (*Arc::<String>::new("foo".into())).clone();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::sync::Arc::unwrap_or_clone(Arc::<String>::new("foo".into()))`
+
+error: aborting due to 5 previous errors
+

tests/ui/clone_on_arc_or_rc_no_std.fixed

@@ -0,0 +1,30 @@
+#![no_std]
+#![warn(clippy::clone_on_arc_or_rc)]
+
+extern crate alloc;
+use alloc::rc::Rc;
+use alloc::string::String;
+use alloc::sync::Arc;
+use alloc::vec;
+use alloc::vec::Vec;
+
+fn no_std() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = alloc::sync::Arc::unwrap_or_clone(arc);
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = alloc::rc::Rc::unwrap_or_clone(rc);
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let arc: Arc<String> = Arc::new("foo bar".into());
+    let _: Vec<_> = alloc::sync::Arc::unwrap_or_clone(arc).split(" ").collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<Vec<u32>> = Rc::new(vec![1, 2, 3]);
+    let _: Vec<_> = alloc::rc::Rc::unwrap_or_clone(rc).iter().map(|x| x + 1).collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let _: String = alloc::sync::Arc::unwrap_or_clone(Arc::<String>::new("foo".into()));
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+}

tests/ui/clone_on_arc_or_rc_no_std.rs

@@ -0,0 +1,30 @@
+#![no_std]
+#![warn(clippy::clone_on_arc_or_rc)]
+
+extern crate alloc;
+use alloc::rc::Rc;
+use alloc::string::String;
+use alloc::sync::Arc;
+use alloc::vec;
+use alloc::vec::Vec;
+
+fn no_std() {
+    let arc: Arc<String> = Arc::new("foo".into());
+    let _: String = (*arc).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<String> = Rc::new("foo".into());
+    let _: String = (*rc).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let arc: Arc<String> = Arc::new("foo bar".into());
+    let _: Vec<_> = (*arc).clone().split(" ").collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let rc: Rc<Vec<u32>> = Rc::new(vec![1, 2, 3]);
+    let _: Vec<_> = (*rc).clone().iter().map(|x| x + 1).collect();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+
+    let _: String = (*Arc::<String>::new("foo".into())).clone();
+    //~^ error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+}

tests/ui/clone_on_arc_or_rc_no_std.stderr

@@ -0,0 +1,35 @@
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc_no_std.rs:13:21
+   |
+LL |     let _: String = (*arc).clone();
+   |                     ^^^^^^^^^^^^^^ help: try: `alloc::sync::Arc::unwrap_or_clone(arc)`
+   |
+   = note: `-D clippy::clone-on-arc-or-rc` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::clone_on_arc_or_rc)]`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc_no_std.rs:17:21
+   |
+LL |     let _: String = (*rc).clone();
+   |                     ^^^^^^^^^^^^^ help: try: `alloc::rc::Rc::unwrap_or_clone(rc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc_no_std.rs:21:21
+   |
+LL |     let _: Vec<_> = (*arc).clone().split(" ").collect();
+   |                     ^^^^^^^^^^^^^^ help: try: `alloc::sync::Arc::unwrap_or_clone(arc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc_no_std.rs:25:21
+   |
+LL |     let _: Vec<_> = (*rc).clone().iter().map(|x| x + 1).collect();
+   |                     ^^^^^^^^^^^^^ help: try: `alloc::rc::Rc::unwrap_or_clone(rc)`
+
+error: conditionally unwrapping an `Arc`/`Rc` may avoid unnecessary clone
+  --> tests/ui/clone_on_arc_or_rc_no_std.rs:28:21
+   |
+LL |     let _: String = (*Arc::<String>::new("foo".into())).clone();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `alloc::sync::Arc::unwrap_or_clone(Arc::<String>::new("foo".into()))`
+
+error: aborting due to 5 previous errors
+