about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-01-05 06:39:46 +0000
committerbors <bors@rust-lang.org>2024-01-05 06:39:46 +0000
commitee8bfb7f7af0c4ba4a6d200627bb48d0e09dcc17 (patch)
treed22d2baa9dd64fe3c13478ae1a3b5451e5bbe740
parent8507e4c80e7e7af605798b8c784d4edb3fb0f436 (diff)
parent5960107415caf34a84fc8bb2f2fd303b7e358041 (diff)
downloadrust-ee8bfb7f7af0c4ba4a6d200627bb48d0e09dcc17.tar.gz
rust-ee8bfb7f7af0c4ba4a6d200627bb48d0e09dcc17.zip
Auto merge of #12051 - y21:option_as_ref_cloned, r=dswij
new lint: `option_as_ref_cloned`

Closes #12009

Adds a new lint that looks for `.as_ref().cloned()` on `Option`s. That's the same as just `.clone()`-ing the option directly.

changelog: new lint: [`option_as_ref_cloned`]
-rw-r--r--CHANGELOG.md1
-rw-r--r--README.md2
-rw-r--r--book/src/README.md2
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs32
-rw-r--r--clippy_lints/src/methods/option_as_ref_cloned.rs24
-rw-r--r--tests/ui/option_as_ref_cloned.fixed21
-rw-r--r--tests/ui/option_as_ref_cloned.rs21
-rw-r--r--tests/ui/option_as_ref_cloned.stderr37
9 files changed, 138 insertions, 3 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e6c081ca94f..4d32bbec914 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5442,6 +5442,7 @@ Released 2018-09-13
 [`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
 [`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
+[`option_as_ref_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_cloned
 [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
 [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
 [`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used
diff --git a/README.md b/README.md
index fb3f7109d7e..fa18447090c 100644
--- a/README.md
+++ b/README.md
@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.
diff --git a/book/src/README.md b/book/src/README.md
index 486ea3df704..e7972b0db19 100644
--- a/book/src/README.md
+++ b/book/src/README.md
@@ -6,7 +6,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index f6c9ffea9fc..20230106d53 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -410,6 +410,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::NO_EFFECT_REPLACE_INFO,
     crate::methods::OBFUSCATED_IF_ELSE_INFO,
     crate::methods::OK_EXPECT_INFO,
+    crate::methods::OPTION_AS_REF_CLONED_INFO,
     crate::methods::OPTION_AS_REF_DEREF_INFO,
     crate::methods::OPTION_FILTER_MAP_INFO,
     crate::methods::OPTION_MAP_OR_ERR_OK_INFO,
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index fbca641cfa3..cf6f6ad1a0f 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -71,6 +71,7 @@ mod no_effect_replace;
 mod obfuscated_if_else;
 mod ok_expect;
 mod open_options;
+mod option_as_ref_cloned;
 mod option_as_ref_deref;
 mod option_map_or_err_ok;
 mod option_map_or_none;
@@ -3887,6 +3888,31 @@ declare_clippy_lint! {
     "splitting a trimmed string at hard-coded newlines"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// 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.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn foo(bar: &Option<Vec<u8>>) -> Option<Vec<u8>> {
+    ///     bar.as_ref().cloned()
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn foo(bar: &Option<Vec<u8>>) -> Option<Vec<u8>> {
+    ///     bar.clone()
+    /// }
+    /// ```
+    #[clippy::version = "1.77.0"]
+    pub OPTION_AS_REF_CLONED,
+    pedantic,
+    "cloning an `Option` via `as_ref().cloned()`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4043,6 +4069,7 @@ impl_lint_pass!(Methods => [
     ITER_FILTER_IS_OK,
     MANUAL_IS_VARIANT_AND,
     STR_SPLIT_AT_NEWLINE,
+    OPTION_AS_REF_CLONED,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4290,7 +4317,10 @@ impl Methods {
                 ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv),
                 ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
                 ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
-                ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, &self.msrv),
+                ("cloned", []) => {
+                    cloned_instead_of_copied::check(cx, expr, recv, span, &self.msrv);
+                    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);
                     match method_call(recv) {
diff --git a/clippy_lints/src/methods/option_as_ref_cloned.rs b/clippy_lints/src/methods/option_as_ref_cloned.rs
new file mode 100644
index 00000000000..d7fec360fa2
--- /dev/null
+++ b/clippy_lints/src/methods/option_as_ref_cloned.rs
@@ -0,0 +1,24 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_span::{sym, Span};
+
+use super::{method_call, OPTION_AS_REF_CLONED};
+
+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_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
new file mode 100644
index 00000000000..394dad219f7
--- /dev/null
+++ b/tests/ui/option_as_ref_cloned.fixed
@@ -0,0 +1,21 @@
+#![warn(clippy::option_as_ref_cloned)]
+#![allow(clippy::clone_on_copy)]
+
+fn main() {
+    let mut x = Some(String::new());
+
+    let _: Option<String> = x.clone();
+    let _: Option<String> = x.clone();
+
+    let y = x.as_ref();
+    let _: Option<&String> = y.clone();
+
+    macro_rules! cloned_recv {
+        () => {
+            x.as_ref()
+        };
+    }
+
+    // Don't lint when part of the expression is from a macro
+    let _: Option<String> = cloned_recv!().cloned();
+}
diff --git a/tests/ui/option_as_ref_cloned.rs b/tests/ui/option_as_ref_cloned.rs
new file mode 100644
index 00000000000..7243957927b
--- /dev/null
+++ b/tests/ui/option_as_ref_cloned.rs
@@ -0,0 +1,21 @@
+#![warn(clippy::option_as_ref_cloned)]
+#![allow(clippy::clone_on_copy)]
+
+fn main() {
+    let mut x = Some(String::new());
+
+    let _: Option<String> = x.as_ref().cloned();
+    let _: Option<String> = x.as_mut().cloned();
+
+    let y = x.as_ref();
+    let _: Option<&String> = y.as_ref().cloned();
+
+    macro_rules! cloned_recv {
+        () => {
+            x.as_ref()
+        };
+    }
+
+    // Don't lint when part of the expression is from a macro
+    let _: Option<String> = cloned_recv!().cloned();
+}
diff --git a/tests/ui/option_as_ref_cloned.stderr b/tests/ui/option_as_ref_cloned.stderr
new file mode 100644
index 00000000000..ea03da3b69f
--- /dev/null
+++ b/tests/ui/option_as_ref_cloned.stderr
@@ -0,0 +1,37 @@
+error: cloning an `Option<_>` using `.as_ref().cloned()`
+  --> $DIR/option_as_ref_cloned.rs:7:31
+   |
+LL |     let _: Option<String> = x.as_ref().cloned();
+   |                               ^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::option-as-ref-cloned` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::option_as_ref_cloned)]`
+help: this can be written more concisely by cloning the `Option<_>` directly
+   |
+LL |     let _: Option<String> = x.clone();
+   |                               ~~~~~
+
+error: cloning an `Option<_>` using `.as_mut().cloned()`
+  --> $DIR/option_as_ref_cloned.rs:8:31
+   |
+LL |     let _: Option<String> = x.as_mut().cloned();
+   |                               ^^^^^^^^^^^^^^^
+   |
+help: this can be written more concisely by cloning the `Option<_>` directly
+   |
+LL |     let _: Option<String> = x.clone();
+   |                               ~~~~~
+
+error: cloning an `Option<_>` using `.as_ref().cloned()`
+  --> $DIR/option_as_ref_cloned.rs:11:32
+   |
+LL |     let _: Option<&String> = y.as_ref().cloned();
+   |                                ^^^^^^^^^^^^^^^
+   |
+help: this can be written more concisely by cloning the `Option<_>` directly
+   |
+LL |     let _: Option<&String> = y.clone();
+   |                                ~~~~~
+
+error: aborting due to 3 previous errors
+