Skip to content

Commit 334be18

Browse files
committed
Auto merge of #9412 - xFrednet:jst-r-bool-to-int-lint, r=xFrednet
New lint `bool_to_int_with_if` This is a rebased version of #9086 I could sadly not push directly push to the PR branch as it's protected. The lint implementation comes from `@jst-r.` Thank you for the work you put into this :) --- Closes: #8131 Closes: #9086 changelog: Add lint [`bool_to_int_with_if`] r? `@ghost`
2 parents a80e278 + b1f86a4 commit 334be18

12 files changed

+419
-17
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3603,6 +3603,7 @@ Released 2018-09-13
36033603
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
36043604
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
36053605
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
3606+
[`bool_to_int_with_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_to_int_with_if
36063607
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
36073608
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
36083609
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
use rustc_ast::{ExprPrecedence, LitKind};
2+
use rustc_hir::{Block, ExprKind};
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
6+
use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, source::snippet_block_with_applicability};
7+
use rustc_errors::Applicability;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Instead of using an if statement to convert a bool to an int,
12+
/// this lint suggests using a `from()` function or an `as` coercion.
13+
///
14+
/// ### Why is this bad?
15+
/// Coercion or `from()` is idiomatic way to convert bool to a number.
16+
/// Both methods are guaranteed to return 1 for true, and 0 for false.
17+
///
18+
/// See https://doc.rust-lang.org/std/primitive.bool.html#impl-From%3Cbool%3E
19+
///
20+
/// ### Example
21+
/// ```rust
22+
/// # let condition = false;
23+
/// if condition {
24+
/// 1_i64
25+
/// } else {
26+
/// 0
27+
/// };
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// # let condition = false;
32+
/// i64::from(condition);
33+
/// ```
34+
/// or
35+
/// ```rust
36+
/// # let condition = false;
37+
/// condition as i64;
38+
/// ```
39+
#[clippy::version = "1.65.0"]
40+
pub BOOL_TO_INT_WITH_IF,
41+
style,
42+
"using if to convert bool to int"
43+
}
44+
declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]);
45+
46+
impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf {
47+
fn check_expr(&mut self, ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
48+
if !expr.span.from_expansion() {
49+
check_if_else(ctx, expr);
50+
}
51+
}
52+
}
53+
54+
fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
55+
if let ExprKind::If(check, then, Some(else_)) = expr.kind
56+
&& let Some(then_lit) = int_literal(then)
57+
&& let Some(else_lit) = int_literal(else_)
58+
&& check_int_literal_equals_val(then_lit, 1)
59+
&& check_int_literal_equals_val(else_lit, 0)
60+
{
61+
let mut applicability = Applicability::MachineApplicable;
62+
let snippet = snippet_block_with_applicability(ctx, check.span, "..", None, &mut applicability);
63+
let snippet_with_braces = {
64+
let need_parens = should_have_parentheses(check);
65+
let (left_paren, right_paren) = if need_parens {("(", ")")} else {("", "")};
66+
format!("{left_paren}{snippet}{right_paren}")
67+
};
68+
69+
let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type
70+
71+
let suggestion = {
72+
let wrap_in_curly = is_else_clause(ctx.tcx, expr);
73+
let (left_curly, right_curly) = if wrap_in_curly {("{", "}")} else {("", "")};
74+
format!(
75+
"{left_curly}{ty}::from({snippet}){right_curly}"
76+
)
77+
}; // when used in else clause if statement should be wrapped in curly braces
78+
79+
span_lint_and_then(ctx,
80+
BOOL_TO_INT_WITH_IF,
81+
expr.span,
82+
"boolean to int conversion using if",
83+
|diag| {
84+
diag.span_suggestion(
85+
expr.span,
86+
"replace with from",
87+
suggestion,
88+
applicability,
89+
);
90+
diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options"));
91+
});
92+
};
93+
}
94+
95+
// If block contains only a int literal expression, return literal expression
96+
fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hir::Expr<'tcx>> {
97+
if let ExprKind::Block(block, _) = expr.kind
98+
&& let Block {
99+
stmts: [], // Shouldn't lint if statements with side effects
100+
expr: Some(expr),
101+
..
102+
} = block
103+
&& let ExprKind::Lit(lit) = &expr.kind
104+
&& let LitKind::Int(_, _) = lit.node
105+
{
106+
Some(expr)
107+
} else {
108+
None
109+
}
110+
}
111+
112+
fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expected_value: u128) -> bool {
113+
if let ExprKind::Lit(lit) = &expr.kind
114+
&& let LitKind::Int(val, _) = lit.node
115+
&& val == expected_value
116+
{
117+
true
118+
} else {
119+
false
120+
}
121+
}
122+
123+
fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool {
124+
check.precedence().order() < ExprPrecedence::Cast.order()
125+
}

clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
1717
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
1818
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
1919
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
20+
LintId::of(bool_to_int_with_if::BOOL_TO_INT_WITH_IF),
2021
LintId::of(booleans::NONMINIMAL_BOOL),
2122
LintId::of(booleans::OVERLY_COMPLEX_BOOL_EXPR),
2223
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ store.register_lints(&[
5656
await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
5757
blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS,
5858
bool_assert_comparison::BOOL_ASSERT_COMPARISON,
59+
bool_to_int_with_if::BOOL_TO_INT_WITH_IF,
5960
booleans::NONMINIMAL_BOOL,
6061
booleans::OVERLY_COMPLEX_BOOL_EXPR,
6162
borrow_deref_ref::BORROW_DEREF_REF,

clippy_lints/src/lib.register_style.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
66
LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
77
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
88
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
9+
LintId::of(bool_to_int_with_if::BOOL_TO_INT_WITH_IF),
910
LintId::of(casts::FN_TO_NUMERIC_CAST),
1011
LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
1112
LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ mod attrs;
178178
mod await_holding_invalid;
179179
mod blocks_in_if_conditions;
180180
mod bool_assert_comparison;
181+
mod bool_to_int_with_if;
181182
mod booleans;
182183
mod borrow_deref_ref;
183184
mod cargo;
@@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
900901
store.register_late_pass(|| Box::new(manual_string_new::ManualStringNew));
901902
store.register_late_pass(|| Box::new(unused_peekable::UnusedPeekable));
902903
store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments));
904+
store.register_late_pass(|| Box::new(bool_to_int_with_if::BoolToIntWithIf));
903905
// add lints here, do not remove this comment, it's used in `new_lint`
904906
}
905907

clippy_lints/src/matches/single_match.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,8 @@ fn form_exhaustive_matches<'a>(cx: &LateContext<'a>, ty: Ty<'a>, left: &Pat<'_>,
201201
// in the arms, so we need to evaluate the correct offsets here in order to iterate in
202202
// both arms at the same time.
203203
let len = max(
204-
left_in.len() + {
205-
if left_pos.is_some() { 1 } else { 0 }
206-
},
207-
right_in.len() + {
208-
if right_pos.is_some() { 1 } else { 0 }
209-
},
204+
left_in.len() + usize::from(left_pos.is_some()),
205+
right_in.len() + usize::from(right_pos.is_some()),
210206
);
211207
let mut left_pos = left_pos.unwrap_or(usize::MAX);
212208
let mut right_pos = right_pos.unwrap_or(usize::MAX);

clippy_lints/src/only_used_in_recursion.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
234234
})) => (
235235
def_id.to_def_id(),
236236
FnKind::TraitFn,
237-
if sig.decl.implicit_self.has_implicit_self() {
238-
1
239-
} else {
240-
0
241-
},
237+
usize::from(sig.decl.implicit_self.has_implicit_self()),
242238
),
243239
Some(Node::ImplItem(&ImplItem {
244240
kind: ImplItemKind::Fn(ref sig, _),
@@ -253,11 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
253249
(
254250
trait_item_id,
255251
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize),
256-
if sig.decl.implicit_self.has_implicit_self() {
257-
1
258-
} else {
259-
0
260-
},
252+
usize::from(sig.decl.implicit_self.has_implicit_self()),
261253
)
262254
} else {
263255
(def_id.to_def_id(), FnKind::Fn, 0)

tests/ui/author/struct.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
#[allow(clippy::unnecessary_operation, clippy::single_match)]
1+
#![allow(
2+
clippy::unnecessary_operation,
3+
clippy::single_match,
4+
clippy::no_effect,
5+
clippy::bool_to_int_with_if
6+
)]
27
fn main() {
38
struct Test {
49
field: u32,

tests/ui/bool_to_int_with_if.fixed

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::bool_to_int_with_if)]
4+
#![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)]
5+
6+
fn main() {
7+
let a = true;
8+
let b = false;
9+
10+
let x = 1;
11+
let y = 2;
12+
13+
// Should lint
14+
// precedence
15+
i32::from(a);
16+
i32::from(!a);
17+
i32::from(a || b);
18+
i32::from(cond(a, b));
19+
i32::from(x + y < 4);
20+
21+
// if else if
22+
if a {
23+
123
24+
} else {i32::from(b)};
25+
26+
// Shouldn't lint
27+
28+
if a {
29+
1
30+
} else if b {
31+
0
32+
} else {
33+
3
34+
};
35+
36+
if a {
37+
3
38+
} else if b {
39+
1
40+
} else {
41+
-2
42+
};
43+
44+
if a {
45+
3
46+
} else {
47+
0
48+
};
49+
if a {
50+
side_effect();
51+
1
52+
} else {
53+
0
54+
};
55+
if a {
56+
1
57+
} else {
58+
side_effect();
59+
0
60+
};
61+
62+
// multiple else ifs
63+
if a {
64+
123
65+
} else if b {
66+
1
67+
} else if a | b {
68+
0
69+
} else {
70+
123
71+
};
72+
73+
some_fn(a);
74+
}
75+
76+
// Lint returns and type inference
77+
fn some_fn(a: bool) -> u8 {
78+
u8::from(a)
79+
}
80+
81+
fn side_effect() {}
82+
83+
fn cond(a: bool, b: bool) -> bool {
84+
a || b
85+
}

0 commit comments

Comments
 (0)