From 5960107415caf34a84fc8bb2f2fd303b7e358041 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 30 Dec 2023 03:15:38 +0100 Subject: [PATCH] new lint: `option_as_ref_cloned` --- CHANGELOG.md | 1 + README.md | 2 +- book/src/README.md | 2 +- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 32 +++++++++++++++- .../src/methods/option_as_ref_cloned.rs | 24 ++++++++++++ tests/ui/option_as_ref_cloned.fixed | 21 +++++++++++ tests/ui/option_as_ref_cloned.rs | 21 +++++++++++ tests/ui/option_as_ref_cloned.stderr | 37 +++++++++++++++++++ 9 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 clippy_lints/src/methods/option_as_ref_cloned.rs create mode 100644 tests/ui/option_as_ref_cloned.fixed create mode 100644 tests/ui/option_as_ref_cloned.rs create mode 100644 tests/ui/option_as_ref_cloned.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c081ca94f1..4d32bbec914a 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 5d490645d897..0450b54121df 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 486ea3df7042..e7972b0db19c 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 f6c9ffea9fcd..20230106d536 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 fbca641cfa30..cf6f6ad1a0fc 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>) -> Option> { + /// bar.as_ref().cloned() + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn foo(bar: &Option>) -> Option> { + /// 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 000000000000..d7fec360fa2a --- /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 000000000000..394dad219f77 --- /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 = x.clone(); + let _: Option = 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 = 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 000000000000..7243957927b2 --- /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 = x.as_ref().cloned(); + let _: Option = 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 = 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 000000000000..ea03da3b69fa --- /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 = 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 = x.clone(); + | ~~~~~ + +error: cloning an `Option<_>` using `.as_mut().cloned()` + --> $DIR/option_as_ref_cloned.rs:8:31 + | +LL | let _: Option = x.as_mut().cloned(); + | ^^^^^^^^^^^^^^^ + | +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()` + --> $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 +