diff options
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 5 | ||||
| -rw-r--r-- | clippy_lints/src/methods/obfuscated_if_else.rs | 18 | ||||
| -rw-r--r-- | tests/ui/obfuscated_if_else.fixed | 24 | ||||
| -rw-r--r-- | tests/ui/obfuscated_if_else.rs | 24 | ||||
| -rw-r--r-- | tests/ui/obfuscated_if_else.stderr | 60 |
5 files changed, 111 insertions, 20 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 291ddc1ce17..3f917430a13 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -5423,7 +5423,7 @@ impl Methods { option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, &self.msrv); }, Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => { - obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method); + obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or"); }, _ => {}, } @@ -5442,6 +5442,9 @@ impl Methods { match method_call(recv) { Some(("map", recv, [map_arg], _, _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, &self.msrv) => {}, + Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => { + obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method, "unwrap_or_else"); + }, _ => { unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); }, diff --git a/clippy_lints/src/methods/obfuscated_if_else.rs b/clippy_lints/src/methods/obfuscated_if_else.rs index 3ef4906b3c9..9a5ffdeaf4e 100644 --- a/clippy_lints/src/methods/obfuscated_if_else.rs +++ b/clippy_lints/src/methods/obfuscated_if_else.rs @@ -16,6 +16,7 @@ pub(super) fn check<'tcx>( then_arg: &'tcx hir::Expr<'_>, unwrap_arg: &'tcx hir::Expr<'_>, then_method_name: &str, + unwrap_method_name: &str, ) { let recv_ty = cx.typeck_results().expr_ty(then_recv); @@ -32,14 +33,27 @@ pub(super) fn check<'tcx>( snippet_with_applicability(cx, body.value.span, "..", &mut applicability) }, "then_some" => snippet_with_applicability(cx, then_arg.span, "..", &mut applicability), - _ => String::new().into(), + _ => return, + }; + + // FIXME: Add `unwrap_or_else` symbol + let els = match unwrap_method_name { + "unwrap_or" => snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability), + "unwrap_or_else" if let ExprKind::Closure(closure) = unwrap_arg.kind => { + let body = cx.tcx.hir_body(closure.body); + snippet_with_applicability(cx, body.value.span, "..", &mut applicability) + }, + "unwrap_or_else" if let ExprKind::Path(_) = unwrap_arg.kind => { + snippet_with_applicability(cx, unwrap_arg.span, "_", &mut applicability) + "()" + }, + _ => return, }; let sugg = format!( "if {} {{ {} }} else {{ {} }}", Sugg::hir_with_applicability(cx, then_recv, "..", &mut applicability), if_then, - snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability) + els ); // To be parsed as an expression, the `if { … } else { … }` as the left operand of a binary operator diff --git a/tests/ui/obfuscated_if_else.fixed b/tests/ui/obfuscated_if_else.fixed index b7a9cc83647..66f5070787b 100644 --- a/tests/ui/obfuscated_if_else.fixed +++ b/tests/ui/obfuscated_if_else.fixed @@ -1,5 +1,10 @@ #![warn(clippy::obfuscated_if_else)] -#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)] +#![allow( + clippy::unnecessary_lazy_evaluations, + clippy::unit_arg, + clippy::unused_unit, + clippy::unwrap_or_default +)] fn main() { if true { "a" } else { "b" }; @@ -24,6 +29,23 @@ fn main() { if true { () } else { a += 2 }; //~^ obfuscated_if_else + + let mut n = 1; + if true { n = 1 } else { n = 2 }; + //~^ obfuscated_if_else + if true { 1 } else { n * 2 }; + //~^ obfuscated_if_else + if true { n += 1 } else { () }; + //~^ obfuscated_if_else + + let _ = if true { 1 } else { n * 2 }; + //~^ obfuscated_if_else + + if true { 1 } else { Default::default() }; + //~^ obfuscated_if_else + + let partial = true.then_some(1); + partial.unwrap_or_else(|| n * 2); // not lint } fn issue11141() { diff --git a/tests/ui/obfuscated_if_else.rs b/tests/ui/obfuscated_if_else.rs index a6470fedd5c..4efd740eb60 100644 --- a/tests/ui/obfuscated_if_else.rs +++ b/tests/ui/obfuscated_if_else.rs @@ -1,5 +1,10 @@ #![warn(clippy::obfuscated_if_else)] -#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)] +#![allow( + clippy::unnecessary_lazy_evaluations, + clippy::unit_arg, + clippy::unused_unit, + clippy::unwrap_or_default +)] fn main() { true.then_some("a").unwrap_or("b"); @@ -24,6 +29,23 @@ fn main() { true.then_some(()).unwrap_or(a += 2); //~^ obfuscated_if_else + + let mut n = 1; + true.then(|| n = 1).unwrap_or_else(|| n = 2); + //~^ obfuscated_if_else + true.then_some(1).unwrap_or_else(|| n * 2); + //~^ obfuscated_if_else + true.then_some(n += 1).unwrap_or_else(|| ()); + //~^ obfuscated_if_else + + let _ = true.then_some(1).unwrap_or_else(|| n * 2); + //~^ obfuscated_if_else + + true.then_some(1).unwrap_or_else(Default::default); + //~^ obfuscated_if_else + + let partial = true.then_some(1); + partial.unwrap_or_else(|| n * 2); // not lint } fn issue11141() { diff --git a/tests/ui/obfuscated_if_else.stderr b/tests/ui/obfuscated_if_else.stderr index 84caa848ece..d676c256695 100644 --- a/tests/ui/obfuscated_if_else.stderr +++ b/tests/ui/obfuscated_if_else.stderr @@ -1,5 +1,5 @@ error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:5:5 + --> tests/ui/obfuscated_if_else.rs:10:5 | LL | true.then_some("a").unwrap_or("b"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }` @@ -8,82 +8,112 @@ LL | true.then_some("a").unwrap_or("b"); = help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:8:5 + --> tests/ui/obfuscated_if_else.rs:13:5 | LL | true.then(|| "a").unwrap_or("b"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:12:5 + --> tests/ui/obfuscated_if_else.rs:17:5 | LL | (a == 1).then_some("a").unwrap_or("b"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:15:5 + --> tests/ui/obfuscated_if_else.rs:20:5 | LL | (a == 1).then(|| "a").unwrap_or("b"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:22:5 + --> tests/ui/obfuscated_if_else.rs:27:5 | LL | true.then_some(a += 1).unwrap_or(()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { a += 1 } else { () }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:25:5 + --> tests/ui/obfuscated_if_else.rs:30:5 | LL | true.then_some(()).unwrap_or(a += 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:31:13 + --> tests/ui/obfuscated_if_else.rs:34:5 + | +LL | true.then(|| n = 1).unwrap_or_else(|| n = 2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { n = 1 } else { n = 2 }` + +error: this method chain can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:36:5 + | +LL | true.then_some(1).unwrap_or_else(|| n * 2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { n * 2 }` + +error: this method chain can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:38:5 + | +LL | true.then_some(n += 1).unwrap_or_else(|| ()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { n += 1 } else { () }` + +error: this method chain can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:41:13 + | +LL | let _ = true.then_some(1).unwrap_or_else(|| n * 2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { n * 2 }` + +error: this method chain can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:44:5 + | +LL | true.then_some(1).unwrap_or_else(Default::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 1 } else { Default::default() }` + +error: this method chain can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:53:13 | LL | let _ = true.then_some(40).unwrap_or(17) | 2; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 40 } else { 17 })` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:35:13 + --> tests/ui/obfuscated_if_else.rs:57:13 | LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(if true { 30 } else { 17 })` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:35:48 + --> tests/ui/obfuscated_if_else.rs:57:48 | LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 2 } else { 3 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:35:81 + --> tests/ui/obfuscated_if_else.rs:57:81 | LL | let _ = true.then_some(30).unwrap_or(17) | true.then_some(2).unwrap_or(3) | true.then_some(10).unwrap_or(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 10 } else { 1 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:41:17 + --> tests/ui/obfuscated_if_else.rs:63:17 | LL | let _ = 2 | true.then_some(40).unwrap_or(17); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 40 } else { 17 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:45:13 + --> tests/ui/obfuscated_if_else.rs:67:13 | LL | let _ = true.then_some(42).unwrap_or(17) as u8; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { 42 } else { 17 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:49:14 + --> tests/ui/obfuscated_if_else.rs:71:14 | LL | let _ = *true.then_some(&42).unwrap_or(&17); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }` error: this method chain can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:53:14 + --> tests/ui/obfuscated_if_else.rs:75:14 | LL | let _ = *true.then_some(&42).unwrap_or(&17) as u8; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { &42 } else { &17 }` -error: aborting due to 14 previous errors +error: aborting due to 19 previous errors |