Skip to content

Commit

Permalink
change the applicability of obfuscated_if_else depending on whether…
Browse files Browse the repository at this point in the history
… the original code can have side effects
  • Loading branch information
lapla-cogito committed Jan 26, 2025
1 parent 9135923 commit 5fc81d0
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 6 deletions.
8 changes: 7 additions & 1 deletion clippy_lints/src/methods/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::OBFUSCATED_IF_ELSE;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability;
Expand All @@ -18,7 +19,12 @@ pub(super) fn check<'tcx>(
let recv_ty = cx.typeck_results().expr_ty(then_recv);

if recv_ty.is_bool() {
let mut applicability = Applicability::MachineApplicable;
let mut applicability = if switch_to_eager_eval(cx, then_arg) && switch_to_eager_eval(cx, unwrap_arg) {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};

let if_then = match then_method_name {
"then" if let ExprKind::Closure(closure) = then_arg.kind => {
let body = cx.tcx.hir().body(closure.body);
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/obfuscated_if_else.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![warn(clippy::obfuscated_if_else)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(clippy::unit_arg, clippy::unused_unit)]

fn main() {
if true { "a" } else { "b" };
Expand All @@ -11,4 +12,8 @@ fn main() {

let partial = (a == 1).then_some("a");
partial.unwrap_or("b"); // not lint

let mut a = 0;
if true { a += 1 } else { () };
if true { () } else { a += 2 };
}
5 changes: 5 additions & 0 deletions tests/ui/obfuscated_if_else.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![warn(clippy::obfuscated_if_else)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(clippy::unit_arg, clippy::unused_unit)]

fn main() {
true.then_some("a").unwrap_or("b");
Expand All @@ -11,4 +12,8 @@ fn main() {

let partial = (a == 1).then_some("a");
partial.unwrap_or("b"); // not lint

let mut a = 0;
true.then_some(a += 1).unwrap_or(());
true.then_some(()).unwrap_or(a += 2);
}
22 changes: 17 additions & 5 deletions tests/ui/obfuscated_if_else.stderr
Original file line number Diff line number Diff line change
@@ -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:6:5
|
LL | true.then_some("a").unwrap_or("b");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { "a" } else { "b" }`
Expand All @@ -8,22 +8,34 @@ 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:6:5
--> tests/ui/obfuscated_if_else.rs:7: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:9:5
--> tests/ui/obfuscated_if_else.rs:10: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:10:5
--> tests/ui/obfuscated_if_else.rs:11:5
|
LL | (a == 1).then(|| "a").unwrap_or("b");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`

error: aborting due to 4 previous errors
error: this method chain can be written more clearly with `if .. else ..`
--> tests/ui/obfuscated_if_else.rs:17: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:18:5
|
LL | true.then_some(()).unwrap_or(a += 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }`

error: aborting due to 6 previous errors

0 comments on commit 5fc81d0

Please sign in to comment.