Skip to content

Commit

Permalink
Add new lint obfuscated_if_else
Browse files Browse the repository at this point in the history
New lint suggests using `if .. else ..` instead of
`.then_some(..).unwrap_or(..)`.
  • Loading branch information
arieluy committed Jul 18, 2022
1 parent 7ea4592 commit 9ff7c91
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3792,6 +3792,7 @@ Released 2018-09-13
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
[`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else
[`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
[`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::NEEDLESS_SPLITN),
LintId::of(methods::NEW_RET_NO_SELF),
LintId::of(methods::NO_EFFECT_REPLACE),
LintId::of(methods::OBFUSCATED_IF_ELSE),
LintId::of(methods::OK_EXPECT),
LintId::of(methods::OPTION_AS_REF_DEREF),
LintId::of(methods::OPTION_FILTER_MAP),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ store.register_lints(&[
methods::NEEDLESS_SPLITN,
methods::NEW_RET_NO_SELF,
methods::NO_EFFECT_REPLACE,
methods::OBFUSCATED_IF_ELSE,
methods::OK_EXPECT,
methods::OPTION_AS_REF_DEREF,
methods::OPTION_FILTER_MAP,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
LintId::of(methods::NEW_RET_NO_SELF),
LintId::of(methods::OBFUSCATED_IF_ELSE),
LintId::of(methods::OK_EXPECT),
LintId::of(methods::OPTION_MAP_OR_NONE),
LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
Expand Down
34 changes: 34 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ mod map_unwrap_or;
mod needless_option_as_deref;
mod needless_option_take;
mod no_effect_replace;
mod obfuscated_if_else;
mod ok_expect;
mod option_as_ref_deref;
mod option_map_or_none;
Expand Down Expand Up @@ -2259,6 +2260,35 @@ declare_clippy_lint! {
"replace with no effect"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `.then_some(..).unwrap_or(..)`
///
/// ### 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
/// to account for similar patterns.
///
/// ### Example
/// ```rust
/// let x = true;
/// x.then_some("a").unwrap_or("b");
/// ```
/// Use instead:
/// ```rust
/// let x = true;
/// if x { "a" } else { "b" };
/// ```
#[clippy::version = "1.64.0"]
pub OBFUSCATED_IF_ELSE,
style,
"use of `.then_some(..).unwrap_or(..)` can be written \
more clearly with `if .. else ..`"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
Expand Down Expand Up @@ -2360,6 +2390,7 @@ impl_lint_pass!(Methods => [
IS_DIGIT_ASCII_RADIX,
NEEDLESS_OPTION_TAKE,
NO_EFFECT_REPLACE,
OBFUSCATED_IF_ELSE,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -2768,6 +2799,9 @@ impl Methods {
Some(("map", [m_recv, m_arg], span)) => {
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
},
Some(("then_some", [t_recv, t_arg], _)) => {
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
},
_ => {},
},
("unwrap_or_else", [u_arg]) => match method_call(recv) {
Expand Down
42 changes: 42 additions & 0 deletions clippy_lints/src/methods/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// run-rustfix

use super::OBFUSCATED_IF_ELSE;
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_with_applicability};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;

pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
then_recv: &'tcx hir::Expr<'_>,
then_arg: &'tcx hir::Expr<'_>,
unwrap_arg: &'tcx hir::Expr<'_>,
) {
// 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 sugg = format!(
"if {} {{ {} }} else {{ {} }}",
snippet_with_applicability(cx, then_recv.span, "..", &mut applicability),
snippet_with_applicability(cx, then_arg.span, "..", &mut applicability),
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 ..`",
"try",
sugg,
applicability,
);
}
}
7 changes: 7 additions & 0 deletions tests/ui/obfuscated_if_else.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// run-rustfix

#![warn(clippy::obfuscated_if_else)]

fn main() {
if true { "a" } else { "b" };
}
7 changes: 7 additions & 0 deletions tests/ui/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// run-rustfix

#![warn(clippy::obfuscated_if_else)]

fn main() {
true.then_some("a").unwrap_or("b");
}
10 changes: 10 additions & 0 deletions tests/ui/obfuscated_if_else.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: use of `.then_some(..).unwrap_or(..)` can be written more clearly with `if .. else ..`
--> $DIR/obfuscated_if_else.rs:6: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`

error: aborting due to previous error

0 comments on commit 9ff7c91

Please sign in to comment.