Refactors to unnecessary_{option,result}_map_or_else:

- When checking if a function is an identity function, try to use existing functionalities.
- Some cases are no longer supported, but they were buggy
This commit is contained in:
Teodoro Freund
2026-03-15 09:07:25 +00:00
parent 7849496c17
commit c455f7ce52
14 changed files with 536 additions and 316 deletions
+3 -4
View File
@@ -134,9 +134,8 @@
mod unnecessary_lazy_eval;
mod unnecessary_literal_unwrap;
mod unnecessary_map_or;
mod unnecessary_map_or_else;
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;
mod unwrap_expect_used;
@@ -4347,6 +4346,7 @@
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.map_or_else()` "map closure" for `Option` type.
///
/// ### Why is this bad?
@@ -5449,8 +5449,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
(sym::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);
unnecessary_map_or_else::check(cx, expr, recv, def, map, call_span);
},
(sym::next, []) => {
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
@@ -0,0 +1,39 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_expr_identity_function;
use clippy_utils::res::MaybeDef;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::Span;
use rustc_span::symbol::sym;
use super::{UNNECESSARY_OPTION_MAP_OR_ELSE, UNNECESSARY_RESULT_MAP_OR_ELSE};
/// lint use of `_.map_or_else(|err| err, |n| n)` for `Result`s and `Option`s.
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
recv: &Expr<'_>,
def_arg: &Expr<'_>,
map_arg: &Expr<'_>,
call_span: Span,
) {
let (symbol, lint) = match cx.typeck_results().expr_ty(recv).opt_diag_name(cx) {
Some(x @ sym::Result) => (x, UNNECESSARY_RESULT_MAP_OR_ELSE),
Some(x @ sym::Option) => (x, UNNECESSARY_OPTION_MAP_OR_ELSE),
_ => return,
};
if is_expr_identity_function(cx, map_arg) {
let msg = format!("unused \"map closure\" when calling `{symbol}::map_or_else` value");
span_lint_and_then(cx, lint, expr.span, msg, |diag| {
let mut applicability = Applicability::MachineApplicable;
let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability);
let sugg = format!("unwrap_or_else({err_snippet})");
diag.span_suggestion_verbose(call_span, "consider using `unwrap_or_else`", sugg, applicability);
});
}
}
@@ -1,111 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::res::MaybeDef;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{expr_or_init, find_binding_init, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Body, BodyId, 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 mut applicability = Applicability::MachineApplicable;
let self_snippet = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability);
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 Res::Local(hir_id) = path.res
&& expected_hir_id == hir_id
{
emit_lint(cx, expr, recv, def_arg);
}
}
fn handle_closure(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body_id: BodyId) {
let body = cx.tcx.hir_body(body_id);
handle_fn_body(cx, expr, recv, def_arg, body);
}
fn handle_fn_body(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body: &Body<'_>) {
if 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);
}
},
_ => {},
}
}
}
/// 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 !cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Option) {
return;
}
match map_arg.kind {
// If the second argument is a closure, we can check its body.
ExprKind::Closure(&Closure { body, .. }) => {
handle_closure(cx, expr, recv, def_arg, body);
},
ExprKind::Path(qpath) => {
let res = cx.qpath_res(&qpath, map_arg.hir_id);
match res {
// Case 1: Local variable (could be a closure)
Res::Local(hir_id) => {
if let Some(init_expr) = find_binding_init(cx, hir_id) {
let origin = expr_or_init(cx, init_expr);
if let ExprKind::Closure(&Closure { body, .. }) = origin.kind {
handle_closure(cx, expr, recv, def_arg, body);
}
}
},
// Case 2: Function definition
Res::Def(DefKind::Fn, def_id) => {
if let Some(local_def_id) = def_id.as_local()
&& let Some(body) = cx.tcx.hir_maybe_body_owned_by(local_def_id)
{
handle_fn_body(cx, expr, recv, def_arg, body);
}
},
_ => (),
}
},
_ => (),
}
}
@@ -1,80 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::peel_blocks;
use clippy_utils::res::MaybeDef;
use clippy_utils::source::snippet;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath};
use rustc_lint::LateContext;
use rustc_span::symbol::sym;
use super::UNNECESSARY_RESULT_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 `Result::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_RESULT_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 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 `Result`s.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
recv: &'tcx Expr<'_>,
def_arg: &'tcx Expr<'_>,
map_arg: &'tcx Expr<'_>,
) {
// lint if the caller of `map_or_else()` is a `Result`
if cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Result)
&& 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);
}
},
_ => {},
}
}
}
+50 -2
View File
@@ -1840,13 +1840,38 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// * `|[x, y]| [x, y]`
/// * `|Foo(bar, baz)| Foo(bar, baz)`
/// * `|Foo { bar, baz }| Foo { bar, baz }`
/// * `|x| { let y = x; ...; let z = y; z }`
/// * `|x| { let y = x; ...; let z = y; return z }`
///
/// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
fn is_body_identity_function<'hir>(cx: &LateContext<'_>, func: &Body<'hir>) -> bool {
let [param] = func.params else {
return false;
};
let mut param_pat = param.pat;
// Given a sequence of `Stmt`s of the form `let p = e` where `e` is an expr identical to the
// current `param_pat`, advance the current `param_pat` to `p`.
//
// Note: This is similar to `clippy_utils::get_last_chain_binding_hir_id`, but it works
// directly over a `Pattern` rather than a `HirId`. And it checks for compatibility via
// `is_expr_identity_of_pat` rather than `HirId` equality
let mut advance_param_pat_over_stmts = |stmts: &[Stmt<'hir>]| {
for stmt in stmts {
if let StmtKind::Let(local) = stmt.kind
&& let Some(init) = local.init
&& is_expr_identity_of_pat(cx, param_pat, init, true)
{
param_pat = local.pat;
} else {
return false;
}
}
true
};
let mut expr = func.value;
loop {
match expr.kind {
@@ -1875,7 +1900,30 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
return false;
}
},
_ => return is_expr_identity_of_pat(cx, param.pat, expr, true),
ExprKind::Block(
&Block {
stmts, expr: Some(e), ..
},
_,
) => {
if !advance_param_pat_over_stmts(stmts) {
return false;
}
expr = e;
},
ExprKind::Block(&Block { stmts, expr: None, .. }, _) => {
if let Some((last_stmt, stmts)) = stmts.split_last()
&& advance_param_pat_over_stmts(stmts)
&& let StmtKind::Semi(e) | StmtKind::Expr(e) = last_stmt.kind
&& let ExprKind::Ret(Some(ret_val)) = e.kind
{
expr = ret_val;
} else {
return false;
}
},
_ => return is_expr_identity_of_pat(cx, param_pat, expr, true),
}
}
}
+2 -1
View File
@@ -13,7 +13,8 @@
clippy::uninlined_format_args,
clippy::useless_vec,
clippy::unnecessary_map_on_constructor,
clippy::needless_lifetimes
clippy::needless_lifetimes,
clippy::unnecessary_option_map_or_else
)]
use std::path::{Path, PathBuf};
+2 -1
View File
@@ -13,7 +13,8 @@
clippy::uninlined_format_args,
clippy::useless_vec,
clippy::unnecessary_map_on_constructor,
clippy::needless_lifetimes
clippy::needless_lifetimes,
clippy::unnecessary_option_map_or_else
)]
use std::path::{Path, PathBuf};
+44 -44
View File
@@ -1,5 +1,5 @@
error: redundant closure
--> tests/ui/eta.rs:34:27
--> tests/ui/eta.rs:35:27
|
LL | let a = Some(1u8).map(|a| foo(a));
| ^^^^^^^^^^ 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)]`
error: redundant closure
--> tests/ui/eta.rs:39:40
--> tests/ui/eta.rs:40: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
--> tests/ui/eta.rs:42:35
--> tests/ui/eta.rs:43: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
--> tests/ui/eta.rs:45:26
--> tests/ui/eta.rs:46: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
--> tests/ui/eta.rs:54:27
--> tests/ui/eta.rs:55:27
|
LL | let e = Some(1u8).map(|a| generic(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic`
error: redundant closure
--> tests/ui/eta.rs:107:51
--> tests/ui/eta.rs:108:51
|
LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo());
| ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo`
@@ -41,229 +41,229 @@ 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)]`
error: redundant closure
--> tests/ui/eta.rs:109:51
--> tests/ui/eta.rs:110: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
--> tests/ui/eta.rs:112:42
--> tests/ui/eta.rs:113: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
--> tests/ui/eta.rs:117:29
--> tests/ui/eta.rs:118: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
--> tests/ui/eta.rs:119:27
--> tests/ui/eta.rs:120: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
--> tests/ui/eta.rs:122:65
--> tests/ui/eta.rs:123: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
--> tests/ui/eta.rs:139:23
--> tests/ui/eta.rs:140:23
|
LL | let _ = x.map(|x| x.parse::<i16>());
| ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `str::parse::<i16>`
error: redundant closure
--> tests/ui/eta.rs:192:22
--> tests/ui/eta.rs:193:22
|
LL | requires_fn_once(|| x());
| ^^^^^^ help: replace the closure with the function itself: `x`
error: redundant closure
--> tests/ui/eta.rs:200:27
--> tests/ui/eta.rs:201:27
|
LL | let a = Some(1u8).map(|a| foo_ptr(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr`
error: redundant closure
--> tests/ui/eta.rs:206:27
--> tests/ui/eta.rs:207:27
|
LL | let a = Some(1u8).map(|a| closure(a));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure`
error: redundant closure
--> tests/ui/eta.rs:239:28
--> tests/ui/eta.rs:240: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
--> tests/ui/eta.rs:241:28
--> tests/ui/eta.rs:242: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
--> tests/ui/eta.rs:243:28
--> tests/ui/eta.rs:244: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
--> tests/ui/eta.rs:251:21
--> tests/ui/eta.rs:252:21
|
LL | Some(1).map(|n| closure(n));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
error: redundant closure
--> tests/ui/eta.rs:256:21
--> tests/ui/eta.rs:257:21
|
LL | Some(1).map(|n| in_loop(n));
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
error: redundant closure
--> tests/ui/eta.rs:350:18
--> tests/ui/eta.rs:351:18
|
LL | takes_fn_mut(|| f());
| ^^^^^^ help: replace the closure with the function itself: `&mut f`
error: redundant closure
--> tests/ui/eta.rs:354:19
--> tests/ui/eta.rs:355:19
|
LL | takes_fn_once(|| f());
| ^^^^^^ help: replace the closure with the function itself: `&mut f`
error: redundant closure
--> tests/ui/eta.rs:359:26
--> tests/ui/eta.rs:360:26
|
LL | move || takes_fn_mut(|| f_used_once())
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once`
error: redundant closure
--> tests/ui/eta.rs:372:19
--> tests/ui/eta.rs:373:19
|
LL | array_opt.map(|a| a.as_slice());
| ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice`
error: redundant closure
--> tests/ui/eta.rs:376:19
--> tests/ui/eta.rs:377:19
|
LL | slice_opt.map(|s| s.len());
| ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len`
error: redundant closure
--> tests/ui/eta.rs:380:17
--> tests/ui/eta.rs:381:17
|
LL | ptr_opt.map(|p| p.is_null());
| ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null`
error: redundant closure
--> tests/ui/eta.rs:385:17
--> tests/ui/eta.rs:386: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
--> tests/ui/eta.rs:446:19
--> tests/ui/eta.rs:447:19
|
LL | let _ = f(&0, |x, y| f2(x, y));
| ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2`
error: redundant closure
--> tests/ui/eta.rs:475:22
--> tests/ui/eta.rs:476:22
|
LL | test.map(|t| t.method())
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Test::method`
error: redundant closure
--> tests/ui/eta.rs:480:22
--> tests/ui/eta.rs:481:22
|
LL | test.map(|t| t.method())
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `super::Outer::method`
error: redundant closure
--> tests/ui/eta.rs:494:18
--> tests/ui/eta.rs:495:18
|
LL | test.map(|t| t.method())
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `test_mod::Test::method`
error: redundant closure
--> tests/ui/eta.rs:502:30
--> tests/ui/eta.rs:503:30
|
LL | test.map(|t| t.method())
| ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method`
error: redundant closure
--> tests/ui/eta.rs:522:38
--> tests/ui/eta.rs:523:38
|
LL | let x = Box::new(|| None.map(|x| f(x)));
| ^^^^^^^^ help: replace the closure with the function itself: `&f`
error: redundant closure
--> tests/ui/eta.rs:527:38
--> tests/ui/eta.rs:528:38
|
LL | let x = Box::new(|| None.map(|x| f(x)));
| ^^^^^^^^ help: replace the closure with the function itself: `f`
error: redundant closure
--> tests/ui/eta.rs:545:35
--> tests/ui/eta.rs:546:35
|
LL | let _field = bind.or_else(|| get_default()).unwrap();
| ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `get_default`
error: redundant closure
--> tests/ui/eta.rs:592:16
--> tests/ui/eta.rs:593:16
|
LL | accepts_fn(|| f());
| ^^^^^^ help: replace the closure with the function itself: `**f`
error: redundant closure
--> tests/ui/eta.rs:596:16
--> tests/ui/eta.rs:597:16
|
LL | accepts_fn(|| g());
| ^^^^^^ help: replace the closure with the function itself: `g`
error: redundant closure
--> tests/ui/eta.rs:609:16
--> tests/ui/eta.rs:610:16
|
LL | accepts_fn(|| b());
| ^^^^^^ help: replace the closure with the function itself: `***b`
error: redundant closure
--> tests/ui/eta.rs:633:14
--> tests/ui/eta.rs:634:14
|
LL | .map(|n| MyError::A(n))
| ^^^^^^^^^^^^^^^^^ help: replace the closure with the tuple variant itself: `MyError::A`
error: redundant closure
--> tests/ui/eta.rs:630:14
--> tests/ui/eta.rs:631:14
|
LL | .map(|n| S(n))
| ^^^^^^^^ help: replace the closure with the tuple struct itself: `S`
error: redundant closure
--> tests/ui/eta.rs:627:14
--> tests/ui/eta.rs:628:14
|
LL | .map(|n| g(n))
| ^^^^^^^^ help: replace the closure with the function itself: `g`
error: redundant closure
--> tests/ui/eta.rs:624:14
--> tests/ui/eta.rs:625:14
|
LL | .map(|n| f(n))
| ^^^^^^^^ help: replace the closure with the function itself: `f`
error: redundant closure
--> tests/ui/eta.rs:651:31
--> tests/ui/eta.rs:652:31
|
LL | array.iter().for_each(|item| item.method());
| ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Self::method`
error: redundant closure
--> tests/ui/eta.rs:661:38
--> tests/ui/eta.rs:662:38
|
LL | (0..10).flat_map(|x| (0..10).map(|y| closure(y))).count();
| ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&*closure`
+23 -15
View File
@@ -3,13 +3,10 @@
clippy::let_and_return,
clippy::let_unit_value,
clippy::unnecessary_lazy_evaluations,
clippy::unnecessary_literal_unwrap
clippy::unnecessary_literal_unwrap,
clippy::needless_return
)]
const fn identity<T>(x: T) -> T {
x
}
const fn double_it(x: i32) -> i32 {
x * 2
}
@@ -18,31 +15,42 @@ fn main() {
// Expected errors
// Basic scenario
let option = Some(());
option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling
option.unwrap_or_else(|| ()); //~ unnecessary_option_map_or_else
// Type ascription
let option = Some(());
option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling
option.unwrap_or_else(|| ()); //~ unnecessary_option_map_or_else
// Auto-deref
let string = String::new();
let option = Some(&string);
let _: &str = option.unwrap_or_else(|| &string); //~ ERROR: unused "map closure" when calling
let _: &str = option.map_or_else(|| &string, |x| x);
// This should in theory lint with a smarter check
// Temporary variable
let option = Some(());
option.unwrap_or_else(|| ());
// Identity
// Temporary variable with pattern
let option = Some(((), ()));
option.unwrap_or_else(|| ((), ()));
// std::convert::identity
let string = String::new();
let option = Some(&string);
let _: &str = option.unwrap_or_else(|| &string); //~ ERROR: unused "map closure" when calling
let _: &str = option.unwrap_or_else(|| &string); //~ unnecessary_option_map_or_else
// Closure bound to a variable
let do_nothing = |x: String| x;
let string = String::new();
let option = Some(string.clone());
let _: String = option.unwrap_or_else(|| string); //~ ERROR: unused "map closure" when calling
let x: Option<((), ())> = Some(((), ()));
x.unwrap_or_else(|| ((), ()));
//~^ unnecessary_option_map_or_else
// Returned temporary variable.
let x: Option<()> = Some(());
x.unwrap_or_else(|| ());
// Returned temporary variable with pattern
let x: Option<((), ())> = Some(((), ()));
x.unwrap_or_else(|| ((), ()));
// Correct usages
let option = Some(());
+48 -16
View File
@@ -3,13 +3,10 @@
clippy::let_and_return,
clippy::let_unit_value,
clippy::unnecessary_lazy_evaluations,
clippy::unnecessary_literal_unwrap
clippy::unnecessary_literal_unwrap,
clippy::needless_return
)]
const fn identity<T>(x: T) -> T {
x
}
const fn double_it(x: i32) -> i32 {
x * 2
}
@@ -18,21 +15,22 @@ fn main() {
// Expected errors
// Basic scenario
let option = Some(());
option.map_or_else(|| (), |x| x); //~ ERROR: unused "map closure" when calling
option.map_or_else(|| (), |x| x); //~ unnecessary_option_map_or_else
// Type ascription
let option = Some(());
option.map_or_else(|| (), |x: ()| x); //~ ERROR: unused "map closure" when calling
option.map_or_else(|| (), |x: ()| x); //~ unnecessary_option_map_or_else
// 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
let _: &str = option.map_or_else(|| &string, |x| x);
// This should in theory lint with a smarter check
// Temporary variable
let option = Some(());
option.map_or_else(
//~^ ERROR: unused "map closure" when calling
//~^ unnecessary_option_map_or_else
|| (),
|x| {
let tmp = x;
@@ -40,16 +38,50 @@ fn main() {
},
);
// Identity
// Temporary variable with pattern
let option = Some(((), ()));
option.map_or_else(
//~^ unnecessary_option_map_or_else
|| ((), ()),
|x| {
let tmp = x;
let (a, b) = tmp;
(a, b)
},
);
// std::convert::identity
let string = String::new();
let option = Some(&string);
let _: &str = option.map_or_else(|| &string, identity); //~ ERROR: unused "map closure" when calling
let _: &str = option.map_or_else(|| &string, std::convert::identity); //~ unnecessary_option_map_or_else
// Closure bound to a variable
let do_nothing = |x: String| x;
let string = String::new();
let option = Some(string.clone());
let _: String = option.map_or_else(|| string, do_nothing); //~ ERROR: unused "map closure" when calling
let x: Option<((), ())> = Some(((), ()));
x.map_or_else(|| ((), ()), |(a, b)| (a, b));
//~^ unnecessary_option_map_or_else
// Returned temporary variable.
let x: Option<()> = Some(());
x.map_or_else(
//~^ unnecessary_option_map_or_else
|| (),
|n| {
let tmp = n;
let tmp2 = tmp;
return tmp2;
},
);
// Returned temporary variable with pattern
let x: Option<((), ())> = Some(((), ()));
x.map_or_else(
//~^ unnecessary_option_map_or_else
|| ((), ()),
|n| {
let tmp = n;
let (a, b) = tmp;
return (a, b);
},
);
// Correct usages
let option = Some(());
+126 -18
View File
@@ -1,26 +1,31 @@
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:21:5
--> tests/ui/unnecessary_option_map_or_else.rs:18: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)]`
help: consider using `unwrap_or_else`
|
LL - option.map_or_else(|| (), |x| x);
LL + option.unwrap_or_else(|| ());
|
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:25:5
--> tests/ui/unnecessary_option_map_or_else.rs:22: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:30:19
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider using `unwrap_or_else`
|
LL - option.map_or_else(|| (), |x: ()| x);
LL + option.unwrap_or_else(|| ());
|
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:34:5
--> tests/ui/unnecessary_option_map_or_else.rs:32:5
|
LL | / option.map_or_else(
LL | |
@@ -29,19 +34,122 @@ LL | | |x| {
... |
LL | | },
LL | | );
| |_____^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())`
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - option.map_or_else(
LL -
LL - || (),
LL - |x| {
LL - let tmp = x;
LL - tmp
LL - },
LL - );
LL + option.unwrap_or_else(|| ());
|
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:46:19
--> tests/ui/unnecessary_option_map_or_else.rs:43:5
|
LL | / option.map_or_else(
LL | |
LL | | || ((), ()),
LL | | |x| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - option.map_or_else(
LL -
LL - || ((), ()),
LL - |x| {
LL - let tmp = x;
LL - let (a, b) = tmp;
LL - (a, b)
LL - },
LL - );
LL + option.unwrap_or_else(|| ((), ()));
|
LL | let _: &str = option.map_or_else(|| &string, identity);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:52:21
--> tests/ui/unnecessary_option_map_or_else.rs:56:19
|
LL | let _: &str = option.map_or_else(|| &string, std::convert::identity);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider using `unwrap_or_else`
|
LL - let _: &str = option.map_or_else(|| &string, std::convert::identity);
LL + let _: &str = option.unwrap_or_else(|| &string);
|
LL | let _: String = option.map_or_else(|| string, do_nothing);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| string)`
error: aborting due to 6 previous errors
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:59:5
|
LL | x.map_or_else(|| ((), ()), |(a, b)| (a, b));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(|| ((), ()), |(a, b)| (a, b));
LL + x.unwrap_or_else(|| ((), ()));
|
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:64:5
|
LL | / x.map_or_else(
LL | |
LL | | || (),
LL | | |n| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - || (),
LL - |n| {
LL - let tmp = n;
LL - let tmp2 = tmp;
LL - return tmp2;
LL - },
LL - );
LL + x.unwrap_or_else(|| ());
|
error: unused "map closure" when calling `Option::map_or_else` value
--> tests/ui/unnecessary_option_map_or_else.rs:76:5
|
LL | / x.map_or_else(
LL | |
LL | | || ((), ()),
LL | | |n| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - || ((), ()),
LL - |n| {
LL - let tmp = n;
LL - let (a, b) = tmp;
LL - return (a, b);
LL - },
LL - );
LL + x.unwrap_or_else(|| ((), ()));
|
error: aborting due to 8 previous errors
+24 -3
View File
@@ -1,5 +1,10 @@
#![warn(clippy::unnecessary_result_map_or_else)]
#![allow(clippy::unnecessary_literal_unwrap, clippy::let_and_return, clippy::let_unit_value)]
#![allow(
clippy::unnecessary_literal_unwrap,
clippy::let_and_return,
clippy::let_unit_value,
clippy::needless_return
)]
fn main() {
let x: Result<(), ()> = Ok(());
@@ -14,13 +19,17 @@ fn main() {
// Auto-deref.
let y = String::new();
let x: Result<&String, &String> = Ok(&y);
let y: &str = x.unwrap_or_else(|err| err);
//~^ unnecessary_result_map_or_else
let y: &str = x.map_or_else(|err| err, |n| n);
// This should lint with a smarter check
// Temporary variable.
let x: Result<(), ()> = Ok(());
x.unwrap_or_else(|err| err);
// Temporary variable with pattern
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.unwrap_or_else(|err| err);
// Should not warn.
let x: Result<usize, usize> = Ok(0);
x.map_or_else(|err| err, |n| n + 1);
@@ -61,4 +70,16 @@ fn main() {
y
},
);
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.unwrap_or_else(|err| err);
//~^ unnecessary_result_map_or_else
// Returned temporary variable.
let x: Result<(), ()> = Ok(());
x.unwrap_or_else(|err| err);
// Returned temporary variable with pattern
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.unwrap_or_else(|err| err);
}
+47 -2
View File
@@ -1,5 +1,10 @@
#![warn(clippy::unnecessary_result_map_or_else)]
#![allow(clippy::unnecessary_literal_unwrap, clippy::let_and_return, clippy::let_unit_value)]
#![allow(
clippy::unnecessary_literal_unwrap,
clippy::let_and_return,
clippy::let_unit_value,
clippy::needless_return
)]
fn main() {
let x: Result<(), ()> = Ok(());
@@ -15,7 +20,7 @@ fn main() {
let y = String::new();
let x: Result<&String, &String> = Ok(&y);
let y: &str = x.map_or_else(|err| err, |n| n);
//~^ unnecessary_result_map_or_else
// This should lint with a smarter check
// Temporary variable.
let x: Result<(), ()> = Ok(());
@@ -29,6 +34,18 @@ fn main() {
},
);
// Temporary variable with pattern
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.map_or_else(
//~^ unnecessary_result_map_or_else
|err| err,
|n| {
let tmp = n;
let (a, b) = tmp;
(a, b)
},
);
// Should not warn.
let x: Result<usize, usize> = Ok(0);
x.map_or_else(|err| err, |n| n + 1);
@@ -69,4 +86,32 @@ fn main() {
y
},
);
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.map_or_else(|err| err, |(a, b)| (a, b));
//~^ unnecessary_result_map_or_else
// Returned temporary variable.
let x: Result<(), ()> = Ok(());
x.map_or_else(
//~^ unnecessary_result_map_or_else
|err| err,
|n| {
let tmp = n;
let tmp2 = tmp;
return tmp2;
},
);
// Returned temporary variable with pattern
let x: Result<((), ()), ((), ())> = Ok(((), ()));
x.map_or_else(
//~^ unnecessary_result_map_or_else
|err| err,
|n| {
let tmp = n;
let (a, b) = tmp;
return (a, b);
},
);
}
+128 -19
View File
@@ -1,26 +1,31 @@
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:6:5
|
LL | x.map_or_else(|err| err, |n| n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
|
= note: `-D clippy::unnecessary-result-map-or-else` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_result_map_or_else)]`
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:11:5
|
LL | x.map_or_else(|err: ()| err, |n: ()| n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err: ()| err)`
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:17:19
LL | x.map_or_else(|err| err, |n| n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unnecessary-result-map-or-else` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_result_map_or_else)]`
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(|err| err, |n| n);
LL + x.unwrap_or_else(|err| err);
|
LL | let y: &str = x.map_or_else(|err| err, |n| n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:22:5
--> tests/ui/unnecessary_result_map_or_else.rs:16:5
|
LL | x.map_or_else(|err: ()| err, |n: ()| n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(|err: ()| err, |n: ()| n);
LL + x.unwrap_or_else(|err: ()| err);
|
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:27:5
|
LL | / x.map_or_else(
LL | |
@@ -29,7 +34,111 @@ LL | | |n| {
... |
LL | | },
LL | | );
| |_____^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)`
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - |err| err,
LL - |n| {
LL - let tmp = n;
LL - let tmp2 = tmp;
LL - tmp2
LL - },
LL - );
LL + x.unwrap_or_else(|err| err);
|
error: aborting due to 4 previous errors
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:39:5
|
LL | / x.map_or_else(
LL | |
LL | | |err| err,
LL | | |n| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - |err| err,
LL - |n| {
LL - let tmp = n;
LL - let (a, b) = tmp;
LL - (a, b)
LL - },
LL - );
LL + x.unwrap_or_else(|err| err);
|
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:91:5
|
LL | x.map_or_else(|err| err, |(a, b)| (a, b));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(|err| err, |(a, b)| (a, b));
LL + x.unwrap_or_else(|err| err);
|
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:96:5
|
LL | / x.map_or_else(
LL | |
LL | | |err| err,
LL | | |n| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - |err| err,
LL - |n| {
LL - let tmp = n;
LL - let tmp2 = tmp;
LL - return tmp2;
LL - },
LL - );
LL + x.unwrap_or_else(|err| err);
|
error: unused "map closure" when calling `Result::map_or_else` value
--> tests/ui/unnecessary_result_map_or_else.rs:108:5
|
LL | / x.map_or_else(
LL | |
LL | | |err| err,
LL | | |n| {
... |
LL | | },
LL | | );
| |_____^
|
help: consider using `unwrap_or_else`
|
LL - x.map_or_else(
LL -
LL - |err| err,
LL - |n| {
LL - let tmp = n;
LL - let (a, b) = tmp;
LL - return (a, b);
LL - },
LL - );
LL + x.unwrap_or_else(|err| err);
|
error: aborting due to 7 previous errors