new lint: option_as_ref_cloned

Author: y21

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

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,

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;
@@ -3885,6 +3886,30 @@ 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
+    ///
+    /// ### 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 {
@@ -4043,6 +4068,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 +4316,12 @@ 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);
+                    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);
+                    }
+                },
                 ("collect", []) if is_trait_method(cx, expr, sym::Iterator) => {
                     needless_collect::check(cx, span, expr, recv, call_span);
                     match method_call(recv) {

clippy_lints/src/methods/option_as_ref_cloned.rs

@@ -0,0 +1,22 @@
+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::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) {
+        span_lint_and_sugg(
+            cx,
+            OPTION_AS_REF_CLONED,
+            as_ref_ident_span.to(cloned_span),
+            "cloning an `Option<_>` using `.as_ref().cloned()`",
+            "this can be written more concisely by cloning the `Option<_>` directly",
+            "clone".into(),
+            Applicability::MachineApplicable,
+        );
+    }
+}

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 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();
+}

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 x = Some(String::new());
+
+    let _: Option<String> = x.as_ref().cloned();
+    let _: Option<String> = x.as_ref().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();
+}

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_ref().cloned()`
+  --> $DIR/option_as_ref_cloned.rs:8:31
+   |
+LL |     let _: Option<String> = x.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_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
+