Add single_option_map lint (#14033)

Checks for functions with method calls to `.map(_)` on an arg of type
`Option` as the outermost expression.

Fixes #774
```
changelog: [`single_option_map`]: Checks for functions with method calls to `.map(_)` on an arg of type `Option` as the outermost expression.
```

Author: llogiq

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

clippy_lints/src/declared_lints.rs

@@ -683,6 +683,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,

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`
 }

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

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

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
+