New lint `bool_to_int_with_if`

This commit is contained in:
Dmitrii Lavrov 2022-06-30 16:57:15 +03:00 committed by xFrednet
parent a80e278036
commit b1f86a49ea
No known key found for this signature in database
GPG Key ID: F5C59D0E669E5302
12 changed files with 419 additions and 17 deletions

View File

@ -3603,6 +3603,7 @@ Released 2018-09-13
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions [`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_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_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_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_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 [`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

View File

@ -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?
/// 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.65.0"]
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>) {
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)
{
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
{
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
{
true
} else {
false
}
}
fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool {
check.precedence().order() < ExprPrecedence::Cast.order()
}

View File

@ -17,6 +17,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF), LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
LintId::of(bool_to_int_with_if::BOOL_TO_INT_WITH_IF),
LintId::of(booleans::NONMINIMAL_BOOL), LintId::of(booleans::NONMINIMAL_BOOL),
LintId::of(booleans::OVERLY_COMPLEX_BOOL_EXPR), LintId::of(booleans::OVERLY_COMPLEX_BOOL_EXPR),
LintId::of(borrow_deref_ref::BORROW_DEREF_REF), LintId::of(borrow_deref_ref::BORROW_DEREF_REF),

View File

@ -56,6 +56,7 @@ store.register_lints(&[
await_holding_invalid::AWAIT_HOLDING_REFCELL_REF, await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS, blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS,
bool_assert_comparison::BOOL_ASSERT_COMPARISON, bool_assert_comparison::BOOL_ASSERT_COMPARISON,
bool_to_int_with_if::BOOL_TO_INT_WITH_IF,
booleans::NONMINIMAL_BOOL, booleans::NONMINIMAL_BOOL,
booleans::OVERLY_COMPLEX_BOOL_EXPR, booleans::OVERLY_COMPLEX_BOOL_EXPR,
borrow_deref_ref::BORROW_DEREF_REF, borrow_deref_ref::BORROW_DEREF_REF,

View File

@ -6,6 +6,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS), LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), 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),
LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF), LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),

View File

@ -178,6 +178,7 @@ mod attrs;
mod await_holding_invalid; mod await_holding_invalid;
mod blocks_in_if_conditions; mod blocks_in_if_conditions;
mod bool_assert_comparison; mod bool_assert_comparison;
mod bool_to_int_with_if;
mod booleans; mod booleans;
mod borrow_deref_ref; mod borrow_deref_ref;
mod cargo; mod cargo;
@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(manual_string_new::ManualStringNew)); store.register_late_pass(|| Box::new(manual_string_new::ManualStringNew));
store.register_late_pass(|| Box::new(unused_peekable::UnusedPeekable)); store.register_late_pass(|| Box::new(unused_peekable::UnusedPeekable));
store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments)); store.register_early_pass(|| Box::new(multi_assignments::MultiAssignments));
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` // add lints here, do not remove this comment, it's used in `new_lint`
} }

View File

@ -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 // in the arms, so we need to evaluate the correct offsets here in order to iterate in
// both arms at the same time. // both arms at the same time.
let len = max( let len = max(
left_in.len() + { left_in.len() + usize::from(left_pos.is_some()),
if left_pos.is_some() { 1 } else { 0 } right_in.len() + usize::from(right_pos.is_some()),
},
right_in.len() + {
if right_pos.is_some() { 1 } else { 0 }
},
); );
let mut left_pos = left_pos.unwrap_or(usize::MAX); let mut left_pos = left_pos.unwrap_or(usize::MAX);
let mut right_pos = right_pos.unwrap_or(usize::MAX); let mut right_pos = right_pos.unwrap_or(usize::MAX);

View File

@ -234,11 +234,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
})) => ( })) => (
def_id.to_def_id(), def_id.to_def_id(),
FnKind::TraitFn, FnKind::TraitFn,
if sig.decl.implicit_self.has_implicit_self() { usize::from(sig.decl.implicit_self.has_implicit_self()),
1
} else {
0
},
), ),
Some(Node::ImplItem(&ImplItem { Some(Node::ImplItem(&ImplItem {
kind: ImplItemKind::Fn(ref sig, _), kind: ImplItemKind::Fn(ref sig, _),
@ -253,11 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
( (
trait_item_id, trait_item_id,
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize), FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize),
if sig.decl.implicit_self.has_implicit_self() { usize::from(sig.decl.implicit_self.has_implicit_self()),
1
} else {
0
},
) )
} else { } else {
(def_id.to_def_id(), FnKind::Fn, 0) (def_id.to_def_id(), FnKind::Fn, 0)

View File

@ -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() { fn main() {
struct Test { struct Test {
field: u32, field: u32,

View File

@ -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
}

View File

@ -0,0 +1,109 @@
// 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
if a {
1
} else {
0
};
if !a {
1
} else {
0
};
if a || b {
1
} else {
0
};
if cond(a, b) {
1
} else {
0
};
if x + y < 4 {
1
} else {
0
};
// if else if
if a {
123
} else if b {
1
} else {
0
};
// 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 {
if a { 1 } else { 0 }
}
fn side_effect() {}
fn cond(a: bool, b: bool) -> bool {
a || b
}

View File

@ -0,0 +1,84 @@
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:15:5
|
LL | / if a {
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(a)`
|
= note: `-D clippy::bool-to-int-with-if` implied by `-D warnings`
= note: `a as i32` or `a.into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:20:5
|
LL | / if !a {
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(!a)`
|
= note: `!a as i32` or `!a.into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:25:5
|
LL | / if a || b {
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(a || b)`
|
= note: `(a || b) as i32` or `(a || b).into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:30:5
|
LL | / if cond(a, b) {
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(cond(a, b))`
|
= note: `cond(a, b) as i32` or `cond(a, b).into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:35:5
|
LL | / if x + y < 4 {
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `i32::from(x + y < 4)`
|
= note: `(x + y < 4) as i32` or `(x + y < 4).into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:44:12
|
LL | } else if b {
| ____________^
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `{i32::from(b)}`
|
= note: `b as i32` or `b.into()` can also be valid options
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:102:5
|
LL | if a { 1 } else { 0 }
| ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(a)`
|
= note: `a as u8` or `a.into()` can also be valid options
error: aborting due to 7 previous errors