Skip to content

Commit 66dc8a1

Browse files
authored
proper applicability for obfuscated_if_else (rust-lang#14061)
fix rust-lang#14034 The currect implementation of `obfuscated_if_else` sometimes makes incorrect suggestions when the original code have side effects (see the example in the above issue). I think this can be fixed by changing the applicability depending on whether it can have side effects or not. changelog: [`obfuscated_if_else`]: change applicability when the original code can have side effects
2 parents 83bde36 + 2640f65 commit 66dc8a1

File tree

4 files changed

+30
-4
lines changed

4 files changed

+30
-4
lines changed

clippy_lints/src/methods/obfuscated_if_else.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::OBFUSCATED_IF_ELSE;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
34
use clippy_utils::source::snippet_with_applicability;
45
use clippy_utils::sugg::Sugg;
56
use rustc_errors::Applicability;
@@ -18,7 +19,12 @@ pub(super) fn check<'tcx>(
1819
let recv_ty = cx.typeck_results().expr_ty(then_recv);
1920

2021
if recv_ty.is_bool() {
21-
let mut applicability = Applicability::MachineApplicable;
22+
let mut applicability = if switch_to_eager_eval(cx, then_arg) && switch_to_eager_eval(cx, unwrap_arg) {
23+
Applicability::MachineApplicable
24+
} else {
25+
Applicability::MaybeIncorrect
26+
};
27+
2228
let if_then = match then_method_name {
2329
"then" if let ExprKind::Closure(closure) = then_arg.kind => {
2430
let body = cx.tcx.hir().body(closure.body);

tests/ui/obfuscated_if_else.fixed

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::obfuscated_if_else)]
2-
#![allow(clippy::unnecessary_lazy_evaluations)]
2+
#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)]
33

44
fn main() {
55
if true { "a" } else { "b" };
@@ -11,4 +11,8 @@ fn main() {
1111

1212
let partial = (a == 1).then_some("a");
1313
partial.unwrap_or("b"); // not lint
14+
15+
let mut a = 0;
16+
if true { a += 1 } else { () };
17+
if true { () } else { a += 2 };
1418
}

tests/ui/obfuscated_if_else.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::obfuscated_if_else)]
2-
#![allow(clippy::unnecessary_lazy_evaluations)]
2+
#![allow(clippy::unnecessary_lazy_evaluations, clippy::unit_arg, clippy::unused_unit)]
33

44
fn main() {
55
true.then_some("a").unwrap_or("b");
@@ -11,4 +11,8 @@ fn main() {
1111

1212
let partial = (a == 1).then_some("a");
1313
partial.unwrap_or("b"); // not lint
14+
15+
let mut a = 0;
16+
true.then_some(a += 1).unwrap_or(());
17+
true.then_some(()).unwrap_or(a += 2);
1418
}

tests/ui/obfuscated_if_else.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,17 @@ error: this method chain can be written more clearly with `if .. else ..`
2525
LL | (a == 1).then(|| "a").unwrap_or("b");
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }`
2727

28-
error: aborting due to 4 previous errors
28+
error: this method chain can be written more clearly with `if .. else ..`
29+
--> tests/ui/obfuscated_if_else.rs:16:5
30+
|
31+
LL | true.then_some(a += 1).unwrap_or(());
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { a += 1 } else { () }`
33+
34+
error: this method chain can be written more clearly with `if .. else ..`
35+
--> tests/ui/obfuscated_if_else.rs:17:5
36+
|
37+
LL | true.then_some(()).unwrap_or(a += 2);
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if true { () } else { a += 2 }`
39+
40+
error: aborting due to 6 previous errors
2941

0 commit comments

Comments
 (0)