Skip to content

Commit

Permalink
add clone_on_arc_or_rc lint
Browse files Browse the repository at this point in the history
  • Loading branch information
lapla-cogito committed Jan 1, 2025
1 parent b9241c3 commit d8ded3f
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
51 changes: 51 additions & 0 deletions clippy_lints/src/methods/clone_on_arc_or_rc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
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,
"using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient",
"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> {
match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(expr)) {
Some(sym::Arc) => Some("std::sync::Arc"),
Some(sym::Rc) => Some("std::rc::Rc"),
_ => None,
}
}
34 changes: 34 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 a Arc or 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 `Arc` or `Rc`"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/clone_on_arc_or_rc.fixed
Original file line number Diff line number Diff line change
@@ -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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let rc: Rc<String> = Rc::new("foo".into());
let _: String = std::rc::Rc::unwrap_or_clone(rc);
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let arc: Arc<String> = Arc::new("foo bar".into());
let _: Vec<_> = std::sync::Arc::unwrap_or_clone(arc).split(" ").collect();
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let _: String = std::sync::Arc::unwrap_or_clone(Arc::<String>::new("foo".into()));
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
}

#[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
}
36 changes: 36 additions & 0 deletions tests/ui/clone_on_arc_or_rc.rs
Original file line number Diff line number Diff line change
@@ -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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let rc: Rc<String> = Rc::new("foo".into());
let _: String = (*rc).clone();
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let arc: Arc<String> = Arc::new("foo bar".into());
let _: Vec<_> = (*arc).clone().split(" ").collect();
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let rc: Rc<Vec<u32>> = Rc::new(vec![1, 2, 3]);
let _: Vec<_> = (*rc).clone().iter().map(|x| x + 1).collect();
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient

let _: String = (*Arc::<String>::new("foo".into())).clone();
//~^ error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
}

#[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
}
35 changes: 35 additions & 0 deletions tests/ui/clone_on_arc_or_rc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
--> 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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
--> 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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
--> 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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
--> 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: using `unwrap_or_clone` on an `Arc` or `Rc` is more efficient
--> 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

0 comments on commit d8ded3f

Please sign in to comment.