new unnecessary_map_on_constructor lint

Author: jonboh

CHANGELOG.md

@@ -5433,6 +5433,7 @@ Released 2018-09-13
 [`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join
 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
 [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
+[`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

clippy_lints/src/declared_lints.rs

@@ -669,6 +669,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
     crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
     crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
+    crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
     crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
     crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
     crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,

clippy_lints/src/lib.rs

@@ -328,6 +328,7 @@ mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
 mod unnecessary_box_returns;
+mod unnecessary_map_on_constructor;
 mod unnecessary_owned_empty_strings;
 mod unnecessary_self_imports;
 mod unnecessary_struct_initialization;
@@ -1099,6 +1100,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns));
     store.register_late_pass(|_| Box::<reserve_after_initialization::ReserveAfterInitialization>::default());
     store.register_late_pass(|_| Box::new(implied_bounds_in_impls::ImpliedBoundsInImpls));
+    store.register_late_pass(|_| Box::new(unnecessary_map_on_constructor::UnnecessaryMapOnConstructor));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/unnecessary_map_on_constructor.rs

@@ -0,0 +1,96 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Suggest removing the use of a may (or map_err) method when an Option or Result is being construted.
+    /// ### Why is this bad?
+    /// It introduces unnecessary complexity. In this case the function can be used directly and
+    /// construct the Option or Result from the output.
+    ///
+    /// ### Example
+    /// ```rust
+    /// Some(4).map(foo)
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// Some(foo(4))
+    /// ```
+    #[clippy::version = "1.73.0"]
+    pub UNNECESSARY_MAP_ON_CONSTRUCTOR,
+    complexity,
+    "Suggest removing usage of map in Option or Result construction"
+}
+declare_lint_pass!(UnnecessaryMapOnConstructor => [UNNECESSARY_MAP_ON_CONSTRUCTOR]);
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryMapOnConstructor {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+        if let hir::ExprKind::MethodCall(path, recv, args, ..) = expr.kind {
+            let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option);
+            let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
+
+            if let (false, false) = (is_option, is_result) {
+                return;
+            };
+
+            let (constructor_path, constructor_item) = if_chain! {
+                if let hir::ExprKind::Call(constructor, constructor_args) = recv.kind;
+                if let hir::ExprKind::Path(constructor_path) = constructor.kind;
+                if let Some(arg) = constructor_args.get(0);
+                then {
+                    (constructor_path, arg)
+                } else {
+                    return;
+                }
+            };
+            let constructor_symbol = match constructor_path {
+                hir::QPath::Resolved(_, path) => {
+                    if let Some(path_segment) = path.segments.last() {
+                        path_segment.ident.name
+                    } else {
+                        return;
+                    }
+                },
+                hir::QPath::TypeRelative(_, path) => path.ident.name,
+                hir::QPath::LangItem(_, _, _) => return,
+            };
+            match constructor_symbol {
+                sym::Some | sym::Ok if path.ident.name == rustc_span::sym::map => (),
+                sym::Err if path.ident.name == sym!(map_err) => (),
+                _ => return,
+            }
+
+            if_chain! {
+                if let Some(arg) = args.get(0);
+                if let hir::ExprKind::Path(fun) = arg.kind;
+                then {
+                    let mut applicability = Applicability::MachineApplicable;
+                    let fun_snippet = snippet_with_applicability(cx, fun.span(), "_", &mut applicability);
+                    let constructor_snippet =
+                        snippet_with_applicability(cx, constructor_path.span(), "_", &mut applicability);
+                    let constructor_arg_snippet =
+                        snippet_with_applicability(cx, constructor_item.span, "_", &mut applicability);
+                    let suggestion = format!("{constructor_snippet}({fun_snippet}({constructor_arg_snippet}))");
+                    span_lint_and_sugg(
+                        cx,
+                        UNNECESSARY_MAP_ON_CONSTRUCTOR,
+                        expr.span,
+                        &format!("unnecessary {fun_snippet} on contstuctor {constructor_snippet}(_)"),
+                        "try using",
+                        suggestion,
+                        applicability,
+                    );
+                }
+            }
+        }
+    }
+}

tests/ui/eta.fixed

@@ -7,7 +7,8 @@
     clippy::option_map_unit_fn,
     clippy::redundant_closure_call,
     clippy::uninlined_format_args,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::unnecessary_map_on_constructor
 )]
 
 use std::path::{Path, PathBuf};

tests/ui/eta.rs

@@ -7,7 +7,8 @@
     clippy::option_map_unit_fn,
     clippy::redundant_closure_call,
     clippy::uninlined_format_args,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::unnecessary_map_on_constructor
 )]
 
 use std::path::{Path, PathBuf};

tests/ui/eta.stderr

@@ -1,165 +1,165 @@
 error: redundant closure
-  --> $DIR/eta.rs:28:27
+  --> $DIR/eta.rs:29:27
    |
 LL |     let a = Some(1u8).map(|a| foo(a));
    |                           ^^^^^^^^^^ help: replace the closure with the function itself: `foo`
    |
    = note: `-D clippy::redundant-closure` implied by `-D warnings`
 
 error: redundant closure
-  --> $DIR/eta.rs:32:40
+  --> $DIR/eta.rs:33:40
    |
 LL |     let _: Option<Vec<u8>> = true.then(|| vec![]); // special case vec!
    |                                        ^^^^^^^^^ help: replace the closure with `Vec::new`: `std::vec::Vec::new`
 
 error: redundant closure
-  --> $DIR/eta.rs:33:35
+  --> $DIR/eta.rs:34:35
    |
 LL |     let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted?
    |                                   ^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo2`
 
 error: redundant closure
-  --> $DIR/eta.rs:34:26
+  --> $DIR/eta.rs:35:26
    |
 LL |     all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
    |                          ^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `below`
 
 error: redundant closure
-  --> $DIR/eta.rs:41:27
+  --> $DIR/eta.rs:42:27
    |
 LL |     let e = Some(1u8).map(|a| generic(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic`
 
 error: redundant closure
-  --> $DIR/eta.rs:93:51
+  --> $DIR/eta.rs:94:51
    |
 LL |     let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
    |                                                   ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo`
    |
    = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings`
 
 error: redundant closure
-  --> $DIR/eta.rs:94:51
+  --> $DIR/eta.rs:95:51
    |
 LL |     let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo());
    |                                                   ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo`
 
 error: redundant closure
-  --> $DIR/eta.rs:96:42
+  --> $DIR/eta.rs:97:42
    |
 LL |     let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear());
    |                                          ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear`
 
 error: redundant closure
-  --> $DIR/eta.rs:100:29
+  --> $DIR/eta.rs:101:29
    |
 LL |     let e = Some("str").map(|s| s.to_string());
    |                             ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string`
 
 error: redundant closure
-  --> $DIR/eta.rs:101:27
+  --> $DIR/eta.rs:102:27
    |
 LL |     let e = Some('a').map(|s| s.to_uppercase());
    |                           ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase`
 
 error: redundant closure
-  --> $DIR/eta.rs:103:65
+  --> $DIR/eta.rs:104:65
    |
 LL |     let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect();
    |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase`
 
 error: redundant closure
-  --> $DIR/eta.rs:166:22
+  --> $DIR/eta.rs:167:22
    |
 LL |     requires_fn_once(|| x());
    |                      ^^^^^^ help: replace the closure with the function itself: `x`
 
 error: redundant closure
-  --> $DIR/eta.rs:173:27
+  --> $DIR/eta.rs:174:27
    |
 LL |     let a = Some(1u8).map(|a| foo_ptr(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
 
 error: redundant closure
-  --> $DIR/eta.rs:178:27
+  --> $DIR/eta.rs:179:27
    |
 LL |     let a = Some(1u8).map(|a| closure(a));
    |                           ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure`
 
 error: redundant closure
-  --> $DIR/eta.rs:210:28
+  --> $DIR/eta.rs:211:28
    |
 LL |     x.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
 
 error: redundant closure
-  --> $DIR/eta.rs:211:28
+  --> $DIR/eta.rs:212:28
    |
 LL |     y.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res`
 
 error: redundant closure
-  --> $DIR/eta.rs:212:28
+  --> $DIR/eta.rs:213:28
    |
 LL |     z.into_iter().for_each(|x| add_to_res(x));
    |                            ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res`
 
 error: redundant closure
-  --> $DIR/eta.rs:219:21
+  --> $DIR/eta.rs:220:21
    |
 LL |         Some(1).map(|n| closure(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
 
 error: redundant closure
-  --> $DIR/eta.rs:223:21
+  --> $DIR/eta.rs:224:21
    |
 LL |         Some(1).map(|n| in_loop(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
 
 error: redundant closure
-  --> $DIR/eta.rs:316:18
+  --> $DIR/eta.rs:317:18
    |
 LL |     takes_fn_mut(|| f());
    |                  ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> $DIR/eta.rs:319:19
+  --> $DIR/eta.rs:320:19
    |
 LL |     takes_fn_once(|| f());
    |                   ^^^^^^ help: replace the closure with the function itself: `&mut f`
 
 error: redundant closure
-  --> $DIR/eta.rs:323:26
+  --> $DIR/eta.rs:324:26
    |
 LL |     move || takes_fn_mut(|| f_used_once())
    |                          ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
 
 error: redundant closure
-  --> $DIR/eta.rs:335:19
+  --> $DIR/eta.rs:336:19
    |
 LL |     array_opt.map(|a| a.as_slice());
    |                   ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
 
 error: redundant closure
-  --> $DIR/eta.rs:338:19
+  --> $DIR/eta.rs:339:19
    |
 LL |     slice_opt.map(|s| s.len());
    |                   ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
 
 error: redundant closure
-  --> $DIR/eta.rs:341:17
+  --> $DIR/eta.rs:342:17
    |
 LL |     ptr_opt.map(|p| p.is_null());
    |                 ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
 
 error: redundant closure
-  --> $DIR/eta.rs:345:17
+  --> $DIR/eta.rs:346:17
    |
 LL |     dyn_opt.map(|d| d.method_on_dyn());
    |                 ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<dyn TestTrait>::method_on_dyn`
 
 error: redundant closure
-  --> $DIR/eta.rs:388:19
+  --> $DIR/eta.rs:389:19
    |
 LL |     let _ = f(&0, |x, y| f2(x, y));
    |                   ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2`

tests/ui/manual_map_option.fixed

@@ -5,6 +5,7 @@
     clippy::unit_arg,
     clippy::match_ref_pats,
     clippy::redundant_pattern_matching,
+    clippy::unnecessary_map_on_constructor,
     for_loops_over_fallibles,
     dead_code
 )]

tests/ui/manual_map_option.rs

@@ -5,6 +5,7 @@
     clippy::unit_arg,
     clippy::match_ref_pats,
     clippy::redundant_pattern_matching,
+    clippy::unnecessary_map_on_constructor,
     for_loops_over_fallibles,
     dead_code
 )]