Auto merge of #11413 - jonboh:master, r=Alexendoo

new unnecessary_map_on_constructor lint

changelog: [`unnecessary_map_on_constructor`]: adds lint for cases in which map is not necessary. `Some(4).map(myfunction)` => `Some(myfunction(4))`

Closes https://github.com/rust-lang/rust-clippy/issues/6472

Note that the case mentioned in the issue `Some(..).and_then(|..| Some(..))` is fixed by a chain of lint changes. This PR completes the last part of that chain.

By `bind_instead_of_map`[lint](https://rust-lang.github.io/rust-clippy/master/index.html#/bind_instead_of_map):
`Some(4).and_then(|x| Some(foo(4)))` => `Some(4).map(|x| foo)`

By `redundant_closure` [lint](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure):
`Some(4).map(|x| foo)` => `Some(4).map(fun)`

Finally by this PR `unnecessary_map_on_constructor`:
`Some(4).map(fun)` => `Some(fun(4))`

I'm not sure this is the desired behavior for clippy and if it should be addressed in another issue/PR. I'd be up to give it a try if that's the case.
This commit is contained in:
bors 2023-09-12 16:09:06 +00:00
commit cb057019d4
16 changed files with 319 additions and 53 deletions

View file

@ -5437,6 +5437,7 @@ Released 2018-09-13
[`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`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_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_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_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_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 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

View file

@ -671,6 +671,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO, crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO, crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_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_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO, crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO, crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,

View file

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

View file

@ -0,0 +1,93 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::get_type_diagnostic_name;
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(i32::swap_bytes);
/// ```
/// Use instead:
/// ```rust
/// Some(i32::swap_bytes(4));
/// ```
#[clippy::version = "1.73.0"]
pub UNNECESSARY_MAP_ON_CONSTRUCTOR,
complexity,
"using `map`/`map_err` on `Option` or `Result` constructors"
}
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 Some(sym::Option | sym::Result) = get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(recv)){
let (constructor_path, constructor_item) =
if let hir::ExprKind::Call(constructor, constructor_args) = recv.kind
&& let hir::ExprKind::Path(constructor_path) = constructor.kind
&& let Some(arg) = constructor_args.get(0)
{
if constructor.span.from_expansion() || arg.span.from_expansion() {
return;
}
(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 let Some(map_arg) = args.get(0)
&& let hir::ExprKind::Path(fun) = map_arg.kind
{
if map_arg.span.from_expansion() {
return;
}
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);
span_lint_and_sugg(
cx,
UNNECESSARY_MAP_ON_CONSTRUCTOR,
expr.span,
&format!("unnecessary {} on constructor {constructor_snippet}(_)", path.ident.name),
"try",
format!("{constructor_snippet}({fun_snippet}({constructor_arg_snippet}))"),
applicability,
);
}
}
}
}

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -1,5 +1,5 @@
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:13:5 --> $DIR/manual_map_option.rs:14:5
| |
LL | / match Some(0) { LL | / match Some(0) {
LL | | Some(_) => Some(2), LL | | Some(_) => Some(2),
@ -11,7 +11,7 @@ LL | | };
= help: to override `-D warnings` add `#[allow(clippy::manual_map)]` = help: to override `-D warnings` add `#[allow(clippy::manual_map)]`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:18:5 --> $DIR/manual_map_option.rs:19:5
| |
LL | / match Some(0) { LL | / match Some(0) {
LL | | Some(x) => Some(x + 1), LL | | Some(x) => Some(x + 1),
@ -20,7 +20,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(|x| x + 1)` | |_____^ help: try: `Some(0).map(|x| x + 1)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:23:5 --> $DIR/manual_map_option.rs:24:5
| |
LL | / match Some("") { LL | / match Some("") {
LL | | Some(x) => Some(x.is_empty()), LL | | Some(x) => Some(x.is_empty()),
@ -29,7 +29,7 @@ LL | | };
| |_____^ help: try: `Some("").map(|x| x.is_empty())` | |_____^ help: try: `Some("").map(|x| x.is_empty())`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:28:5 --> $DIR/manual_map_option.rs:29:5
| |
LL | / if let Some(x) = Some(0) { LL | / if let Some(x) = Some(0) {
LL | | Some(!x) LL | | Some(!x)
@ -39,7 +39,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(|x| !x)` | |_____^ help: try: `Some(0).map(|x| !x)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:35:5 --> $DIR/manual_map_option.rs:36:5
| |
LL | / match Some(0) { LL | / match Some(0) {
LL | | Some(x) => { Some(std::convert::identity(x)) } LL | | Some(x) => { Some(std::convert::identity(x)) }
@ -48,7 +48,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(std::convert::identity)` | |_____^ help: try: `Some(0).map(std::convert::identity)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:40:5 --> $DIR/manual_map_option.rs:41:5
| |
LL | / match Some(&String::new()) { LL | / match Some(&String::new()) {
LL | | Some(x) => Some(str::len(x)), LL | | Some(x) => Some(str::len(x)),
@ -57,7 +57,7 @@ LL | | };
| |_____^ help: try: `Some(&String::new()).map(|x| str::len(x))` | |_____^ help: try: `Some(&String::new()).map(|x| str::len(x))`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:50:5 --> $DIR/manual_map_option.rs:51:5
| |
LL | / match &Some([0, 1]) { LL | / match &Some([0, 1]) {
LL | | Some(x) => Some(x[0]), LL | | Some(x) => Some(x[0]),
@ -66,7 +66,7 @@ LL | | };
| |_____^ help: try: `Some([0, 1]).as_ref().map(|x| x[0])` | |_____^ help: try: `Some([0, 1]).as_ref().map(|x| x[0])`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:55:5 --> $DIR/manual_map_option.rs:56:5
| |
LL | / match &Some(0) { LL | / match &Some(0) {
LL | | &Some(x) => Some(x * 2), LL | | &Some(x) => Some(x * 2),
@ -75,7 +75,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(|x| x * 2)` | |_____^ help: try: `Some(0).map(|x| x * 2)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:60:5 --> $DIR/manual_map_option.rs:61:5
| |
LL | / match Some(String::new()) { LL | / match Some(String::new()) {
LL | | Some(ref x) => Some(x.is_empty()), LL | | Some(ref x) => Some(x.is_empty()),
@ -84,7 +84,7 @@ LL | | };
| |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.is_empty())` | |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.is_empty())`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:65:5 --> $DIR/manual_map_option.rs:66:5
| |
LL | / match &&Some(String::new()) { LL | / match &&Some(String::new()) {
LL | | Some(x) => Some(x.len()), LL | | Some(x) => Some(x.len()),
@ -93,7 +93,7 @@ LL | | };
| |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.len())` | |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.len())`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:70:5 --> $DIR/manual_map_option.rs:71:5
| |
LL | / match &&Some(0) { LL | / match &&Some(0) {
LL | | &&Some(x) => Some(x + x), LL | | &&Some(x) => Some(x + x),
@ -102,7 +102,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(|x| x + x)` | |_____^ help: try: `Some(0).map(|x| x + x)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:83:9 --> $DIR/manual_map_option.rs:84:9
| |
LL | / match &mut Some(String::new()) { LL | / match &mut Some(String::new()) {
LL | | Some(x) => Some(x.push_str("")), LL | | Some(x) => Some(x.push_str("")),
@ -111,7 +111,7 @@ LL | | };
| |_________^ help: try: `Some(String::new()).as_mut().map(|x| x.push_str(""))` | |_________^ help: try: `Some(String::new()).as_mut().map(|x| x.push_str(""))`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:89:5 --> $DIR/manual_map_option.rs:90:5
| |
LL | / match &mut Some(String::new()) { LL | / match &mut Some(String::new()) {
LL | | Some(ref x) => Some(x.len()), LL | | Some(ref x) => Some(x.len()),
@ -120,7 +120,7 @@ LL | | };
| |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.len())` | |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.len())`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:94:5 --> $DIR/manual_map_option.rs:95:5
| |
LL | / match &mut &Some(String::new()) { LL | / match &mut &Some(String::new()) {
LL | | Some(x) => Some(x.is_empty()), LL | | Some(x) => Some(x.is_empty()),
@ -129,7 +129,7 @@ LL | | };
| |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.is_empty())` | |_____^ help: try: `Some(String::new()).as_ref().map(|x| x.is_empty())`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:99:5 --> $DIR/manual_map_option.rs:100:5
| |
LL | / match Some((0, 1, 2)) { LL | / match Some((0, 1, 2)) {
LL | | Some((x, y, z)) => Some(x + y + z), LL | | Some((x, y, z)) => Some(x + y + z),
@ -138,7 +138,7 @@ LL | | };
| |_____^ help: try: `Some((0, 1, 2)).map(|(x, y, z)| x + y + z)` | |_____^ help: try: `Some((0, 1, 2)).map(|(x, y, z)| x + y + z)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:104:5 --> $DIR/manual_map_option.rs:105:5
| |
LL | / match Some([1, 2, 3]) { LL | / match Some([1, 2, 3]) {
LL | | Some([first, ..]) => Some(first), LL | | Some([first, ..]) => Some(first),
@ -147,7 +147,7 @@ LL | | };
| |_____^ help: try: `Some([1, 2, 3]).map(|[first, ..]| first)` | |_____^ help: try: `Some([1, 2, 3]).map(|[first, ..]| first)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:109:5 --> $DIR/manual_map_option.rs:110:5
| |
LL | / match &Some((String::new(), "test")) { LL | / match &Some((String::new(), "test")) {
LL | | Some((x, y)) => Some((y, x)), LL | | Some((x, y)) => Some((y, x)),
@ -156,7 +156,7 @@ LL | | };
| |_____^ help: try: `Some((String::new(), "test")).as_ref().map(|(x, y)| (y, x))` | |_____^ help: try: `Some((String::new(), "test")).as_ref().map(|(x, y)| (y, x))`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:167:5 --> $DIR/manual_map_option.rs:168:5
| |
LL | / match Some(0) { LL | / match Some(0) {
LL | | Some(x) => Some(vec![x]), LL | | Some(x) => Some(vec![x]),
@ -165,7 +165,7 @@ LL | | };
| |_____^ help: try: `Some(0).map(|x| vec![x])` | |_____^ help: try: `Some(0).map(|x| vec![x])`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:172:5 --> $DIR/manual_map_option.rs:173:5
| |
LL | / match option_env!("") { LL | / match option_env!("") {
LL | | Some(x) => Some(String::from(x)), LL | | Some(x) => Some(String::from(x)),
@ -174,7 +174,7 @@ LL | | };
| |_____^ help: try: `option_env!("").map(String::from)` | |_____^ help: try: `option_env!("").map(String::from)`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:192:12 --> $DIR/manual_map_option.rs:193:12
| |
LL | } else if let Some(x) = Some(0) { LL | } else if let Some(x) = Some(0) {
| ____________^ | ____________^
@ -185,7 +185,7 @@ LL | | };
| |_____^ help: try: `{ Some(0).map(|x| x + 1) }` | |_____^ help: try: `{ Some(0).map(|x| x + 1) }`
error: manual implementation of `Option::map` error: manual implementation of `Option::map`
--> $DIR/manual_map_option.rs:200:12 --> $DIR/manual_map_option.rs:201:12
| |
LL | } else if let Some(x) = Some(0) { LL | } else if let Some(x) = Some(0) {
| ____________^ | ____________^

View file

@ -1,5 +1,5 @@
#![warn(clippy::option_filter_map)] #![warn(clippy::option_filter_map)]
#![allow(clippy::map_flatten)] #![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
fn main() { fn main() {
let _ = Some(Some(1)).flatten(); let _ = Some(Some(1)).flatten();

View file

@ -1,5 +1,5 @@
#![warn(clippy::option_filter_map)] #![warn(clippy::option_filter_map)]
#![allow(clippy::map_flatten)] #![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
fn main() { fn main() {
let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap); let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap);

View file

@ -1,6 +1,6 @@
#![warn(clippy::result_map_unit_fn)] #![warn(clippy::result_map_unit_fn)]
#![feature(never_type)] #![feature(never_type)]
#![allow(unused)] #![allow(unused, clippy::unnecessary_map_on_constructor)]
//@no-rustfix //@no-rustfix
struct HasResult { struct HasResult {
field: Result<usize, usize>, field: Result<usize, usize>,

View file

@ -0,0 +1,56 @@
#![allow(unused)]
#![warn(clippy::unnecessary_map_on_constructor)]
use std::ffi::OsStr;
fn fun(t: i32) -> i32 {
t
}
fn notfun(e: SimpleError) -> SimpleError {
e
}
macro_rules! expands_to_fun {
() => {
fun
};
}
#[derive(Copy, Clone)]
struct SimpleError {}
type SimpleResult = std::result::Result<i32, SimpleError>;
fn main() {
let x: i32 = 4;
let err = SimpleError {};
let a = Some(x);
let b: SimpleResult = Ok(x);
let c: SimpleResult = Err(err);
let a = Some(fun(x));
let b: SimpleResult = Ok(fun(x));
let c: SimpleResult = Err(notfun(err));
let a = Option::Some(fun(x));
let b: SimpleResult = SimpleResult::Ok(fun(x));
let c: SimpleResult = SimpleResult::Err(notfun(err));
let b: std::result::Result<i32, SimpleError> = Ok(fun(x));
let c: std::result::Result<i32, SimpleError> = Err(notfun(err));
let a = Some(fun(x));
let b: SimpleResult = Ok(fun(x));
let c: SimpleResult = Err(notfun(err));
// Should not trigger warning
a.map(fun);
b.map(fun);
c.map_err(notfun);
b.map_err(notfun); // Ok(_).map_err
c.map(fun); // Err(_).map()
option_env!("PATH").map(OsStr::new);
Some(x).map(expands_to_fun!());
}

View file

@ -0,0 +1,56 @@
#![allow(unused)]
#![warn(clippy::unnecessary_map_on_constructor)]
use std::ffi::OsStr;
fn fun(t: i32) -> i32 {
t
}
fn notfun(e: SimpleError) -> SimpleError {
e
}
macro_rules! expands_to_fun {
() => {
fun
};
}
#[derive(Copy, Clone)]
struct SimpleError {}
type SimpleResult = std::result::Result<i32, SimpleError>;
fn main() {
let x: i32 = 4;
let err = SimpleError {};
let a = Some(x);
let b: SimpleResult = Ok(x);
let c: SimpleResult = Err(err);
let a = Some(x).map(fun);
let b: SimpleResult = Ok(x).map(fun);
let c: SimpleResult = Err(err).map_err(notfun);
let a = Option::Some(x).map(fun);
let b: SimpleResult = SimpleResult::Ok(x).map(fun);
let c: SimpleResult = SimpleResult::Err(err).map_err(notfun);
let b: std::result::Result<i32, SimpleError> = Ok(x).map(fun);
let c: std::result::Result<i32, SimpleError> = Err(err).map_err(notfun);
let a = Some(fun(x));
let b: SimpleResult = Ok(fun(x));
let c: SimpleResult = Err(notfun(err));
// Should not trigger warning
a.map(fun);
b.map(fun);
c.map_err(notfun);
b.map_err(notfun); // Ok(_).map_err
c.map(fun); // Err(_).map()
option_env!("PATH").map(OsStr::new);
Some(x).map(expands_to_fun!());
}

View file

@ -0,0 +1,53 @@
error: unnecessary map on constructor Some(_)
--> $DIR/unnecessary_map_on_constructor.rs:32:13
|
LL | let a = Some(x).map(fun);
| ^^^^^^^^^^^^^^^^ help: try: `Some(fun(x))`
|
= note: `-D clippy::unnecessary-map-on-constructor` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_on_constructor)]`
error: unnecessary map on constructor Ok(_)
--> $DIR/unnecessary_map_on_constructor.rs:33:27
|
LL | let b: SimpleResult = Ok(x).map(fun);
| ^^^^^^^^^^^^^^ help: try: `Ok(fun(x))`
error: unnecessary map_err on constructor Err(_)
--> $DIR/unnecessary_map_on_constructor.rs:34:27
|
LL | let c: SimpleResult = Err(err).map_err(notfun);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Err(notfun(err))`
error: unnecessary map on constructor Option::Some(_)
--> $DIR/unnecessary_map_on_constructor.rs:36:13
|
LL | let a = Option::Some(x).map(fun);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Option::Some(fun(x))`
error: unnecessary map on constructor SimpleResult::Ok(_)
--> $DIR/unnecessary_map_on_constructor.rs:37:27
|
LL | let b: SimpleResult = SimpleResult::Ok(x).map(fun);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `SimpleResult::Ok(fun(x))`
error: unnecessary map_err on constructor SimpleResult::Err(_)
--> $DIR/unnecessary_map_on_constructor.rs:38:27
|
LL | let c: SimpleResult = SimpleResult::Err(err).map_err(notfun);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `SimpleResult::Err(notfun(err))`
error: unnecessary map on constructor Ok(_)
--> $DIR/unnecessary_map_on_constructor.rs:39:52
|
LL | let b: std::result::Result<i32, SimpleError> = Ok(x).map(fun);
| ^^^^^^^^^^^^^^ help: try: `Ok(fun(x))`
error: unnecessary map_err on constructor Err(_)
--> $DIR/unnecessary_map_on_constructor.rs:40:52
|
LL | let c: std::result::Result<i32, SimpleError> = Err(err).map_err(notfun);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Err(notfun(err))`
error: aborting due to 8 previous errors