diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b62c9a59aa5..ede6f7682b4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6292,6 +6292,7 @@ Released 2018-09-13 [`unnecessary_min_or_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_or_max [`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_option_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_option_map_or_else [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings [`unnecessary_result_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_result_map_or_else [`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2cccd6ba2702..a0d34b35ad7d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -477,6 +477,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO, crate::methods::UNNECESSARY_MAP_OR_INFO, crate::methods::UNNECESSARY_MIN_OR_MAX_INFO, + crate::methods::UNNECESSARY_OPTION_MAP_OR_ELSE_INFO, crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO, crate::methods::UNNECESSARY_SORT_BY_INFO, crate::methods::UNNECESSARY_TO_OWNED_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index ad374dee516c..9c731dac35e2 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -130,6 +130,7 @@ mod unnecessary_lazy_eval; mod unnecessary_literal_unwrap; mod unnecessary_map_or; mod unnecessary_min_or_max; +mod unnecessary_option_map_or_else; mod unnecessary_result_map_or_else; mod unnecessary_sort_by; mod unnecessary_to_owned; @@ -4529,6 +4530,31 @@ declare_clippy_lint! { "detect swap with a temporary value" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `.map_or_else()` "map closure" for `Option` type. + /// + /// ### Why is this bad? + /// This can be written more concisely by using `unwrap_or_else()`. + /// + /// ### Example + /// ```no_run + /// let k = 10; + /// let x: Option = Some(4); + /// let y = x.map_or_else(|| 2 * k, |n| n); + /// ``` + /// Use instead: + /// ```no_run + /// let k = 10; + /// let x: Option = Some(4); + /// let y = x.unwrap_or_else(|| 2 * k); + /// ``` + #[clippy::version = "1.88.0"] + pub UNNECESSARY_OPTION_MAP_OR_ELSE, + suspicious, + "making no use of the \"map closure\" when calling `.map_or_else(|| 2 * k, |n| n)`" +} + #[expect(clippy::struct_excessive_bools)] pub struct Methods { avoid_breaking_exported_api: bool, @@ -4707,6 +4733,7 @@ impl_lint_pass!(Methods => [ MANUAL_CONTAINS, IO_OTHER_ERROR, SWAP_WITH_TEMPORARY, + UNNECESSARY_OPTION_MAP_OR_ELSE, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -5262,6 +5289,7 @@ impl Methods { }, ("map_or_else", [def, map]) => { result_map_or_else_none::check(cx, expr, recv, def, map); + unnecessary_option_map_or_else::check(cx, expr, recv, def, map); unnecessary_result_map_or_else::check(cx, expr, recv, def, map); }, ("next", []) => { diff --git a/clippy_lints/src/methods/unnecessary_option_map_or_else.rs b/clippy_lints/src/methods/unnecessary_option_map_or_else.rs new file mode 100644 index 000000000000..ec709e1ab5cd --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_option_map_or_else.rs @@ -0,0 +1,73 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::peel_blocks; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_errors::Applicability; +use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::UNNECESSARY_OPTION_MAP_OR_ELSE; +use super::utils::get_last_chain_binding_hir_id; + +fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>) { + let msg = "unused \"map closure\" when calling `Option::map_or_else` value"; + let self_snippet = snippet(cx, recv.span, ".."); + let err_snippet = snippet(cx, def_arg.span, ".."); + span_lint_and_sugg( + cx, + UNNECESSARY_OPTION_MAP_OR_ELSE, + expr.span, + msg, + "consider using `unwrap_or_else`", + format!("{self_snippet}.unwrap_or_else({err_snippet})"), + Applicability::MachineApplicable, + ); +} + +fn handle_qpath( + cx: &LateContext<'_>, + expr: &Expr<'_>, + recv: &Expr<'_>, + def_arg: &Expr<'_>, + expected_hir_id: HirId, + qpath: QPath<'_>, +) { + if let QPath::Resolved(_, path) = qpath + && let rustc_hir::def::Res::Local(hir_id) = path.res + && expected_hir_id == hir_id + { + emit_lint(cx, expr, recv, def_arg); + } +} + +/// lint use of `_.map_or_else(|err| err, |n| n)` for `Option`s. +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, map_arg: &Expr<'_>) { + // lint if the caller of `map_or_else()` is an `Option` + if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option) + && let ExprKind::Closure(&Closure { body, .. }) = map_arg.kind + && let body = cx.tcx.hir_body(body) + && let Some(first_param) = body.params.first() + { + let body_expr = peel_blocks(body.value); + + match body_expr.kind { + ExprKind::Path(qpath) => { + handle_qpath(cx, expr, recv, def_arg, first_param.pat.hir_id, qpath); + }, + // If this is a block (that wasn't peeled off), then it means there are statements. + ExprKind::Block(block, _) => { + if let Some(block_expr) = block.expr + // First we ensure that this is a "binding chain" (each statement is a binding + // of the previous one) and that it is a binding of the closure argument. + && let Some(last_chain_binding_id) = + get_last_chain_binding_hir_id(first_param.pat.hir_id, block.stmts) + && let ExprKind::Path(qpath) = block_expr.kind + { + handle_qpath(cx, expr, recv, def_arg, last_chain_binding_id, qpath); + } + }, + _ => {}, + } + } +} diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index fe3ac9e8f92c..843cb78df787 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -6,7 +6,8 @@ clippy::redundant_locals, clippy::manual_midpoint, clippy::manual_unwrap_or_default, - clippy::manual_unwrap_or + clippy::manual_unwrap_or, + clippy::unnecessary_option_map_or_else )] fn bad1(string: Option<&str>) -> (bool, &str) { diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index 5b7498bc8e23..65f92ab4ad9b 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -6,7 +6,8 @@ clippy::redundant_locals, clippy::manual_midpoint, clippy::manual_unwrap_or_default, - clippy::manual_unwrap_or + clippy::manual_unwrap_or, + clippy::unnecessary_option_map_or_else )] fn bad1(string: Option<&str>) -> (bool, &str) { diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index 9eb41f81a539..48c89452e7f2 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -1,5 +1,5 @@ error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:13:5 + --> tests/ui/option_if_let_else.rs:14:5 | LL | / if let Some(x) = string { LL | | @@ -13,19 +13,19 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::option_if_let_else)]` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:32:13 + --> tests/ui/option_if_let_else.rs:33:13 | LL | let _ = if let Some(s) = *string { s.len() } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:34:13 + --> tests/ui/option_if_let_else.rs:35:13 | LL | let _ = if let Some(s) = &num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:36:13 + --> tests/ui/option_if_let_else.rs:37:13 | LL | let _ = if let Some(s) = &mut num { | _____________^ @@ -47,13 +47,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:43:13 + --> tests/ui/option_if_let_else.rs:44:13 | LL | let _ = if let Some(ref s) = num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:45:13 + --> tests/ui/option_if_let_else.rs:46:13 | LL | let _ = if let Some(mut s) = num { | _____________^ @@ -75,7 +75,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:52:13 + --> tests/ui/option_if_let_else.rs:53:13 | LL | let _ = if let Some(ref mut s) = num { | _____________^ @@ -97,7 +97,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:62:5 + --> tests/ui/option_if_let_else.rs:63:5 | LL | / if let Some(x) = arg { LL | | @@ -118,7 +118,7 @@ LL + }) | error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:76:13 + --> tests/ui/option_if_let_else.rs:77:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -131,7 +131,7 @@ LL | | }; | |_____^ help: try: `arg.map_or_else(side_effect, |x| x)` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:86:13 + --> tests/ui/option_if_let_else.rs:87:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -154,7 +154,7 @@ LL ~ }, |x| x * x * x * x); | error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:120:13 + --> tests/ui/option_if_let_else.rs:121:13 | LL | / if let Some(idx) = s.find('.') { LL | | @@ -165,7 +165,7 @@ LL | | } | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:132:5 + --> tests/ui/option_if_let_else.rs:133:5 | LL | / if let Ok(binding) = variable { LL | | @@ -189,13 +189,13 @@ LL + }) | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:157:13 + --> tests/ui/option_if_let_else.rs:158:13 | LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:168:13 + --> tests/ui/option_if_let_else.rs:169:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -217,13 +217,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:197:13 + --> tests/ui/option_if_let_else.rs:198:13 | LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:202:13 + --> tests/ui/option_if_let_else.rs:203:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -245,7 +245,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:242:13 + --> tests/ui/option_if_let_else.rs:243:13 | LL | let _ = match s { | _____________^ @@ -256,7 +256,7 @@ LL | | }; | |_____^ help: try: `s.map_or(1, |string| string.len())` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:247:13 + --> tests/ui/option_if_let_else.rs:248:13 | LL | let _ = match Some(10) { | _____________^ @@ -267,7 +267,7 @@ LL | | }; | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:254:13 + --> tests/ui/option_if_let_else.rs:255:13 | LL | let _ = match res { | _____________^ @@ -278,7 +278,7 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:259:13 + --> tests/ui/option_if_let_else.rs:260:13 | LL | let _ = match res { | _____________^ @@ -289,13 +289,13 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:264:13 + --> tests/ui/option_if_let_else.rs:265:13 | LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:282:17 + --> tests/ui/option_if_let_else.rs:283:17 | LL | let _ = match initial { | _________________^ @@ -306,7 +306,7 @@ LL | | }; | |_________^ help: try: `initial.as_ref().map_or(42, |value| do_something(value))` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:290:17 + --> tests/ui/option_if_let_else.rs:291:17 | LL | let _ = match initial { | _________________^ @@ -317,7 +317,7 @@ LL | | }; | |_________^ help: try: `initial.as_mut().map_or(42, |value| do_something2(value))` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:314:24 + --> tests/ui/option_if_let_else.rs:315:24 | LL | let mut _hashmap = if let Some(hm) = &opt { | ________________________^ @@ -329,7 +329,7 @@ LL | | }; | |_____^ help: try: `opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone())` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:321:19 + --> tests/ui/option_if_let_else.rs:322:19 | LL | let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())` diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index a1119d75c231..698a9c9a5cf6 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -5,6 +5,7 @@ clippy::uninlined_format_args, clippy::unnecessary_wraps, clippy::unnecessary_literal_unwrap, + clippy::unnecessary_option_map_or_else, clippy::useless_vec )] diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index a7cd632bf166..28baf01f0753 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -5,6 +5,7 @@ clippy::uninlined_format_args, clippy::unnecessary_wraps, clippy::unnecessary_literal_unwrap, + clippy::unnecessary_option_map_or_else, clippy::useless_vec )] diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index 35bda7e4d331..c5e6d6230d1d 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -1,5 +1,5 @@ error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:52:22 + --> tests/ui/or_fun_call.rs:53:22 | LL | with_constructor.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(make)` @@ -8,7 +8,7 @@ LL | with_constructor.unwrap_or(make()); = help: to override `-D warnings` add `#[allow(clippy::or_fun_call)]` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:56:14 + --> tests/ui/or_fun_call.rs:57:14 | LL | with_new.unwrap_or(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` @@ -17,199 +17,199 @@ LL | with_new.unwrap_or(Vec::new()); = help: to override `-D warnings` add `#[allow(clippy::unwrap_or_default)]` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:60:21 + --> tests/ui/or_fun_call.rs:61:21 | LL | with_const_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Vec::with_capacity(12))` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:64:14 + --> tests/ui/or_fun_call.rs:65:14 | LL | with_err.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|_| make())` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:68:19 + --> tests/ui/or_fun_call.rs:69:19 | LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:72:24 + --> tests/ui/or_fun_call.rs:73:24 | LL | with_default_trait.unwrap_or(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:76:23 + --> tests/ui/or_fun_call.rs:77:23 | LL | with_default_type.unwrap_or(u64::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:80:18 + --> tests/ui/or_fun_call.rs:81:18 | LL | self_default.unwrap_or(::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(::default)` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:84:18 + --> tests/ui/or_fun_call.rs:85:18 | LL | real_default.unwrap_or(::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:88:14 + --> tests/ui/or_fun_call.rs:89:14 | LL | with_vec.unwrap_or(vec![]); | ^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:92:21 + --> tests/ui/or_fun_call.rs:93:21 | LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(Foo::new)` error: use of `or_insert` to construct default value - --> tests/ui/or_fun_call.rs:96:19 + --> tests/ui/or_fun_call.rs:97:19 | LL | map.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `or_insert` to construct default value - --> tests/ui/or_fun_call.rs:100:23 + --> tests/ui/or_fun_call.rs:101:23 | LL | map_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `or_insert` to construct default value - --> tests/ui/or_fun_call.rs:104:21 + --> tests/ui/or_fun_call.rs:105:21 | LL | btree.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `or_insert` to construct default value - --> tests/ui/or_fun_call.rs:108:25 + --> tests/ui/or_fun_call.rs:109:25 | LL | btree_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:112:21 + --> tests/ui/or_fun_call.rs:113:21 | LL | let _ = stringy.unwrap_or(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: function call inside of `ok_or` - --> tests/ui/or_fun_call.rs:117:17 + --> tests/ui/or_fun_call.rs:118:17 | LL | let _ = opt.ok_or(format!("{} world.", hello)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ok_or_else(|| format!("{} world.", hello))` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:122:21 + --> tests/ui/or_fun_call.rs:123:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| map[&1])` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:125:21 + --> tests/ui/or_fun_call.rs:126:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| map[&1])` error: function call inside of `or` - --> tests/ui/or_fun_call.rs:150:35 + --> tests/ui/or_fun_call.rs:151:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_else(|| Some("b".to_string()))` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:193:18 + --> tests/ui/or_fun_call.rs:194:18 | LL | None.unwrap_or(ptr_to_ref(s)); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| ptr_to_ref(s))` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:201:14 + --> tests/ui/or_fun_call.rs:202:14 | LL | None.unwrap_or(unsafe { ptr_to_ref(s) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:204:14 + --> tests/ui/or_fun_call.rs:205:14 | LL | None.unwrap_or( unsafe { ptr_to_ref(s) } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })` error: function call inside of `map_or` - --> tests/ui/or_fun_call.rs:280:25 + --> tests/ui/or_fun_call.rs:281:25 | LL | let _ = Some(4).map_or(g(), |v| v); | ^^^^^^^^^^^^^^^^^^ help: try: `map_or_else(g, |v| v)` error: function call inside of `map_or` - --> tests/ui/or_fun_call.rs:282:25 + --> tests/ui/or_fun_call.rs:283:25 | LL | let _ = Some(4).map_or(g(), f); | ^^^^^^^^^^^^^^ help: try: `map_or_else(g, f)` error: use of `unwrap_or_else` to construct default value - --> tests/ui/or_fun_call.rs:314:18 + --> tests/ui/or_fun_call.rs:315:18 | LL | with_new.unwrap_or_else(Vec::new); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `unwrap_or_else` to construct default value - --> tests/ui/or_fun_call.rs:318:28 + --> tests/ui/or_fun_call.rs:319:28 | LL | with_default_trait.unwrap_or_else(Default::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `unwrap_or_else` to construct default value - --> tests/ui/or_fun_call.rs:322:27 + --> tests/ui/or_fun_call.rs:323:27 | LL | with_default_type.unwrap_or_else(u64::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `unwrap_or_else` to construct default value - --> tests/ui/or_fun_call.rs:326:22 + --> tests/ui/or_fun_call.rs:327:22 | LL | real_default.unwrap_or_else(::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: use of `or_insert_with` to construct default value - --> tests/ui/or_fun_call.rs:330:23 + --> tests/ui/or_fun_call.rs:331:23 | LL | map.entry(42).or_insert_with(String::new); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `or_insert_with` to construct default value - --> tests/ui/or_fun_call.rs:334:25 + --> tests/ui/or_fun_call.rs:335:25 | LL | btree.entry(42).or_insert_with(String::new); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()` error: use of `unwrap_or_else` to construct default value - --> tests/ui/or_fun_call.rs:338:25 + --> tests/ui/or_fun_call.rs:339:25 | LL | let _ = stringy.unwrap_or_else(String::new); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:380:17 + --> tests/ui/or_fun_call.rs:381:17 | LL | let _ = opt.unwrap_or({ f() }); // suggest `.unwrap_or_else(f)` | ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(f)` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:385:17 + --> tests/ui/or_fun_call.rs:386:17 | LL | let _ = opt.unwrap_or(f() + 1); // suggest `.unwrap_or_else(|| f() + 1)` | ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| f() + 1)` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:390:17 + --> tests/ui/or_fun_call.rs:391:17 | LL | let _ = opt.unwrap_or({ | _________________^ @@ -229,19 +229,19 @@ LL ~ }); | error: function call inside of `map_or` - --> tests/ui/or_fun_call.rs:396:17 + --> tests/ui/or_fun_call.rs:397:17 | LL | let _ = opt.map_or(f() + 1, |v| v); // suggest `.map_or_else(|| f() + 1, |v| v)` | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `map_or_else(|| f() + 1, |v| v)` error: use of `unwrap_or` to construct default value - --> tests/ui/or_fun_call.rs:401:17 + --> tests/ui/or_fun_call.rs:402:17 | LL | let _ = opt.unwrap_or({ i32::default() }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()` error: function call inside of `unwrap_or` - --> tests/ui/or_fun_call.rs:408:21 + --> tests/ui/or_fun_call.rs:409:21 | LL | let _ = opt_foo.unwrap_or(Foo { val: String::default() }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Foo { val: String::default() })` diff --git a/tests/ui/unnecessary_option_map_or_else.fixed b/tests/ui/unnecessary_option_map_or_else.fixed new file mode 100644 index 000000000000..5f0039f9fea5 --- /dev/null +++ b/tests/ui/unnecessary_option_map_or_else.fixed @@ -0,0 +1,47 @@ +#![warn(clippy::unnecessary_option_map_or_else)] +#![allow( + clippy::let_and_return, + clippy::let_unit_value, + clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_literal_unwrap +)] + +fn main() { + // Expected errors + // Basic scenario + let option = Some(()); + option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling + + // Type ascription + let option = Some(()); + option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling + + // Auto-deref + let string = String::new(); + let option = Some(&string); + let _: &str = option.unwrap_or_else(|| &string); //~ ERROR: unused "map closure" when calling + + // Temporary variable + let option = Some(()); + option.unwrap_or_else(|| ()); + + // Correct usages + let option = Some(()); + option.map_or_else(|| (), |_| ()); + + let option = Some(()); + option.map_or_else(|| (), |_: ()| ()); + + let string = String::new(); + let option = Some(&string); + let _: &str = option.map_or_else(|| &string, |_| &string); + + let option = Some(()); + option.map_or_else( + || (), + |_| { + let tmp = (); + tmp + }, + ); +} diff --git a/tests/ui/unnecessary_option_map_or_else.rs b/tests/ui/unnecessary_option_map_or_else.rs new file mode 100644 index 000000000000..a748b3301814 --- /dev/null +++ b/tests/ui/unnecessary_option_map_or_else.rs @@ -0,0 +1,54 @@ +#![warn(clippy::unnecessary_option_map_or_else)] +#![allow( + clippy::let_and_return, + clippy::let_unit_value, + clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_literal_unwrap +)] + +fn main() { + // Expected errors + // Basic scenario + let option = Some(()); + option.map_or_else(|| (), |x| x); //~ ERROR: unused "map closure" when calling + + // Type ascription + let option = Some(()); + option.map_or_else(|| (), |x: ()| x); //~ ERROR: unused "map closure" when calling + + // Auto-deref + let string = String::new(); + let option = Some(&string); + let _: &str = option.map_or_else(|| &string, |x| x); //~ ERROR: unused "map closure" when calling + + // Temporary variable + let option = Some(()); + option.map_or_else( + //~^ ERROR: unused "map closure" when calling + || (), + |x| { + let tmp = x; + tmp + }, + ); + + // Correct usages + let option = Some(()); + option.map_or_else(|| (), |_| ()); + + let option = Some(()); + option.map_or_else(|| (), |_: ()| ()); + + let string = String::new(); + let option = Some(&string); + let _: &str = option.map_or_else(|| &string, |_| &string); + + let option = Some(()); + option.map_or_else( + || (), + |_| { + let tmp = (); + tmp + }, + ); +} diff --git a/tests/ui/unnecessary_option_map_or_else.stderr b/tests/ui/unnecessary_option_map_or_else.stderr new file mode 100644 index 000000000000..3fc4cdc73d0f --- /dev/null +++ b/tests/ui/unnecessary_option_map_or_else.stderr @@ -0,0 +1,35 @@ +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:13:5 + | +LL | option.map_or_else(|| (), |x| x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` + | + = note: `-D clippy::unnecessary-option-map-or-else` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_option_map_or_else)]` + +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:17:5 + | +LL | option.map_or_else(|| (), |x: ()| x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` + +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:22:19 + | +LL | let _: &str = option.map_or_else(|| &string, |x| x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| &string)` + +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:26:5 + | +LL | / option.map_or_else( +LL | | +LL | | || (), +LL | | |x| { +... | +LL | | }, +LL | | ); + | |_____^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` + +error: aborting due to 4 previous errors +