From 0fb27477e47d9e823451b212dc53b3e919200b00 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Sat, 18 Jan 2025 18:06:04 +0900 Subject: [PATCH] trigger `obfuscated_if_else` for `.then(..).unwrap_or(..)` --- clippy_lints/src/methods/mod.rs | 8 +++--- .../src/methods/obfuscated_if_else.rs | 25 +++++++++++++------ tests/ui/obfuscated_if_else.fixed | 2 ++ tests/ui/obfuscated_if_else.rs | 2 ++ tests/ui/obfuscated_if_else.stderr | 10 ++++++-- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d7e9f65bfa3a..666e5b848ba7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2417,14 +2417,14 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `.then_some(..).unwrap_or(..)` + /// Checks for unnecessary method chains that can be simplified into `if .. else ..`. /// /// ### Why is this bad? /// This can be written more clearly with `if .. else ..` /// /// ### Limitations /// This lint currently only looks for usages of - /// `.then_some(..).unwrap_or(..)`, but will be expanded + /// `.then_some(..).unwrap_or(..)` and `.then(..).unwrap_or(..)`, but will be expanded /// to account for similar patterns. /// /// ### Example @@ -5250,8 +5250,8 @@ impl Methods { Some(("map", m_recv, [m_arg], span, _)) => { option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, &self.msrv); }, - Some(("then_some", t_recv, [t_arg], _, _)) => { - obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg); + Some((then_method @ ("then" | "then_some"), t_recv, [t_arg], _, _)) => { + obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg, then_method); }, _ => {}, } diff --git a/clippy_lints/src/methods/obfuscated_if_else.rs b/clippy_lints/src/methods/obfuscated_if_else.rs index 697eab32a33b..f36d4f3a085e 100644 --- a/clippy_lints/src/methods/obfuscated_if_else.rs +++ b/clippy_lints/src/methods/obfuscated_if_else.rs @@ -1,8 +1,9 @@ use super::OBFUSCATED_IF_ELSE; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::{snippet, snippet_with_applicability}; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::ExprKind; use rustc_lint::LateContext; pub(super) fn check<'tcx>( @@ -11,19 +12,25 @@ pub(super) fn check<'tcx>( then_recv: &'tcx hir::Expr<'_>, then_arg: &'tcx hir::Expr<'_>, unwrap_arg: &'tcx hir::Expr<'_>, + then_method_name: &str, ) { - // something.then_some(blah).unwrap_or(blah) - // ^^^^^^^^^-then_recv ^^^^-then_arg ^^^^- unwrap_arg - // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr - let recv_ty = cx.typeck_results().expr_ty(then_recv); if recv_ty.is_bool() { let mut applicability = Applicability::MachineApplicable; + let if_then = match then_method_name { + "then" if let ExprKind::Closure(closure) = then_arg.kind => { + let body = cx.tcx.hir().body(closure.body); + snippet(cx, body.value.span, "..") + }, + "then_some" => snippet_with_applicability(cx, then_arg.span, "..", &mut applicability), + _ => String::new().into(), + }; + let sugg = format!( "if {} {{ {} }} else {{ {} }}", snippet_with_applicability(cx, then_recv.span, "..", &mut applicability), - snippet_with_applicability(cx, then_arg.span, "..", &mut applicability), + if_then, snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability) ); @@ -31,8 +38,10 @@ pub(super) fn check<'tcx>( cx, OBFUSCATED_IF_ELSE, expr.span, - "use of `.then_some(..).unwrap_or(..)` can be written \ - more clearly with `if .. else ..`", + format!( + "use of `.{then_method_name}(..).unwrap_or(..)` can be written \ + more clearly with `if .. else ..`" + ), "try", sugg, applicability, diff --git a/tests/ui/obfuscated_if_else.fixed b/tests/ui/obfuscated_if_else.fixed index c5ee569800ab..1ba8490d5ed3 100644 --- a/tests/ui/obfuscated_if_else.fixed +++ b/tests/ui/obfuscated_if_else.fixed @@ -1,5 +1,7 @@ #![warn(clippy::obfuscated_if_else)] +#![allow(clippy::unnecessary_lazy_evaluations)] fn main() { if true { "a" } else { "b" }; + if true { "a" } else { "b" }; } diff --git a/tests/ui/obfuscated_if_else.rs b/tests/ui/obfuscated_if_else.rs index 2b60c855a555..b9e4e4321a9d 100644 --- a/tests/ui/obfuscated_if_else.rs +++ b/tests/ui/obfuscated_if_else.rs @@ -1,5 +1,7 @@ #![warn(clippy::obfuscated_if_else)] +#![allow(clippy::unnecessary_lazy_evaluations)] fn main() { true.then_some("a").unwrap_or("b"); + true.then(|| "a").unwrap_or("b"); } diff --git a/tests/ui/obfuscated_if_else.stderr b/tests/ui/obfuscated_if_else.stderr index d4c2f9b331a8..b114c7fc1239 100644 --- a/tests/ui/obfuscated_if_else.stderr +++ b/tests/ui/obfuscated_if_else.stderr @@ -1,5 +1,5 @@ error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..` - --> tests/ui/obfuscated_if_else.rs:4:5 + --> tests/ui/obfuscated_if_else.rs:5:5 | LL | true.then_some("a").unwrap_or("b"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }` @@ -7,5 +7,11 @@ LL | true.then_some("a").unwrap_or("b"); = note: `-D clippy::obfuscated-if-else` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::obfuscated_if_else)]` -error: aborting due to 1 previous error +error: use of `.then(..).unwrap_or(..)` can be written more clearly with `if .. else ..` + --> tests/ui/obfuscated_if_else.rs:6:5 + | +LL | true.then(|| "a").unwrap_or("b"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }` + +error: aborting due to 2 previous errors