Skip to content

Commit

Permalink
trigger obfuscated_if_else for .then(..).unwrap_or(..)
Browse files Browse the repository at this point in the history
  • Loading branch information
lapla-cogito committed Jan 18, 2025
1 parent e359e88 commit ef52d42
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 16 deletions.
8 changes: 4 additions & 4 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
},
_ => {},
}
Expand Down
40 changes: 31 additions & 9 deletions clippy_lints/src/methods/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
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 clippy_utils::sugg::has_enclosing_paren;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::ExprKind;
use rustc_lint::LateContext;

pub(super) fn check<'tcx>(
Expand All @@ -11,31 +13,51 @@ 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),
remove_needless_parentheses(&snippet_with_applicability(
cx,
then_recv.span,
"..",
&mut applicability
)),
if_then,
snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
);

span_lint_and_sugg(
cx,
OBFUSCATED_IF_ELSE,
expr.span,
"use of `.then_some(..).unwrap_or(..)` can be written \
more clearly with `if .. else ..`",
"use of method chains can be written more clearly with `if .. else ..`",
"try",
sugg,
applicability,
);
}
}

// Remove the enclosing parentheses if they exist
// e.g. "(a == 1)" -> "a == 1"
fn remove_needless_parentheses(s: &str) -> String {
if has_enclosing_paren(s) {
s[1..s.len() - 1].to_string()
} else {
s.to_string()
}
}
6 changes: 6 additions & 0 deletions tests/ui/obfuscated_if_else.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
#![warn(clippy::obfuscated_if_else)]
#![allow(clippy::unnecessary_lazy_evaluations)]

fn main() {
if true { "a" } else { "b" };
if true { "a" } else { "b" };

let a = 1;
if a == 1 { "a" } else { "b" };
if a == 1 { "a" } else { "b" };
}
6 changes: 6 additions & 0 deletions tests/ui/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
#![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");

let a = 1;
(a == 1).then_some("a").unwrap_or("b");
(a == 1).then(|| "a").unwrap_or("b");
}
24 changes: 21 additions & 3 deletions tests/ui/obfuscated_if_else.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:4:5
error: use of method chains can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:5:5
|
LL | true.then_some("a").unwrap_or("b");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "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 method chains 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: use of method chains can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:9:5
|
LL | (a == 1).then_some("a").unwrap_or("b");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`

error: use of method chains can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:10:5
|
LL | (a == 1).then(|| "a").unwrap_or("b");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`

error: aborting due to 4 previous errors

0 comments on commit ef52d42

Please sign in to comment.