From 39f171939575a242e19aa7e6cec070689804e53f Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Sun, 19 Jan 2025 16:30:58 +0100 Subject: [PATCH 01/10] Add single_option_map lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 1 + clippy_lints/src/single_option_map.rs | 68 +++++++++++++++++++++++++++ tests/ui/single_option_map.rs | 17 +++++++ tests/ui/single_option_map.stderr | 21 +++++++++ 6 files changed, 109 insertions(+) create mode 100644 clippy_lints/src/single_option_map.rs create mode 100644 tests/ui/single_option_map.rs create mode 100644 tests/ui/single_option_map.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index fa03c953aa59..ea1119aca984 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6067,6 +6067,7 @@ Released 2018-09-13 [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else +[`single_option_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_option_map [`single_range_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_range_in_vec_init [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count [`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9fbeab5bf2e1..3d4ca4faecb4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -684,6 +684,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::single_call_fn::SINGLE_CALL_FN_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, + crate::single_option_map::SINGLE_OPTION_MAP_INFO, crate::single_range_in_vec_init::SINGLE_RANGE_IN_VEC_INIT_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, crate::size_of_ref::SIZE_OF_REF_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8887ab7ec0d7..e0bc8e36d705 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -339,6 +339,7 @@ mod significant_drop_tightening; mod single_call_fn; mod single_char_lifetime_names; mod single_component_path_imports; +mod single_option_map; mod single_range_in_vec_init; mod size_of_in_element_count; mod size_of_ref; diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs new file mode 100644 index 000000000000..ba8f0330598e --- /dev/null +++ b/clippy_lints/src/single_option_map.rs @@ -0,0 +1,68 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::peel_blocks; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::{Span, sym}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for functions with method calls to `.map(_)` on an arg + /// of type `Option` as the outermost expression. + /// + /// ### Why is this bad? + /// Taking and returning an `Option` may require additional + /// `Some(_)` and `unwrap` if all you have is a `T` + /// + /// ### Example + /// ```no_run + /// fn double(param: Option) -> Option { + /// param.map(|x| x * 2) + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn double(param: u32) -> u32 { + /// param * 2 + /// } + /// ``` + #[clippy::version = "1.86.0"] + pub SINGLE_OPTION_MAP, + nursery, + "Checks for functions with method calls to `.map(_)` on an arg of type `Option` as the outermost expression." +} + +declare_lint_pass!(SingleOptionMap => [SINGLE_OPTION_MAP]); + +impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + _fn_def: LocalDefId, + ) { + if let FnRetTy::Return(_ret) = decl.output + && matches!(kind, FnKind::ItemFn(_, _, _) | FnKind::Method(_, _)) + { + let func_body = peel_blocks(body.value); + if let ExprKind::MethodCall(method_name, callee, _args, _span) = func_body.kind + && method_name.ident.name == sym::map + && let callee_type = cx.typeck_results().expr_ty(callee) + && is_type_diagnostic_item(cx, callee_type, sym::Option) + { + span_lint( + cx, + SINGLE_OPTION_MAP, + span, + "mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function.", + ); + } + } + } +} diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs new file mode 100644 index 000000000000..9edef21eab9a --- /dev/null +++ b/tests/ui/single_option_map.rs @@ -0,0 +1,17 @@ +#![warn(clippy::single_option_map)] + +fn h(arg: Option) -> Option { + arg.map(|x| x * 2) +} + +fn j(arg: Option) -> Option { + arg.map(|x| x * 2) +} + +fn main() { + let answer = Some(42u32); + let h_result = h(answer); + + let answer = Some(42u64); + let j_result = j(answer); +} diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr new file mode 100644 index 000000000000..8ad5838dd1dd --- /dev/null +++ b/tests/ui/single_option_map.stderr @@ -0,0 +1,21 @@ +error: mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function. + --> tests/ui/single_option_map.rs:3:1 + | +LL | / fn h(arg: Option) -> Option { +LL | | arg.map(|x| x * 2) +LL | | } + | |_^ + | + = note: `-D clippy::single-option-map` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::single_option_map)]` + +error: mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function. + --> tests/ui/single_option_map.rs:7:1 + | +LL | / fn j(arg: Option) -> Option { +LL | | arg.map(|x| x * 2) +LL | | } + | |_^ + +error: aborting due to 2 previous errors + From 5db4dc37bf9bd3d49040e75ad66f887d950bb068 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Mon, 20 Jan 2025 21:50:15 +0100 Subject: [PATCH 02/10] Check that map() receiver is the func param --- clippy_lints/src/single_option_map.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs index ba8f0330598e..a4dd9d999a27 100644 --- a/clippy_lints/src/single_option_map.rs +++ b/clippy_lints/src/single_option_map.rs @@ -55,6 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { && method_name.ident.name == sym::map && let callee_type = cx.typeck_results().expr_ty(callee) && is_type_diagnostic_item(cx, callee_type, sym::Option) + && let ExprKind::Path(_path) = callee.kind { span_lint( cx, From a4ea1a2c99e95e53ef208ed2b21cc50693f9b636 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Mon, 20 Jan 2025 22:10:04 +0100 Subject: [PATCH 03/10] Fix single_option_map lint emission --- clippy_lints/src/single_option_map.rs | 8 +++++--- tests/ui/single_option_map.stderr | 7 +++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs index a4dd9d999a27..b23704c96e39 100644 --- a/clippy_lints/src/single_option_map.rs +++ b/clippy_lints/src/single_option_map.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::peel_blocks; use clippy_utils::ty::is_type_diagnostic_item; use rustc_hir::def_id::LocalDefId; @@ -57,11 +57,13 @@ impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { && is_type_diagnostic_item(cx, callee_type, sym::Option) && let ExprKind::Path(_path) = callee.kind { - span_lint( + span_lint_and_help( cx, SINGLE_OPTION_MAP, span, - "mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function.", + "`fn` that only maps over argument", + None, + "move the `.map` to the caller or to an `_opt` function", ); } } diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr index 8ad5838dd1dd..96827a783d7f 100644 --- a/tests/ui/single_option_map.stderr +++ b/tests/ui/single_option_map.stderr @@ -1,4 +1,4 @@ -error: mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function. +error: `fn` that only maps over argument --> tests/ui/single_option_map.rs:3:1 | LL | / fn h(arg: Option) -> Option { @@ -6,16 +6,19 @@ LL | | arg.map(|x| x * 2) LL | | } | |_^ | + = help: move the `.map` to the caller or to an `_opt` function = note: `-D clippy::single-option-map` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::single_option_map)]` -error: mapping over single function argument of `Option`, to return `Option`, can be best expressed by applying the map outside of the function. +error: `fn` that only maps over argument --> tests/ui/single_option_map.rs:7:1 | LL | / fn j(arg: Option) -> Option { LL | | arg.map(|x| x * 2) LL | | } | |_^ + | + = help: move the `.map` to the caller or to an `_opt` function error: aborting due to 2 previous errors From fd98bd83f4beda7c648a9d84970a0459bd3c9d2a Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Tue, 21 Jan 2025 21:01:18 +0100 Subject: [PATCH 04/10] Ensure map callee references an argument --- clippy_lints/src/single_option_map.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs index b23704c96e39..4eb7dad07f67 100644 --- a/clippy_lints/src/single_option_map.rs +++ b/clippy_lints/src/single_option_map.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::peel_blocks; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{path_res, peel_blocks}; +use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy}; @@ -56,6 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { && let callee_type = cx.typeck_results().expr_ty(callee) && is_type_diagnostic_item(cx, callee_type, sym::Option) && let ExprKind::Path(_path) = callee.kind + && let Res::Local(_id) = path_res(cx, callee) { span_lint_and_help( cx, From a4225ad0b8f0fad9ad35a6d6798f08115fde1268 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Thu, 23 Jan 2025 19:20:09 +0100 Subject: [PATCH 05/10] Test scenarios that shouldn't be linted --- tests/ui/single_option_map.rs | 16 ++++++++++++++++ tests/ui/single_option_map.stderr | 24 ------------------------ 2 files changed, 16 insertions(+), 24 deletions(-) delete mode 100644 tests/ui/single_option_map.stderr diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index 9edef21eab9a..3606e10de705 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -1,5 +1,9 @@ #![warn(clippy::single_option_map)] +use std::sync::atomic::{AtomicUsize, Ordering}; + +static MAYBE_ATOMIC: Option = Some(AtomicUsize::new(42)); + fn h(arg: Option) -> Option { arg.map(|x| x * 2) } @@ -8,10 +12,22 @@ fn j(arg: Option) -> Option { arg.map(|x| x * 2) } +fn maps_static_option() -> Option { + MAYBE_ATOMIC.as_ref().map(|a| a.load(Ordering::Relaxed)) +} + +fn manipulate(i: i32) -> i32 { + i + 1 +} +fn manipulate_opt(opt_i: Option) -> Option { + opt_i.map(manipulate) +} + fn main() { let answer = Some(42u32); let h_result = h(answer); let answer = Some(42u64); let j_result = j(answer); + maps_static_option(); } diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr deleted file mode 100644 index 96827a783d7f..000000000000 --- a/tests/ui/single_option_map.stderr +++ /dev/null @@ -1,24 +0,0 @@ -error: `fn` that only maps over argument - --> tests/ui/single_option_map.rs:3:1 - | -LL | / fn h(arg: Option) -> Option { -LL | | arg.map(|x| x * 2) -LL | | } - | |_^ - | - = help: move the `.map` to the caller or to an `_opt` function - = note: `-D clippy::single-option-map` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::single_option_map)]` - -error: `fn` that only maps over argument - --> tests/ui/single_option_map.rs:7:1 - | -LL | / fn j(arg: Option) -> Option { -LL | | arg.map(|x| x * 2) -LL | | } - | |_^ - | - = help: move the `.map` to the caller or to an `_opt` function - -error: aborting due to 2 previous errors - From 6a2054a9cc7131d5e24a4d6b702408775d0c6f37 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Tue, 28 Jan 2025 08:12:11 +0100 Subject: [PATCH 06/10] Fix test case for scenario that shouldn't be linted --- clippy_lints/src/lib.rs | 1 + clippy_lints/src/single_option_map.rs | 3 ++- tests/ui/single_option_map.rs | 5 +++-- tests/ui/single_option_map.stderr | 24 ++++++++++++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 tests/ui/single_option_map.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e0bc8e36d705..53a05a5a5586 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -979,5 +979,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::::default()); store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf))); store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf))); + store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs index 4eb7dad07f67..7f69f33ff584 100644 --- a/clippy_lints/src/single_option_map.rs +++ b/clippy_lints/src/single_option_map.rs @@ -52,12 +52,13 @@ impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { && matches!(kind, FnKind::ItemFn(_, _, _) | FnKind::Method(_, _)) { let func_body = peel_blocks(body.value); - if let ExprKind::MethodCall(method_name, callee, _args, _span) = func_body.kind + if let ExprKind::MethodCall(method_name, callee, args, _span) = func_body.kind && method_name.ident.name == sym::map && let callee_type = cx.typeck_results().expr_ty(callee) && is_type_diagnostic_item(cx, callee_type, sym::Option) && let ExprKind::Path(_path) = callee.kind && let Res::Local(_id) = path_res(cx, callee) + && !matches!(args[0].kind, ExprKind::Path(_)) { span_lint_and_help( cx, diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index 3606e10de705..dd99f2350399 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -2,7 +2,8 @@ use std::sync::atomic::{AtomicUsize, Ordering}; -static MAYBE_ATOMIC: Option = Some(AtomicUsize::new(42)); +static ATOM: AtomicUsize = AtomicUsize::new(42); +static MAYBE_ATOMIC: Option<&AtomicUsize> = Some(&ATOM); fn h(arg: Option) -> Option { arg.map(|x| x * 2) @@ -13,7 +14,7 @@ fn j(arg: Option) -> Option { } fn maps_static_option() -> Option { - MAYBE_ATOMIC.as_ref().map(|a| a.load(Ordering::Relaxed)) + MAYBE_ATOMIC.map(|a| a.load(Ordering::Relaxed)) } fn manipulate(i: i32) -> i32 { diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr new file mode 100644 index 000000000000..f93e4997af70 --- /dev/null +++ b/tests/ui/single_option_map.stderr @@ -0,0 +1,24 @@ +error: `fn` that only maps over argument + --> tests/ui/single_option_map.rs:8:1 + | +LL | / fn h(arg: Option) -> Option { +LL | | arg.map(|x| x * 2) +LL | | } + | |_^ + | + = help: move the `.map` to the caller or to an `_opt` function + = note: `-D clippy::single-option-map` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::single_option_map)]` + +error: `fn` that only maps over argument + --> tests/ui/single_option_map.rs:12:1 + | +LL | / fn j(arg: Option) -> Option { +LL | | arg.map(|x| x * 2) +LL | | } + | |_^ + | + = help: move the `.map` to the caller or to an `_opt` function + +error: aborting due to 2 previous errors + From 774b34652ff0b1700177f9257651e2f3024b5265 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Tue, 28 Jan 2025 21:35:05 +0100 Subject: [PATCH 07/10] Add comments over test cases that shouldn't trigger the lint Co-authored-by: Samuel Tardieu --- tests/ui/single_option_map.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index dd99f2350399..6dc54e975f9c 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -13,13 +13,16 @@ fn j(arg: Option) -> Option { arg.map(|x| x * 2) } +// No lint: no `Option` argument argument fn maps_static_option() -> Option { MAYBE_ATOMIC.map(|a| a.load(Ordering::Relaxed)) } +// No lint: wrapped by another function fn manipulate(i: i32) -> i32 { i + 1 } +// No lint: wraps another function to do the optional thing fn manipulate_opt(opt_i: Option) -> Option { opt_i.map(manipulate) } From e3d8cc512413a6d8ea9f1efcb293c4e1e9a72fe9 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Tue, 28 Jan 2025 22:05:33 +0100 Subject: [PATCH 08/10] Add markers to test cases that triggers the lint This ensures lints aren't accidentally lost during future refactoring, as they will be checked by uitest Co-authored-by: Samuel Tardieu --- tests/ui/single_option_map.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index 6dc54e975f9c..f13797b7579f 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -6,10 +6,12 @@ static ATOM: AtomicUsize = AtomicUsize::new(42); static MAYBE_ATOMIC: Option<&AtomicUsize> = Some(&ATOM); fn h(arg: Option) -> Option { + //~^ ERROR: `fn` that only maps over argument arg.map(|x| x * 2) } fn j(arg: Option) -> Option { + //~^ ERROR: `fn` that only maps over argument arg.map(|x| x * 2) } From d2dbd86c92057744875f7b955e894352624bc06c Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Tue, 28 Jan 2025 22:18:59 +0100 Subject: [PATCH 09/10] Add more test cases Co-authored-by: Samuel Tardieu --- tests/ui/single_option_map.rs | 11 +++++++++++ tests/ui/single_option_map.stderr | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index f13797b7579f..5b96bde4f71e 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -29,6 +29,17 @@ fn manipulate_opt(opt_i: Option) -> Option { opt_i.map(manipulate) } +// No lint: maps other than the receiver +fn map_not_arg(arg: Option) -> Option { + maps_static_option().map(|_| arg.unwrap()) +} + +// No lint: wrapper function with η-expanded form +#[allow(clippy::redundant_closure)] +fn manipulate_opt_explicit(opt_i: Option) -> Option { + opt_i.map(|x| manipulate(x)) +} + fn main() { let answer = Some(42u32); let h_result = h(answer); diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr index f93e4997af70..b4f366a9f797 100644 --- a/tests/ui/single_option_map.stderr +++ b/tests/ui/single_option_map.stderr @@ -2,6 +2,7 @@ error: `fn` that only maps over argument --> tests/ui/single_option_map.rs:8:1 | LL | / fn h(arg: Option) -> Option { +LL | | LL | | arg.map(|x| x * 2) LL | | } | |_^ @@ -11,14 +12,25 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::single_option_map)]` error: `fn` that only maps over argument - --> tests/ui/single_option_map.rs:12:1 + --> tests/ui/single_option_map.rs:13:1 | LL | / fn j(arg: Option) -> Option { +LL | | LL | | arg.map(|x| x * 2) LL | | } | |_^ | = help: move the `.map` to the caller or to an `_opt` function -error: aborting due to 2 previous errors +error: `fn` that only maps over argument + --> tests/ui/single_option_map.rs:39:1 + | +LL | / fn manipulate_opt_explicit(opt_i: Option) -> Option { +LL | | opt_i.map(|x| manipulate(x)) +LL | | } + | |_^ + | + = help: move the `.map` to the caller or to an `_opt` function + +error: aborting due to 3 previous errors From 40edb89f2d344c1fd6f01b665db137c031fcbbb7 Mon Sep 17 00:00:00 2001 From: Yusuf Raji Date: Wed, 5 Feb 2025 23:13:14 +0100 Subject: [PATCH 10/10] Lint closures that only bind other args with no other expressions --- clippy_lints/src/single_option_map.rs | 17 +++++++++++++++++ tests/ui/single_option_map.rs | 10 ++++++++++ tests/ui/single_option_map.stderr | 12 +----------- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/single_option_map.rs b/clippy_lints/src/single_option_map.rs index 7f69f33ff584..b4396501de91 100644 --- a/clippy_lints/src/single_option_map.rs +++ b/clippy_lints/src/single_option_map.rs @@ -58,8 +58,25 @@ impl<'tcx> LateLintPass<'tcx> for SingleOptionMap { && is_type_diagnostic_item(cx, callee_type, sym::Option) && let ExprKind::Path(_path) = callee.kind && let Res::Local(_id) = path_res(cx, callee) + && matches!(path_res(cx, callee), Res::Local(_id)) && !matches!(args[0].kind, ExprKind::Path(_)) { + if let ExprKind::Closure(closure) = args[0].kind { + let Body { params: [..], value } = cx.tcx.hir().body(closure.body); + if let ExprKind::Call(func, f_args) = value.kind + && matches!(func.kind, ExprKind::Path(_)) + && f_args.iter().all(|arg| matches!(arg.kind, ExprKind::Path(_))) + { + return; + } else if let ExprKind::MethodCall(_segment, receiver, method_args, _span) = value.kind + && matches!(receiver.kind, ExprKind::Path(_)) + && method_args.iter().all(|arg| matches!(arg.kind, ExprKind::Path(_))) + && method_args.iter().all(|arg| matches!(path_res(cx, arg), Res::Local(_))) + { + return; + } + } + span_lint_and_help( cx, SINGLE_OPTION_MAP, diff --git a/tests/ui/single_option_map.rs b/tests/ui/single_option_map.rs index 5b96bde4f71e..c55feace6f74 100644 --- a/tests/ui/single_option_map.rs +++ b/tests/ui/single_option_map.rs @@ -40,6 +40,16 @@ fn manipulate_opt_explicit(opt_i: Option) -> Option { opt_i.map(|x| manipulate(x)) } +// No lint +fn multi_args(a: String, b: bool, c: u64) -> String { + a +} + +// No lint: contains only map of a closure that binds other arguments +fn multi_args_opt(a: Option, b: bool, c: u64) -> Option { + a.map(|a| multi_args(a, b, c)) +} + fn main() { let answer = Some(42u32); let h_result = h(answer); diff --git a/tests/ui/single_option_map.stderr b/tests/ui/single_option_map.stderr index b4f366a9f797..8e41d2ce65fa 100644 --- a/tests/ui/single_option_map.stderr +++ b/tests/ui/single_option_map.stderr @@ -22,15 +22,5 @@ LL | | } | = help: move the `.map` to the caller or to an `_opt` function -error: `fn` that only maps over argument - --> tests/ui/single_option_map.rs:39:1 - | -LL | / fn manipulate_opt_explicit(opt_i: Option) -> Option { -LL | | opt_i.map(|x| manipulate(x)) -LL | | } - | |_^ - | - = help: move the `.map` to the caller or to an `_opt` function - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors