diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/single_option_map.rs | 91 | ||||
| -rw-r--r-- | tests/ui/single_option_map.rs | 69 | ||||
| -rw-r--r-- | tests/ui/single_option_map.stderr | 37 |
6 files changed, 201 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index fa03c953aa5..ea1119aca98 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 9fbeab5bf2e..3d4ca4faecb 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 8887ab7ec0d..53a05a5a558 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; @@ -978,5 +979,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::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 new file mode 100644 index 00000000000..ad8463870a6 --- /dev/null +++ b/clippy_lints/src/single_option_map.rs @@ -0,0 +1,91 @@ +use clippy_utils::diagnostics::span_lint_and_help; +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}; +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<T>` may require additional + /// `Some(_)` and `unwrap` if all you have is a `T`. + /// + /// ### Example + /// ```no_run + /// fn double(param: Option<u32>) -> Option<u32> { + /// 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) + && 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, + span, + "`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.rs b/tests/ui/single_option_map.rs new file mode 100644 index 00000000000..571beec5479 --- /dev/null +++ b/tests/ui/single_option_map.rs @@ -0,0 +1,69 @@ +#![warn(clippy::single_option_map)] + +use std::sync::atomic::{AtomicUsize, Ordering}; + +static ATOM: AtomicUsize = AtomicUsize::new(42); +static MAYBE_ATOMIC: Option<&AtomicUsize> = Some(&ATOM); + +fn h(arg: Option<u32>) -> Option<u32> { + //~^ ERROR: `fn` that only maps over argument + arg.map(|x| x * 2) +} + +fn j(arg: Option<u64>) -> Option<u64> { + //~^ ERROR: `fn` that only maps over argument + arg.map(|x| x * 2) +} + +fn mul_args(a: String, b: u64) -> String { + a +} + +fn mul_args_opt(a: Option<String>, b: u64) -> Option<String> { + //~^ ERROR: `fn` that only maps over argument + a.map(|val| mul_args(val, b + 1)) +} + +// No lint: no `Option` argument argument +fn maps_static_option() -> Option<usize> { + 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<i32>) -> Option<i32> { + opt_i.map(manipulate) +} + +// No lint: maps other than the receiver +fn map_not_arg(arg: Option<u32>) -> Option<u32> { + maps_static_option().map(|_| arg.unwrap()) +} + +// No lint: wrapper function with η-expanded form +#[allow(clippy::redundant_closure)] +fn manipulate_opt_explicit(opt_i: Option<i32>) -> Option<i32> { + 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<String>, b: bool, c: u64) -> Option<String> { + a.map(|a| multi_args(a, b, c)) +} + +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 new file mode 100644 index 00000000000..f7d48eba71e --- /dev/null +++ b/tests/ui/single_option_map.stderr @@ -0,0 +1,37 @@ +error: `fn` that only maps over argument + --> tests/ui/single_option_map.rs:8:1 + | +LL | / fn h(arg: Option<u32>) -> Option<u32> { +LL | | +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:13:1 + | +LL | / fn j(arg: Option<u64>) -> Option<u64> { +LL | | +LL | | arg.map(|x| x * 2) +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:22:1 + | +LL | / fn mul_args_opt(a: Option<String>, b: u64) -> Option<String> { +LL | | +LL | | a.map(|val| mul_args(val, b + 1)) +LL | | } + | |_^ + | + = help: move the `.map` to the caller or to an `_opt` function + +error: aborting due to 3 previous errors + |