Skip to content

Commit 9ff7c91

Browse files
committed
Add new lint obfuscated_if_else
New lint suggests using `if .. else ..` instead of `.then_some(..).unwrap_or(..)`.
1 parent 7ea4592 commit 9ff7c91

9 files changed

+104
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3792,6 +3792,7 @@ Released 2018-09-13
37923792
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
37933793
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
37943794
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
3795+
[`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else
37953796
[`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
37963797
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
37973798
[`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion

clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
188188
LintId::of(methods::NEEDLESS_SPLITN),
189189
LintId::of(methods::NEW_RET_NO_SELF),
190190
LintId::of(methods::NO_EFFECT_REPLACE),
191+
LintId::of(methods::OBFUSCATED_IF_ELSE),
191192
LintId::of(methods::OK_EXPECT),
192193
LintId::of(methods::OPTION_AS_REF_DEREF),
193194
LintId::of(methods::OPTION_FILTER_MAP),

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ store.register_lints(&[
330330
methods::NEEDLESS_SPLITN,
331331
methods::NEW_RET_NO_SELF,
332332
methods::NO_EFFECT_REPLACE,
333+
methods::OBFUSCATED_IF_ELSE,
333334
methods::OK_EXPECT,
334335
methods::OPTION_AS_REF_DEREF,
335336
methods::OPTION_FILTER_MAP,

clippy_lints/src/lib.register_style.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
7070
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
7171
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
7272
LintId::of(methods::NEW_RET_NO_SELF),
73+
LintId::of(methods::OBFUSCATED_IF_ELSE),
7374
LintId::of(methods::OK_EXPECT),
7475
LintId::of(methods::OPTION_MAP_OR_NONE),
7576
LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),

clippy_lints/src/methods/mod.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ mod map_unwrap_or;
4646
mod needless_option_as_deref;
4747
mod needless_option_take;
4848
mod no_effect_replace;
49+
mod obfuscated_if_else;
4950
mod ok_expect;
5051
mod option_as_ref_deref;
5152
mod option_map_or_none;
@@ -2259,6 +2260,35 @@ declare_clippy_lint! {
22592260
"replace with no effect"
22602261
}
22612262

2263+
declare_clippy_lint! {
2264+
/// ### What it does
2265+
/// Checks for usages of `.then_some(..).unwrap_or(..)`
2266+
///
2267+
/// ### Why is this bad?
2268+
/// This can be written more clearly with `if .. else ..`
2269+
///
2270+
/// ### Limitations
2271+
/// This lint currently only looks for usages of
2272+
/// `.then_some(..).unwrap_or(..)`, but will be expanded
2273+
/// to account for similar patterns.
2274+
///
2275+
/// ### Example
2276+
/// ```rust
2277+
/// let x = true;
2278+
/// x.then_some("a").unwrap_or("b");
2279+
/// ```
2280+
/// Use instead:
2281+
/// ```rust
2282+
/// let x = true;
2283+
/// if x { "a" } else { "b" };
2284+
/// ```
2285+
#[clippy::version = "1.64.0"]
2286+
pub OBFUSCATED_IF_ELSE,
2287+
style,
2288+
"use of `.then_some(..).unwrap_or(..)` can be written \
2289+
more clearly with `if .. else ..`"
2290+
}
2291+
22622292
pub struct Methods {
22632293
avoid_breaking_exported_api: bool,
22642294
msrv: Option<RustcVersion>,
@@ -2360,6 +2390,7 @@ impl_lint_pass!(Methods => [
23602390
IS_DIGIT_ASCII_RADIX,
23612391
NEEDLESS_OPTION_TAKE,
23622392
NO_EFFECT_REPLACE,
2393+
OBFUSCATED_IF_ELSE,
23632394
]);
23642395

23652396
/// Extracts a method call name, args, and `Span` of the method name.
@@ -2768,6 +2799,9 @@ impl Methods {
27682799
Some(("map", [m_recv, m_arg], span)) => {
27692800
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
27702801
},
2802+
Some(("then_some", [t_recv, t_arg], _)) => {
2803+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
2804+
},
27712805
_ => {},
27722806
},
27732807
("unwrap_or_else", [u_arg]) => match method_call(recv) {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// run-rustfix
2+
3+
use super::OBFUSCATED_IF_ELSE;
4+
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_with_applicability};
5+
use rustc_errors::Applicability;
6+
use rustc_hir as hir;
7+
use rustc_lint::LateContext;
8+
9+
pub(super) fn check<'tcx>(
10+
cx: &LateContext<'tcx>,
11+
expr: &'tcx hir::Expr<'_>,
12+
then_recv: &'tcx hir::Expr<'_>,
13+
then_arg: &'tcx hir::Expr<'_>,
14+
unwrap_arg: &'tcx hir::Expr<'_>,
15+
) {
16+
// something.then_some(blah).unwrap_or(blah)
17+
// ^^^^^^^^^-then_recv ^^^^-then_arg ^^^^- unwrap_arg
18+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr
19+
20+
let recv_ty = cx.typeck_results().expr_ty(then_recv);
21+
22+
if recv_ty.is_bool() {
23+
let mut applicability = Applicability::MachineApplicable;
24+
let sugg = format!(
25+
"if {} {{ {} }} else {{ {} }}",
26+
snippet_with_applicability(cx, then_recv.span, "..", &mut applicability),
27+
snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
28+
snippet_with_applicability(cx, unwrap_arg.span, "..", &mut applicability)
29+
);
30+
31+
span_lint_and_sugg(
32+
cx,
33+
OBFUSCATED_IF_ELSE,
34+
expr.span,
35+
"use of `.then_some(..).unwrap_or(..)` can be written \
36+
more clearly with `if .. else ..`",
37+
"try",
38+
sugg,
39+
applicability,
40+
);
41+
}
42+
}

tests/ui/obfuscated_if_else.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::obfuscated_if_else)]
4+
5+
fn main() {
6+
if true { "a" } else { "b" };
7+
}

tests/ui/obfuscated_if_else.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::obfuscated_if_else)]
4+
5+
fn main() {
6+
true.then_some("a").unwrap_or("b");
7+
}

tests/ui/obfuscated_if_else.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..`
2+
--> $DIR/obfuscated_if_else.rs:6:5
3+
|
4+
LL | true.then_some("a").unwrap_or("b");
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
6+
|
7+
= note: `-D clippy::obfuscated-if-else` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)