Skip to content

Commit 106db26

Browse files
authored
match_bool: fix suggestion if guard is present (rust-lang#14039)
Without this check, the lint would suggest that ```rust match test { true if option == 5 => 10, _ => 1, }; ``` is replaced by `if test { 10 } else { 1 }`. changelog: [`match_bool`]: omit suggestion when guards are present in `match` expression
2 parents c024c13 + 0c3deeb commit 106db26

File tree

4 files changed

+195
-54
lines changed

4 files changed

+195
-54
lines changed

clippy_lints/src/matches/match_bool.rs

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::is_unit_expr;
3-
use clippy_utils::source::{expr_block, snippet};
3+
use clippy_utils::source::expr_block;
44
use clippy_utils::sugg::Sugg;
55
use rustc_ast::LitKind;
66
use rustc_errors::Applicability;
@@ -20,54 +20,57 @@ pub(crate) fn check(cx: &LateContext<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]
2020
"you seem to be trying to match on a boolean expression",
2121
move |diag| {
2222
if arms.len() == 2 {
23-
// no guards
24-
let exprs = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
23+
let mut app = Applicability::MachineApplicable;
24+
let test_sugg = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
25+
let test = Sugg::hir_with_applicability(cx, scrutinee, "_", &mut app);
2526
if let ExprKind::Lit(lit) = arm_bool.kind {
26-
match lit.node {
27-
LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
28-
LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
27+
match &lit.node {
28+
LitKind::Bool(true) => Some(test),
29+
LitKind::Bool(false) => Some(!test),
2930
_ => None,
3031
}
32+
.map(|test| {
33+
if let Some(guard) = &arms[0]
34+
.guard
35+
.map(|g| Sugg::hir_with_applicability(cx, g, "_", &mut app))
36+
{
37+
test.and(guard)
38+
} else {
39+
test
40+
}
41+
})
3142
} else {
3243
None
3344
}
3445
} else {
3546
None
3647
};
3748

38-
if let Some((true_expr, false_expr)) = exprs {
39-
let mut app = Applicability::HasPlaceholders;
49+
if let Some(test_sugg) = test_sugg {
4050
let ctxt = expr.span.ctxt();
51+
let (true_expr, false_expr) = (arms[0].body, arms[1].body);
4152
let sugg = match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
4253
(false, false) => Some(format!(
4354
"if {} {} else {}",
44-
snippet(cx, scrutinee.span, "b"),
55+
test_sugg,
4556
expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app),
4657
expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
4758
)),
4859
(false, true) => Some(format!(
4960
"if {} {}",
50-
snippet(cx, scrutinee.span, "b"),
61+
test_sugg,
5162
expr_block(cx, true_expr, ctxt, "..", Some(expr.span), &mut app)
5263
)),
53-
(true, false) => {
54-
let test = Sugg::hir(cx, scrutinee, "..");
55-
Some(format!(
56-
"if {} {}",
57-
!test,
58-
expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
59-
))
60-
},
64+
(true, false) => Some(format!(
65+
"if {} {}",
66+
!test_sugg,
67+
expr_block(cx, false_expr, ctxt, "..", Some(expr.span), &mut app)
68+
)),
6169
(true, true) => None,
6270
};
6371

6472
if let Some(sugg) = sugg {
65-
diag.span_suggestion(
66-
expr.span,
67-
"consider using an `if`/`else` expression",
68-
sugg,
69-
Applicability::HasPlaceholders,
70-
);
73+
diag.span_suggestion(expr.span, "consider using an `if`/`else` expression", sugg, app);
7174
}
7275
}
7376
}

tests/ui/match_bool.fixed

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
#![deny(clippy::match_bool)]
2+
#![allow(clippy::nonminimal_bool, clippy::eq_op)]
3+
4+
fn match_bool() {
5+
let test: bool = true;
6+
7+
if test { 0 } else { 42 };
8+
9+
let option = 1;
10+
if option == 1 { 1 } else { 0 };
11+
12+
if !test {
13+
println!("Noooo!");
14+
};
15+
16+
if !test {
17+
println!("Noooo!");
18+
};
19+
20+
if !(test && test) {
21+
println!("Noooo!");
22+
};
23+
24+
if !test {
25+
println!("Noooo!");
26+
} else {
27+
println!("Yes!");
28+
};
29+
30+
// Not linted
31+
match option {
32+
1..=10 => 1,
33+
11..=20 => 2,
34+
_ => 3,
35+
};
36+
37+
// Don't lint
38+
let _ = match test {
39+
#[cfg(feature = "foo")]
40+
true if option == 5 => 10,
41+
true => 0,
42+
false => 1,
43+
};
44+
45+
let _ = if test && option == 5 { 10 } else { 1 };
46+
47+
let _ = if !test && option == 5 { 10 } else { 1 };
48+
49+
if test && option == 5 { println!("Hello") };
50+
51+
if !(test && option == 5) { println!("Hello") };
52+
53+
if !test && option == 5 { println!("Hello") };
54+
55+
if !(!test && option == 5) { println!("Hello") };
56+
}
57+
58+
fn main() {}

tests/ui/match_bool.rs

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
//@no-rustfix: overlapping suggestions
21
#![deny(clippy::match_bool)]
2+
#![allow(clippy::nonminimal_bool, clippy::eq_op)]
33

44
fn match_bool() {
55
let test: bool = true;
@@ -34,11 +34,7 @@ fn match_bool() {
3434
};
3535

3636
match test && test {
37-
//~^ ERROR: this boolean expression can be simplified
38-
//~| NOTE: `-D clippy::nonminimal-bool` implied by `-D warnings`
39-
//~| ERROR: you seem to be trying to match on a boolean expression
40-
//~| ERROR: equal expressions as operands to `&&`
41-
//~| NOTE: `#[deny(clippy::eq_op)]` on by default
37+
//~^ ERROR: you seem to be trying to match on a boolean expression
4238
false => {
4339
println!("Noooo!");
4440
},
@@ -69,6 +65,42 @@ fn match_bool() {
6965
true => 0,
7066
false => 1,
7167
};
68+
69+
let _ = match test {
70+
//~^ ERROR: you seem to be trying to match on a boolean expression
71+
true if option == 5 => 10,
72+
_ => 1,
73+
};
74+
75+
let _ = match test {
76+
//~^ ERROR: you seem to be trying to match on a boolean expression
77+
false if option == 5 => 10,
78+
_ => 1,
79+
};
80+
81+
match test {
82+
//~^ ERROR: you seem to be trying to match on a boolean expression
83+
true if option == 5 => println!("Hello"),
84+
_ => (),
85+
};
86+
87+
match test {
88+
//~^ ERROR: you seem to be trying to match on a boolean expression
89+
true if option == 5 => (),
90+
_ => println!("Hello"),
91+
};
92+
93+
match test {
94+
//~^ ERROR: you seem to be trying to match on a boolean expression
95+
false if option == 5 => println!("Hello"),
96+
_ => (),
97+
};
98+
99+
match test {
100+
//~^ ERROR: you seem to be trying to match on a boolean expression
101+
false if option == 5 => (),
102+
_ => println!("Hello"),
103+
};
72104
}
73105

74106
fn main() {}

tests/ui/match_bool.stderr

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,3 @@
1-
error: this boolean expression can be simplified
2-
--> tests/ui/match_bool.rs:36:11
3-
|
4-
LL | match test && test {
5-
| ^^^^^^^^^^^^ help: try: `test`
6-
|
7-
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
8-
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
9-
101
error: you seem to be trying to match on a boolean expression
112
--> tests/ui/match_bool.rs:7:5
123
|
@@ -18,7 +9,7 @@ LL | | };
189
| |_____^ help: consider using an `if`/`else` expression: `if test { 0 } else { 42 }`
1910
|
2011
note: the lint level is defined here
21-
--> tests/ui/match_bool.rs:2:9
12+
--> tests/ui/match_bool.rs:1:9
2213
|
2314
LL | #![deny(clippy::match_bool)]
2415
| ^^^^^^^^^^^^^^^^^^
@@ -75,7 +66,10 @@ error: you seem to be trying to match on a boolean expression
7566
--> tests/ui/match_bool.rs:36:5
7667
|
7768
LL | / match test && test {
78-
... |
69+
LL | |
70+
LL | | false => {
71+
LL | | println!("Noooo!");
72+
LL | | },
7973
LL | | _ => (),
8074
LL | | };
8175
| |_____^
@@ -87,16 +81,8 @@ LL + println!("Noooo!");
8781
LL ~ };
8882
|
8983

90-
error: equal expressions as operands to `&&`
91-
--> tests/ui/match_bool.rs:36:11
92-
|
93-
LL | match test && test {
94-
| ^^^^^^^^^^^^
95-
|
96-
= note: `#[deny(clippy::eq_op)]` on by default
97-
9884
error: you seem to be trying to match on a boolean expression
99-
--> tests/ui/match_bool.rs:48:5
85+
--> tests/ui/match_bool.rs:44:5
10086
|
10187
LL | / match test {
10288
LL | |
@@ -109,12 +95,74 @@ LL | | };
10995
|
11096
help: consider using an `if`/`else` expression
11197
|
112-
LL ~ if test {
113-
LL + println!("Yes!");
114-
LL + } else {
98+
LL ~ if !test {
11599
LL + println!("Noooo!");
100+
LL + } else {
101+
LL + println!("Yes!");
116102
LL ~ };
117103
|
118104

119-
error: aborting due to 8 previous errors
105+
error: you seem to be trying to match on a boolean expression
106+
--> tests/ui/match_bool.rs:69:13
107+
|
108+
LL | let _ = match test {
109+
| _____________^
110+
LL | |
111+
LL | | true if option == 5 => 10,
112+
LL | | _ => 1,
113+
LL | | };
114+
| |_____^ help: consider using an `if`/`else` expression: `if test && option == 5 { 10 } else { 1 }`
115+
116+
error: you seem to be trying to match on a boolean expression
117+
--> tests/ui/match_bool.rs:75:13
118+
|
119+
LL | let _ = match test {
120+
| _____________^
121+
LL | |
122+
LL | | false if option == 5 => 10,
123+
LL | | _ => 1,
124+
LL | | };
125+
| |_____^ help: consider using an `if`/`else` expression: `if !test && option == 5 { 10 } else { 1 }`
126+
127+
error: you seem to be trying to match on a boolean expression
128+
--> tests/ui/match_bool.rs:81:5
129+
|
130+
LL | / match test {
131+
LL | |
132+
LL | | true if option == 5 => println!("Hello"),
133+
LL | | _ => (),
134+
LL | | };
135+
| |_____^ help: consider using an `if`/`else` expression: `if test && option == 5 { println!("Hello") }`
136+
137+
error: you seem to be trying to match on a boolean expression
138+
--> tests/ui/match_bool.rs:87:5
139+
|
140+
LL | / match test {
141+
LL | |
142+
LL | | true if option == 5 => (),
143+
LL | | _ => println!("Hello"),
144+
LL | | };
145+
| |_____^ help: consider using an `if`/`else` expression: `if !(test && option == 5) { println!("Hello") }`
146+
147+
error: you seem to be trying to match on a boolean expression
148+
--> tests/ui/match_bool.rs:93:5
149+
|
150+
LL | / match test {
151+
LL | |
152+
LL | | false if option == 5 => println!("Hello"),
153+
LL | | _ => (),
154+
LL | | };
155+
| |_____^ help: consider using an `if`/`else` expression: `if !test && option == 5 { println!("Hello") }`
156+
157+
error: you seem to be trying to match on a boolean expression
158+
--> tests/ui/match_bool.rs:99:5
159+
|
160+
LL | / match test {
161+
LL | |
162+
LL | | false if option == 5 => (),
163+
LL | | _ => println!("Hello"),
164+
LL | | };
165+
| |_____^ help: consider using an `if`/`else` expression: `if !(!test && option == 5) { println!("Hello") }`
166+
167+
error: aborting due to 12 previous errors
120168

0 commit comments

Comments
 (0)