From 692ec68a7473cc7324d3e13aee8836fbb34d54a1 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 30 Dec 2023 03:28:08 +0100 Subject: [PATCH] also lint `.as_mut().cloned()` --- clippy_lints/src/methods/mod.rs | 11 +++++------ clippy_lints/src/methods/option_as_ref_cloned.rs | 12 +++++++----- tests/ui/option_as_ref_cloned.fixed | 2 +- tests/ui/option_as_ref_cloned.rs | 4 ++-- tests/ui/option_as_ref_cloned.stderr | 4 ++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 462c84698d1d..29551c869efb 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3886,13 +3886,14 @@ declare_clippy_lint! { pub STR_SPLIT_AT_NEWLINE, pedantic, "splitting a trimmed string at hard-coded newlines" - +} + declare_clippy_lint! { /// ### What it does - /// Checks for usage of `.as_ref().cloned()` on `Option`s + /// Checks for usage of `.as_ref().cloned()` and `.as_mut().cloned()` on `Option`s /// /// ### Why is this bad? - /// This can be written more concisely by cloning the option directly. + /// This can be written more concisely by cloning the `Option` directly. /// /// ### Example /// ```no_run @@ -4318,9 +4319,7 @@ impl Methods { ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv), ("cloned", []) => { cloned_instead_of_copied::check(cx, expr, recv, span, &self.msrv); - if let Some(("as_ref", recv, .., as_ref_ident_span)) = method_call(recv) { - option_as_ref_cloned::check(cx, recv, as_ref_ident_span, span); - } + option_as_ref_cloned::check(cx, recv, span); }, ("collect", []) if is_trait_method(cx, expr, sym::Iterator) => { needless_collect::check(cx, span, expr, recv, call_span); diff --git a/clippy_lints/src/methods/option_as_ref_cloned.rs b/clippy_lints/src/methods/option_as_ref_cloned.rs index 32633b72032d..d7fec360fa2a 100644 --- a/clippy_lints/src/methods/option_as_ref_cloned.rs +++ b/clippy_lints/src/methods/option_as_ref_cloned.rs @@ -5,15 +5,17 @@ use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_span::{sym, Span}; -use super::OPTION_AS_REF_CLONED; +use super::{method_call, OPTION_AS_REF_CLONED}; -pub(super) fn check(cx: &LateContext<'_>, as_ref_recv: &Expr<'_>, as_ref_ident_span: Span, cloned_span: Span) { - if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(as_ref_recv).peel_refs(), sym::Option) { +pub(super) fn check(cx: &LateContext<'_>, cloned_recv: &Expr<'_>, cloned_ident_span: Span) { + if let Some((method @ ("as_ref" | "as_mut"), as_ref_recv, [], as_ref_ident_span, _)) = method_call(cloned_recv) + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(as_ref_recv).peel_refs(), sym::Option) + { span_lint_and_sugg( cx, OPTION_AS_REF_CLONED, - as_ref_ident_span.to(cloned_span), - "cloning an `Option<_>` using `.as_ref().cloned()`", + as_ref_ident_span.to(cloned_ident_span), + &format!("cloning an `Option<_>` using `.{method}().cloned()`"), "this can be written more concisely by cloning the `Option<_>` directly", "clone".into(), Applicability::MachineApplicable, diff --git a/tests/ui/option_as_ref_cloned.fixed b/tests/ui/option_as_ref_cloned.fixed index 758e71606750..394dad219f77 100644 --- a/tests/ui/option_as_ref_cloned.fixed +++ b/tests/ui/option_as_ref_cloned.fixed @@ -2,7 +2,7 @@ #![allow(clippy::clone_on_copy)] fn main() { - let x = Some(String::new()); + let mut x = Some(String::new()); let _: Option = x.clone(); let _: Option = x.clone(); diff --git a/tests/ui/option_as_ref_cloned.rs b/tests/ui/option_as_ref_cloned.rs index 7beaf6ea6701..7243957927b2 100644 --- a/tests/ui/option_as_ref_cloned.rs +++ b/tests/ui/option_as_ref_cloned.rs @@ -2,10 +2,10 @@ #![allow(clippy::clone_on_copy)] fn main() { - let x = Some(String::new()); + let mut x = Some(String::new()); let _: Option = x.as_ref().cloned(); - let _: Option = x.as_ref().cloned(); + let _: Option = x.as_mut().cloned(); let y = x.as_ref(); let _: Option<&String> = y.as_ref().cloned(); diff --git a/tests/ui/option_as_ref_cloned.stderr b/tests/ui/option_as_ref_cloned.stderr index 4e482d812fe7..ea03da3b69fa 100644 --- a/tests/ui/option_as_ref_cloned.stderr +++ b/tests/ui/option_as_ref_cloned.stderr @@ -11,10 +11,10 @@ help: this can be written more concisely by cloning the `Option<_>` directly LL | let _: Option = x.clone(); | ~~~~~ -error: cloning an `Option<_>` using `.as_ref().cloned()` +error: cloning an `Option<_>` using `.as_mut().cloned()` --> $DIR/option_as_ref_cloned.rs:8:31 | -LL | let _: Option = x.as_ref().cloned(); +LL | let _: Option = x.as_mut().cloned(); | ^^^^^^^^^^^^^^^ | help: this can be written more concisely by cloning the `Option<_>` directly