From 528308b5aa544454e6c90f6d051c3adeb1a46e1b Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 2 Jul 2022 15:00:24 -0700 Subject: [PATCH] Extend unnecessary_lazy_eval to cover `bool::then` -> `bool::then_some` --- clippy_lints/src/methods/mod.rs | 1 + .../src/methods/unnecessary_lazy_eval.rs | 7 +- tests/ui/unnecessary_lazy_eval.fixed | 2 + tests/ui/unnecessary_lazy_eval.rs | 2 + tests/ui/unnecessary_lazy_eval.stderr | 76 ++++++++++--------- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index cc1530f0e..36389d798 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2737,6 +2737,7 @@ impl Methods { } }, ("take", []) => needless_option_take::check(cx, expr, recv), + ("then", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "then_some"), ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => { implicit_clone::check(cx, name, expr, recv); }, diff --git a/clippy_lints/src/methods/unnecessary_lazy_eval.rs b/clippy_lints/src/methods/unnecessary_lazy_eval.rs index 865f6d031..a9c641b46 100644 --- a/clippy_lints/src/methods/unnecessary_lazy_eval.rs +++ b/clippy_lints/src/methods/unnecessary_lazy_eval.rs @@ -20,8 +20,9 @@ pub(super) fn check<'tcx>( ) { let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option); let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result); + let is_bool = cx.typeck_results().expr_ty(recv).is_bool(); - if is_option || is_result { + if is_option || is_result || is_bool { if let hir::ExprKind::Closure { body, .. } = arg.kind { let body = cx.tcx.hir().body(body); let body_expr = &body.value; @@ -33,8 +34,10 @@ pub(super) fn check<'tcx>( if eager_or_lazy::switch_to_eager_eval(cx, body_expr) { let msg = if is_option { "unnecessary closure used to substitute value for `Option::None`" - } else { + } else if is_result { "unnecessary closure used to substitute value for `Result::Err`" + } else { + "unnecessary closure used with `bool::then`" }; let applicability = if body .params diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed index 65fcdc430..eed817968 100644 --- a/tests/ui/unnecessary_lazy_eval.fixed +++ b/tests/ui/unnecessary_lazy_eval.fixed @@ -30,6 +30,7 @@ fn main() { let ext_opt = Some(42); let nested_opt = Some(Some(42)); let nested_tuple_opt = Some(Some((42, 43))); + let cond = true; // Should lint - Option let _ = opt.unwrap_or(2); @@ -42,6 +43,7 @@ fn main() { let _ = opt.get_or_insert(2); let _ = opt.ok_or(2); let _ = nested_tuple_opt.unwrap_or(Some((1, 2))); + let _ = cond.then_some(astronomers_pi); // Cases when unwrap is not called on a simple variable let _ = Some(10).unwrap_or(2); diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs index 206080ed6..1588db79b 100644 --- a/tests/ui/unnecessary_lazy_eval.rs +++ b/tests/ui/unnecessary_lazy_eval.rs @@ -30,6 +30,7 @@ fn main() { let ext_opt = Some(42); let nested_opt = Some(Some(42)); let nested_tuple_opt = Some(Some((42, 43))); + let cond = true; // Should lint - Option let _ = opt.unwrap_or_else(|| 2); @@ -42,6 +43,7 @@ fn main() { let _ = opt.get_or_insert_with(|| 2); let _ = opt.ok_or_else(|| 2); let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); + let _ = cond.then(|| astronomers_pi); // Cases when unwrap is not called on a simple variable let _ = Some(10).unwrap_or_else(|| 2); diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr index 7e4dd7730..83dc7fd83 100644 --- a/tests/ui/unnecessary_lazy_eval.stderr +++ b/tests/ui/unnecessary_lazy_eval.stderr @@ -1,5 +1,5 @@ error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:35:13 + --> $DIR/unnecessary_lazy_eval.rs:36:13 | LL | let _ = opt.unwrap_or_else(|| 2); | ^^^^-------------------- @@ -9,7 +9,7 @@ LL | let _ = opt.unwrap_or_else(|| 2); = note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:36:13 + --> $DIR/unnecessary_lazy_eval.rs:37:13 | LL | let _ = opt.unwrap_or_else(|| astronomers_pi); | ^^^^--------------------------------- @@ -17,7 +17,7 @@ LL | let _ = opt.unwrap_or_else(|| astronomers_pi); | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:37:13 + --> $DIR/unnecessary_lazy_eval.rs:38:13 | LL | let _ = opt.unwrap_or_else(|| ext_str.some_field); | ^^^^------------------------------------- @@ -25,7 +25,7 @@ LL | let _ = opt.unwrap_or_else(|| ext_str.some_field); | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:39:13 + --> $DIR/unnecessary_lazy_eval.rs:40:13 | LL | let _ = opt.and_then(|_| ext_opt); | ^^^^--------------------- @@ -33,7 +33,7 @@ LL | let _ = opt.and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:40:13 + --> $DIR/unnecessary_lazy_eval.rs:41:13 | LL | let _ = opt.or_else(|| ext_opt); | ^^^^------------------- @@ -41,7 +41,7 @@ LL | let _ = opt.or_else(|| ext_opt); | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:41:13 + --> $DIR/unnecessary_lazy_eval.rs:42:13 | LL | let _ = opt.or_else(|| None); | ^^^^---------------- @@ -49,7 +49,7 @@ LL | let _ = opt.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:42:13 + --> $DIR/unnecessary_lazy_eval.rs:43:13 | LL | let _ = opt.get_or_insert_with(|| 2); | ^^^^------------------------ @@ -57,7 +57,7 @@ LL | let _ = opt.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:43:13 + --> $DIR/unnecessary_lazy_eval.rs:44:13 | LL | let _ = opt.ok_or_else(|| 2); | ^^^^---------------- @@ -65,15 +65,23 @@ LL | let _ = opt.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:44:13 + --> $DIR/unnecessary_lazy_eval.rs:45:13 | LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); | ^^^^^^^^^^^^^^^^^------------------------------- | | | help: use `unwrap_or(..)` instead: `unwrap_or(Some((1, 2)))` +error: unnecessary closure used with `bool::then` + --> $DIR/unnecessary_lazy_eval.rs:46:13 + | +LL | let _ = cond.then(|| astronomers_pi); + | ^^^^^----------------------- + | | + | help: use `then_some(..)` instead: `then_some(astronomers_pi)` + error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:47:13 + --> $DIR/unnecessary_lazy_eval.rs:49:13 | LL | let _ = Some(10).unwrap_or_else(|| 2); | ^^^^^^^^^-------------------- @@ -81,7 +89,7 @@ LL | let _ = Some(10).unwrap_or_else(|| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:48:13 + --> $DIR/unnecessary_lazy_eval.rs:50:13 | LL | let _ = Some(10).and_then(|_| ext_opt); | ^^^^^^^^^--------------------- @@ -89,7 +97,7 @@ LL | let _ = Some(10).and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:49:28 + --> $DIR/unnecessary_lazy_eval.rs:51:28 | LL | let _: Option = None.or_else(|| ext_opt); | ^^^^^------------------- @@ -97,7 +105,7 @@ LL | let _: Option = None.or_else(|| ext_opt); | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:50:13 + --> $DIR/unnecessary_lazy_eval.rs:52:13 | LL | let _ = None.get_or_insert_with(|| 2); | ^^^^^------------------------ @@ -105,7 +113,7 @@ LL | let _ = None.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:51:35 + --> $DIR/unnecessary_lazy_eval.rs:53:35 | LL | let _: Result = None.ok_or_else(|| 2); | ^^^^^---------------- @@ -113,7 +121,7 @@ LL | let _: Result = None.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:52:28 + --> $DIR/unnecessary_lazy_eval.rs:54:28 | LL | let _: Option = None.or_else(|| None); | ^^^^^---------------- @@ -121,7 +129,7 @@ LL | let _: Option = None.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:55:13 + --> $DIR/unnecessary_lazy_eval.rs:57:13 | LL | let _ = deep.0.unwrap_or_else(|| 2); | ^^^^^^^-------------------- @@ -129,7 +137,7 @@ LL | let _ = deep.0.unwrap_or_else(|| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:56:13 + --> $DIR/unnecessary_lazy_eval.rs:58:13 | LL | let _ = deep.0.and_then(|_| ext_opt); | ^^^^^^^--------------------- @@ -137,7 +145,7 @@ LL | let _ = deep.0.and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:57:13 + --> $DIR/unnecessary_lazy_eval.rs:59:13 | LL | let _ = deep.0.or_else(|| None); | ^^^^^^^---------------- @@ -145,7 +153,7 @@ LL | let _ = deep.0.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:58:13 + --> $DIR/unnecessary_lazy_eval.rs:60:13 | LL | let _ = deep.0.get_or_insert_with(|| 2); | ^^^^^^^------------------------ @@ -153,7 +161,7 @@ LL | let _ = deep.0.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:59:13 + --> $DIR/unnecessary_lazy_eval.rs:61:13 | LL | let _ = deep.0.ok_or_else(|| 2); | ^^^^^^^---------------- @@ -161,7 +169,7 @@ LL | let _ = deep.0.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:79:28 + --> $DIR/unnecessary_lazy_eval.rs:81:28 | LL | let _: Option = None.or_else(|| Some(3)); | ^^^^^------------------- @@ -169,7 +177,7 @@ LL | let _: Option = None.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:80:13 + --> $DIR/unnecessary_lazy_eval.rs:82:13 | LL | let _ = deep.0.or_else(|| Some(3)); | ^^^^^^^------------------- @@ -177,7 +185,7 @@ LL | let _ = deep.0.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:81:13 + --> $DIR/unnecessary_lazy_eval.rs:83:13 | LL | let _ = opt.or_else(|| Some(3)); | ^^^^------------------- @@ -185,7 +193,7 @@ LL | let _ = opt.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:87:13 + --> $DIR/unnecessary_lazy_eval.rs:89:13 | LL | let _ = res2.unwrap_or_else(|_| 2); | ^^^^^--------------------- @@ -193,7 +201,7 @@ LL | let _ = res2.unwrap_or_else(|_| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:88:13 + --> $DIR/unnecessary_lazy_eval.rs:90:13 | LL | let _ = res2.unwrap_or_else(|_| astronomers_pi); | ^^^^^---------------------------------- @@ -201,7 +209,7 @@ LL | let _ = res2.unwrap_or_else(|_| astronomers_pi); | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:89:13 + --> $DIR/unnecessary_lazy_eval.rs:91:13 | LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field); | ^^^^^-------------------------------------- @@ -209,7 +217,7 @@ LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field); | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:111:35 + --> $DIR/unnecessary_lazy_eval.rs:113:35 | LL | let _: Result = res.and_then(|_| Err(2)); | ^^^^-------------------- @@ -217,7 +225,7 @@ LL | let _: Result = res.and_then(|_| Err(2)); | help: use `and(..)` instead: `and(Err(2))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:112:35 + --> $DIR/unnecessary_lazy_eval.rs:114:35 | LL | let _: Result = res.and_then(|_| Err(astronomers_pi)); | ^^^^--------------------------------- @@ -225,7 +233,7 @@ LL | let _: Result = res.and_then(|_| Err(astronomers_pi)); | help: use `and(..)` instead: `and(Err(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:113:35 + --> $DIR/unnecessary_lazy_eval.rs:115:35 | LL | let _: Result = res.and_then(|_| Err(ext_str.some_field)); | ^^^^------------------------------------- @@ -233,7 +241,7 @@ LL | let _: Result = res.and_then(|_| Err(ext_str.some_field)) | help: use `and(..)` instead: `and(Err(ext_str.some_field))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:115:35 + --> $DIR/unnecessary_lazy_eval.rs:117:35 | LL | let _: Result = res.or_else(|_| Ok(2)); | ^^^^------------------ @@ -241,7 +249,7 @@ LL | let _: Result = res.or_else(|_| Ok(2)); | help: use `or(..)` instead: `or(Ok(2))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:116:35 + --> $DIR/unnecessary_lazy_eval.rs:118:35 | LL | let _: Result = res.or_else(|_| Ok(astronomers_pi)); | ^^^^------------------------------- @@ -249,7 +257,7 @@ LL | let _: Result = res.or_else(|_| Ok(astronomers_pi)); | help: use `or(..)` instead: `or(Ok(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:117:35 + --> $DIR/unnecessary_lazy_eval.rs:119:35 | LL | let _: Result = res.or_else(|_| Ok(ext_str.some_field)); | ^^^^----------------------------------- @@ -257,7 +265,7 @@ LL | let _: Result = res.or_else(|_| Ok(ext_str.some_field)); | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:118:35 + --> $DIR/unnecessary_lazy_eval.rs:120:35 | LL | let _: Result = res. | ___________________________________^ @@ -271,5 +279,5 @@ LL | | or_else(|_| Ok(ext_str.some_field)); | | | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` -error: aborting due to 33 previous errors +error: aborting due to 34 previous errors