Skip to content

Add lint unnecessary_option_map_or_else #14662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+308 −65
Open

Add lint unnecessary_option_map_or_else #14662

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
View file
Original file line number Diff line number Diff line change
@@ -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,
28 changes: 28 additions & 0 deletions clippy_lints/src/methods/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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<u32> = Some(4);
/// let y = x.map_or_else(|| 2 * k, |n| n);
/// ```
/// Use instead:
/// ```no_run
/// let k = 10;
/// let x: Option<u32> = 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", []) => {
73 changes: 73 additions & 0 deletions clippy_lints/src/methods/unnecessary_option_map_or_else.rs
View file
Original file line number Diff line number Diff line change
@@ -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);
}
},
_ => {},
}
}
}
3 changes: 2 additions & 1 deletion tests/ui/option_if_let_else.fixed
View file
Original file line number Diff line number Diff line change
@@ -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) {
3 changes: 2 additions & 1 deletion tests/ui/option_if_let_else.rs
View file
Original file line number Diff line number Diff line change
@@ -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) {
50 changes: 25 additions & 25 deletions tests/ui/option_if_let_else.stderr
View file
Original file line number Diff line number Diff line change
@@ -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())`
1 change: 1 addition & 0 deletions tests/ui/or_fun_call.fixed
View file
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
clippy::uninlined_format_args,
clippy::unnecessary_wraps,
clippy::unnecessary_literal_unwrap,
clippy::unnecessary_option_map_or_else,
clippy::useless_vec
)]

1 change: 1 addition & 0 deletions tests/ui/or_fun_call.rs
View file
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
clippy::uninlined_format_args,
clippy::unnecessary_wraps,
clippy::unnecessary_literal_unwrap,
clippy::unnecessary_option_map_or_else,
clippy::useless_vec
)]

76 changes: 38 additions & 38 deletions tests/ui/or_fun_call.stderr
View file
Original file line number Diff line number Diff line change
@@ -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(<FakeDefault>::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(<FakeDefault>::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(<FakeDefault as Default>::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(<FakeDefault as Default>::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() })`
47 changes: 47 additions & 0 deletions tests/ui/unnecessary_option_map_or_else.fixed
View file
Original file line number Diff line number Diff line change
@@ -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
},
);
}
54 changes: 54 additions & 0 deletions tests/ui/unnecessary_option_map_or_else.rs
View file
Original file line number Diff line number Diff line change
@@ -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
},
);
}
35 changes: 35 additions & 0 deletions tests/ui/unnecessary_option_map_or_else.stderr
View file
Original file line number Diff line number Diff line change
@@ -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