Skip to content

Commit 949b3bc

Browse files
authored
bool_to_int_with_if: properly handle macros (#14629)
- Do not replace macro results in then/else branches - Extract condition snippet from the right context - Make suggestion `MaybeIncorrect` if it would lead to losing comments changelog: [`bool_to_int_with_if`]: properly handle macros Fixes #14628
2 parents 222660b + 0bfcd0d commit 949b3bc

File tree

4 files changed

+83
-10
lines changed

4 files changed

+83
-10
lines changed

clippy_lints/src/bool_to_int_with_if.rs

+18-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::HasSession;
23
use clippy_utils::sugg::Sugg;
3-
use clippy_utils::{is_else_clause, is_in_const_context};
4+
use clippy_utils::{higher, is_else_clause, is_in_const_context, span_contains_comment};
45
use rustc_ast::LitKind;
56
use rustc_errors::Applicability;
67
use rustc_hir::{Expr, ExprKind};
@@ -46,18 +47,25 @@ declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]);
4647

4748
impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf {
4849
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
49-
if let ExprKind::If(cond, then, Some(else_)) = expr.kind
50-
&& matches!(cond.kind, ExprKind::DropTemps(_))
50+
if !expr.span.from_expansion()
51+
&& let Some(higher::If {
52+
cond,
53+
then,
54+
r#else: Some(r#else),
55+
}) = higher::If::hir(expr)
5156
&& let Some(then_lit) = as_int_bool_lit(then)
52-
&& let Some(else_lit) = as_int_bool_lit(else_)
57+
&& let Some(else_lit) = as_int_bool_lit(r#else)
5358
&& then_lit != else_lit
54-
&& !expr.span.from_expansion()
5559
&& !is_in_const_context(cx)
5660
{
5761
let ty = cx.typeck_results().expr_ty(then);
58-
let mut applicability = Applicability::MachineApplicable;
62+
let mut applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
63+
Applicability::MaybeIncorrect
64+
} else {
65+
Applicability::MachineApplicable
66+
};
5967
let snippet = {
60-
let mut sugg = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability);
68+
let mut sugg = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "..", &mut applicability);
6169
if !then_lit {
6270
sugg = !sugg;
6371
}
@@ -91,10 +99,11 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf {
9199
}
92100
}
93101

94-
fn as_int_bool_lit(e: &Expr<'_>) -> Option<bool> {
95-
if let ExprKind::Block(b, _) = e.kind
102+
fn as_int_bool_lit(expr: &Expr<'_>) -> Option<bool> {
103+
if let ExprKind::Block(b, _) = expr.kind
96104
&& b.stmts.is_empty()
97105
&& let Some(e) = b.expr
106+
&& !e.span.from_expansion()
98107
&& let ExprKind::Lit(lit) = e.kind
99108
&& let LitKind::Int(x, _) = lit.node
100109
{

tests/ui/bool_to_int_with_if.fixed

+24
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,27 @@ fn if_let(a: Enum, b: Enum) {
117117
0
118118
};
119119
}
120+
121+
fn issue14628() {
122+
macro_rules! mac {
123+
(if $cond:expr, $then:expr, $else:expr) => {
124+
if $cond { $then } else { $else }
125+
};
126+
(zero) => {
127+
0
128+
};
129+
(one) => {
130+
1
131+
};
132+
}
133+
134+
let _ = i32::from(dbg!(4 > 0));
135+
//~^ bool_to_int_with_if
136+
137+
let _ = dbg!(i32::from(4 > 0));
138+
//~^ bool_to_int_with_if
139+
140+
let _ = mac!(if 4 > 0, 1, 0);
141+
let _ = if 4 > 0 { mac!(one) } else { 0 };
142+
let _ = if 4 > 0 { 1 } else { mac!(zero) };
143+
}

tests/ui/bool_to_int_with_if.rs

+24
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,27 @@ fn if_let(a: Enum, b: Enum) {
157157
0
158158
};
159159
}
160+
161+
fn issue14628() {
162+
macro_rules! mac {
163+
(if $cond:expr, $then:expr, $else:expr) => {
164+
if $cond { $then } else { $else }
165+
};
166+
(zero) => {
167+
0
168+
};
169+
(one) => {
170+
1
171+
};
172+
}
173+
174+
let _ = if dbg!(4 > 0) { 1 } else { 0 };
175+
//~^ bool_to_int_with_if
176+
177+
let _ = dbg!(if 4 > 0 { 1 } else { 0 });
178+
//~^ bool_to_int_with_if
179+
180+
let _ = mac!(if 4 > 0, 1, 0);
181+
let _ = if 4 > 0 { mac!(one) } else { 0 };
182+
let _ = if 4 > 0 { 1 } else { mac!(zero) };
183+
}

tests/ui/bool_to_int_with_if.stderr

+17-1
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,21 @@ LL | if a { 1 } else { 0 }
114114
|
115115
= note: `a as u8` or `a.into()` can also be valid options
116116

117-
error: aborting due to 9 previous errors
117+
error: boolean to int conversion using if
118+
--> tests/ui/bool_to_int_with_if.rs:174:13
119+
|
120+
LL | let _ = if dbg!(4 > 0) { 1 } else { 0 };
121+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(dbg!(4 > 0))`
122+
|
123+
= note: `dbg!(4 > 0) as i32` or `dbg!(4 > 0).into()` can also be valid options
124+
125+
error: boolean to int conversion using if
126+
--> tests/ui/bool_to_int_with_if.rs:177:18
127+
|
128+
LL | let _ = dbg!(if 4 > 0 { 1 } else { 0 });
129+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(4 > 0)`
130+
|
131+
= note: `(4 > 0) as i32` or `(4 > 0).into()` can also be valid options
132+
133+
error: aborting due to 11 previous errors
118134

0 commit comments

Comments
 (0)