Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint [bool_to_int_with_if] #9086

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
7444dca
initialize new lint
Jun 30, 2022
243e671
it detects errors
Jun 30, 2022
a925d91
fix unused variable
Jun 30, 2022
033e8f1
fromat and add some tests
Jun 30, 2022
d3b0535
Add lint
Jun 30, 2022
5548723
remove ?unnecessary? reference
Jun 30, 2022
63a5799
add type awareness test
Jun 30, 2022
d46ecd3
another type test
Jun 30, 2022
3b52d8e
remove coercion suggestion
Jun 30, 2022
3dec8dc
add run-rustfix
Jun 30, 2022
595e2ea
cargo dev bless
Jun 30, 2022
fe0172f
fix tests
Jun 30, 2022
cc045ab
fix style
Jun 30, 2022
0c2e07c
implement as version
Jul 1, 2022
74a65cd
Add note
Jul 1, 2022
e905ec2
cargo dev bless
Jul 1, 2022
ecf9a76
fix tests
Jul 1, 2022
ff3cda1
rollback to suggesting from
Jul 1, 2022
d55afc0
fix dogfood
Jul 1, 2022
1d5aa04
cargo dev bless
Jul 1, 2022
894363e
add lint description
Jul 1, 2022
75c93fd
fix doctest?
Jul 1, 2022
26468a8
restore author\struct test
Jul 1, 2022
c599291
Update clippy_lints/src/bool_to_int_with_if.rs
jst-r Jul 3, 2022
e3519e0
if_chain with curly braces
Jul 4, 2022
70f8a07
add tests
Jul 5, 2022
3bf093d
Merge branch 'master' of https://github.com/jst-r/rust-clippy
Jul 5, 2022
d857ce3
if chains and format
Jul 25, 2022
ddc16e2
cleaner code snippets
Jul 25, 2022
a673d6c
update description
Jul 25, 2022
85b9c05
handle possibly missing snippet
Jul 25, 2022
5cda293
handle else if
Jul 29, 2022
41929d1
update tests
Jul 29, 2022
daa005b
Update clippy_lints/src/bool_to_int_with_if.rs
jst-r Aug 5, 2022
f139c59
Update clippy_lints/src/bool_to_int_with_if.rs
jst-r Aug 5, 2022
c1961d3
formatting
Aug 5, 2022
267fc35
Merge branch 'master' of https://github.com/jst-r/rust-clippy
Aug 5, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3297,6 +3297,7 @@ Released 2018-09-13
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`bool_to_int_with_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_to_int_with_if
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
Expand Down
125 changes: 125 additions & 0 deletions clippy_lints/src/bool_to_int_with_if.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use rustc_ast::{ExprPrecedence, LitKind};
use rustc_hir::{Block, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, source::snippet_block_with_applicability};
use rustc_errors::Applicability;

declare_clippy_lint! {
/// ### What it does
/// Instead of using an if statement to convert a bool to an int,
/// this lint suggests using a `from()` function or an `as` coercion.
/// ### Why is this bad?
jst-r marked this conversation as resolved.
Show resolved Hide resolved
/// Coercion or `from()` is idiomatic way to convert bool to a number.
/// Both methods are guaranteed to return 1 for true, and 0 for false.
///
/// See https://doc.rust-lang.org/std/primitive.bool.html#impl-From%3Cbool%3E
///
/// ### Example
/// ```rust
/// # let condition = false;
/// if condition {
/// 1_i64
/// } else {
/// 0
/// };
/// ```
/// Use instead:
/// ```rust
/// # let condition = false;
/// i64::from(condition);
/// ```
/// or
/// ```rust
/// # let condition = false;
/// condition as i64;
/// ```
#[clippy::version = "1.64.0"]
jst-r marked this conversation as resolved.
Show resolved Hide resolved
pub BOOL_TO_INT_WITH_IF,
style,
"using if to convert bool to int"
}
declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]);

impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf {
fn check_expr(&mut self, ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
if !expr.span.from_expansion() {
check_if_else(ctx, expr);
}
}
}

fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
jst-r marked this conversation as resolved.
Show resolved Hide resolved
if let ExprKind::If(check, then, Some(else_)) = expr.kind &&
let Some(then_lit) = int_literal(then) &&
let Some(else_lit) = int_literal(else_) &&
check_int_literal_equals_val(then_lit, 1) &&
check_int_literal_equals_val(else_lit, 0)
jst-r marked this conversation as resolved.
Show resolved Hide resolved
{
let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_block_with_applicability(ctx, check.span, "..", None, &mut applicability);
let snippet_with_braces = {
let need_parens = should_have_parentheses(check);
let (left_paren, right_paren) = if need_parens {("(", ")")} else {("", "")};
format!("{left_paren}{snippet}{right_paren}")
};

let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type

let suggestion = {
let wrap_in_curly = is_else_clause(ctx.tcx, expr);
let (left_curly, right_curly) = if wrap_in_curly {("{", "}")} else {("", "")};
format!(
"{left_curly}{ty}::from({snippet}){right_curly}"
)
}; // when used in else clause if statement should be wrapped in curly braces

span_lint_and_then(ctx,
BOOL_TO_INT_WITH_IF,
expr.span,
"boolean to int conversion using if",
|diag| {
diag.span_suggestion(
expr.span,
"replace with from",
suggestion,
applicability,
);
diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options"));
});
};
}

// If block contains only a int literal expression, return literal expression
fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hir::Expr<'tcx>> {
if let ExprKind::Block(block, _) = expr.kind &&
let Block {
stmts: [], // Shouldn't lint if statements with side effects
expr: Some(expr),
..
} = block &&
let ExprKind::Lit(lit) = &expr.kind &&

let LitKind::Int(_, _) = lit.node
jst-r marked this conversation as resolved.
Show resolved Hide resolved
{
Some(expr)
} else {
None
}
}

fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expected_value: u128) -> bool {
if let ExprKind::Lit(lit) = &expr.kind &&
let LitKind::Int(val, _) = lit.node &&
val == expected_value
jst-r marked this conversation as resolved.
Show resolved Hide resolved
{
true
} else {
false
}
}

fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool {
check.precedence().order() < ExprPrecedence::Cast.order()
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(blacklisted_name::BLACKLISTED_NAME),
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
LintId::of(bool_to_int_with_if::BOOL_TO_INT_WITH_IF),
LintId::of(booleans::LOGIC_BUG),
LintId::of(booleans::NONMINIMAL_BOOL),
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ store.register_lints(&[
blacklisted_name::BLACKLISTED_NAME,
blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS,
bool_assert_comparison::BOOL_ASSERT_COMPARISON,
bool_to_int_with_if::BOOL_TO_INT_WITH_IF,
booleans::LOGIC_BUG,
booleans::NONMINIMAL_BOOL,
borrow_as_ptr::BORROW_AS_PTR,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(blacklisted_name::BLACKLISTED_NAME),
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
LintId::of(bool_to_int_with_if::BOOL_TO_INT_WITH_IF),
LintId::of(casts::FN_TO_NUMERIC_CAST),
LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ mod await_holding_invalid;
mod blacklisted_name;
mod blocks_in_if_conditions;
mod bool_assert_comparison;
mod bool_to_int_with_if;
mod booleans;
mod borrow_as_ptr;
mod borrow_deref_ref;
Expand Down Expand Up @@ -913,6 +914,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(manual_retain::ManualRetain::new(msrv)));
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
store.register_late_pass(|| Box::new(bool_to_int_with_if::BoolToIntWithIf));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
8 changes: 2 additions & 6 deletions clippy_lints/src/matches/single_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,8 @@ fn form_exhaustive_matches<'a>(cx: &LateContext<'a>, ty: Ty<'a>, left: &Pat<'_>,
// in the arms, so we need to evaluate the correct offsets here in order to iterate in
// both arms at the same time.
let len = max(
left_in.len() + {
if left_pos.is_some() { 1 } else { 0 }
},
right_in.len() + {
if right_pos.is_some() { 1 } else { 0 }
},
left_in.len() + usize::from(left_pos.is_some()),
right_in.len() + usize::from(right_pos.is_some()),
);
let mut left_pos = left_pos.unwrap_or(usize::MAX);
let mut right_pos = right_pos.unwrap_or(usize::MAX);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
});
v
})
.skip(if has_self { 1 } else { 0 })
.skip(usize::from(has_self))
.filter(|(_, _, ident)| !ident.name.as_str().starts_with('_'))
.collect_vec();

Expand Down
7 changes: 6 additions & 1 deletion tests/ui/author/struct.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
#[allow(clippy::unnecessary_operation, clippy::single_match)]
#![allow(
clippy::unnecessary_operation,
clippy::single_match,
clippy::no_effect,
clippy::bool_to_int_with_if
)]
fn main() {
struct Test {
field: u32,
Expand Down
85 changes: 85 additions & 0 deletions tests/ui/bool_to_int_with_if.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// run-rustfix

#![warn(clippy::bool_to_int_with_if)]
#![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)]

fn main() {
let a = true;
let b = false;

let x = 1;
let y = 2;

// Should lint
// precedence
i32::from(a);
i32::from(!a);
i32::from(a || b);
i32::from(cond(a, b));
i32::from(x + y < 4);

// if else if
if a {
123
} else {i32::from(b)};

// Shouldn't lint

if a {
1
} else if b {
0
} else {
3
};

if a {
3
} else if b {
1
} else {
-2
};

if a {
3
} else {
0
};
if a {
side_effect();
1
} else {
0
};
if a {
1
} else {
side_effect();
0
};

// multiple else ifs
if a {
123
} else if b {
1
} else if a | b {
0
} else {
123
};

some_fn(a);
}

// Lint returns and type inference
fn some_fn(a: bool) -> u8 {
u8::from(a)
}

fn side_effect() {}

fn cond(a: bool, b: bool) -> bool {
a || b
}
Loading