From 7444dcad9aafac5c65d30b0d6751ce1b0f4db058 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 16:57:15 +0300 Subject: [PATCH 01/35] initialize new lint --- CHANGELOG.md | 1 + clippy_lints/src/bool_to_int_with_if.rs | 25 +++++++++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 ++ tests/ui/bool_to_int_with_if.rs | 5 +++++ 7 files changed, 36 insertions(+) create mode 100644 clippy_lints/src/bool_to_int_with_if.rs create mode 100644 tests/ui/bool_to_int_with_if.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bc93c1cb42c..3b778f61654b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs new file mode 100644 index 000000000000..4f8a1f44f933 --- /dev/null +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -0,0 +1,25 @@ +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.64.0"] + pub BOOL_TO_INT_WITH_IF, + style, + "default lint description" +} +declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]); + +impl LateLintPass<'_> for BoolToIntWithIf {} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 563ad891603a..3f428c5e22b7 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -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), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index d3c75f8b5191..3845ba2d0a80 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -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, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 15a1bc569af2..248a6581680f 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -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), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 172fdf8c8526..fbb90414a238 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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` } diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs new file mode 100644 index 000000000000..d27c5a9f09d8 --- /dev/null +++ b/tests/ui/bool_to_int_with_if.rs @@ -0,0 +1,5 @@ +#![warn(clippy::bool_to_int_with_if)] + +fn main() { + // test code goes here +} From 243e671b7297639833093417fc0f9769d96ff30d Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 20:02:52 +0300 Subject: [PATCH 02/35] it detects errors --- clippy_lints/src/bool_to_int_with_if.rs | 79 +++++++++++++++++++++++-- tests/ui/bool_to_int_with_if.rs | 22 ++++++- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 4f8a1f44f933..3474b1ca5bd7 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -1,19 +1,30 @@ +use rustc_ast::LitKind; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// + /// Instead of using an if statement to convert a bool to an int, this lint suggests using a `as` coercion or a `from()` function. /// ### Why is this bad? + /// Coersion 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 - /// // example code where clippy issues a warning + /// if condition { + /// 1_i64 + /// } else { + /// 0 + /// } /// ``` /// Use instead: /// ```rust - /// // example code which does not raise clippy warning + /// i64::from(condition) + /// ``` + /// or + /// ```rust + /// condition as i64 /// ``` #[clippy::version = "1.64.0"] pub BOOL_TO_INT_WITH_IF, @@ -22,4 +33,64 @@ declare_clippy_lint! { } declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]); -impl LateLintPass<'_> for BoolToIntWithIf {} +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_chain!( + if let ExprKind::If(check, then, Some(else_)) = &expr.kind; + + if let Some(then_lit) = int_literal(then); + if let Some(else_lit) = int_literal(else_); + + if check_int_literal_equals_val(then_lit, 1); + if check_int_literal_equals_val(else_lit, 0); + + then { + dbg!(expr.span); + } + ); +} + +// 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_chain!( + if let ExprKind::Block(block, _) = expr.kind; + if let Block { + stmts: [], // Shouldn't lint if statements with side effects + expr: Some(expr), + hir_id: _, + rules: _, + span: _, + targeted_by_break: _, + } = block; + if let ExprKind::Lit(lit) = &expr.kind; + + if let LitKind::Int(val, _) = lit.node; + + then { + return Some(expr) + } + ); + + None +} + +fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expected_value: u128) -> bool { + if_chain!( + if let ExprKind::Lit(lit) = &expr.kind; + if let LitKind::Int(val, _) = lit.node; + if val == expected_value; + + then { + return true; + } + ); + + return false; +} diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index d27c5a9f09d8..48ab7252c42d 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,5 +1,25 @@ #![warn(clippy::bool_to_int_with_if)] fn main() { - // test code goes here + // should warn + let cond = true; + let x = if (cond) { 1 } else { 0 }; + + // shouldn't + let x = if (cond) { Some(1) } else { None }; + + let x = if (cond) { + side_effect(); + 1 + } else { + 0 + }; + + let x = if (cond) { 2 } else { 0 }; + + // questionable + // maybe `!cond as usize` is reasonable + let x = if (cond) { 0 } else { 1 }; } + +fn side_effect() {} From a925d91685d815d2a92b655ca08941bfba6c55e8 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 20:06:28 +0300 Subject: [PATCH 03/35] fix unused variable --- clippy_lints/src/bool_to_int_with_if.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 3474b1ca5bd7..71752b7d659a 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -71,7 +71,7 @@ fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hi } = block; if let ExprKind::Lit(lit) = &expr.kind; - if let LitKind::Int(val, _) = lit.node; + if let LitKind::Int(_, _) = lit.node; then { return Some(expr) From 033e8f160c766a3edc1b5a56cc10e2acaf8b22e7 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 20:50:18 +0300 Subject: [PATCH 04/35] fromat and add some tests --- tests/ui/bool_to_int_with_if.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 48ab7252c42d..76eb4da370ce 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,25 +1,29 @@ #![warn(clippy::bool_to_int_with_if)] fn main() { - // should warn let cond = true; - let x = if (cond) { 1 } else { 0 }; + + // should warn + let x = if cond { 1 } else { 0 }; + + let x = if cond ^ true { 1 } else { 0 }; // shouldn't - let x = if (cond) { Some(1) } else { None }; - let x = if (cond) { + let x = if cond { Some(1) } else { None }; + + let x = if cond { side_effect(); 1 } else { 0 }; - let x = if (cond) { 2 } else { 0 }; + let x = if cond { 2 } else { 0 }; // questionable // maybe `!cond as usize` is reasonable - let x = if (cond) { 0 } else { 1 }; + let x = if cond { 0 } else { 1 }; } fn side_effect() {} From d3b0535164e081cec1ed37b1185b10504ba45055 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 20:51:34 +0300 Subject: [PATCH 05/35] Add lint --- clippy_lints/src/bool_to_int_with_if.rs | 31 +++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 71752b7d659a..aadb152ccf4f 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -3,11 +3,14 @@ use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use clippy_utils::{diagnostics::span_lint_and_then, source::snippet_block}; +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 `as` coercion or a `from()` function. /// ### Why is this bad? - /// Coersion or `from()` is idiomatic way to convert bool to a number. + /// 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 @@ -52,7 +55,31 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx if check_int_literal_equals_val(else_lit, 0); then { - dbg!(expr.span); + let lit_type = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + + 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", + format!( + "{}::from({})", + lit_type, + snippet_block(ctx, check.span, "..", None), + ), + Applicability::MachineApplicable, + ); + + diag.span_suggestion( + expr.span, + "replace with coercion", + format!( + "({}) as {}", + snippet_block(ctx, check.span, "..", None), + lit_type, + ), + Applicability::MachineApplicable, + ); + }); } ); } From 5548723694ffaa7bea20035a144b702126c0819d Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 20:58:13 +0300 Subject: [PATCH 06/35] remove ?unnecessary? reference --- clippy_lints/src/bool_to_int_with_if.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index aadb152ccf4f..c11c4f7056cc 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -46,7 +46,7 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { if_chain!( - if let ExprKind::If(check, then, Some(else_)) = &expr.kind; + if let ExprKind::If(check, then, Some(else_)) = expr.kind; if let Some(then_lit) = int_literal(then); if let Some(else_lit) = int_literal(else_); From 63a5799d9c2ce4f169f33f9d67a4ec687ff078f2 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:15:22 +0300 Subject: [PATCH 07/35] add type awareness test --- tests/ui/bool_to_int_with_if.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 76eb4da370ce..789104961415 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -8,6 +8,9 @@ fn main() { let x = if cond ^ true { 1 } else { 0 }; + // it should be type aware + let x: u8 = if cond { 1 } else { 0 }; + // shouldn't let x = if cond { Some(1) } else { None }; From d46ecd3558255d2c3fd94ffb4f69d115e7e50915 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:17:47 +0300 Subject: [PATCH 08/35] another type test --- tests/ui/bool_to_int_with_if.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 789104961415..e6064c066362 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -10,6 +10,7 @@ fn main() { // it should be type aware let x: u8 = if cond { 1 } else { 0 }; + let x = if cond { 1i16 } else { 0i16 }; // shouldn't From 3b52d8e76d66e865f8c0e1856cf9c0d1bbf97df0 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:32:18 +0300 Subject: [PATCH 09/35] remove coercion suggestion --- clippy_lints/src/bool_to_int_with_if.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index c11c4f7056cc..caf4e34a50e9 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -69,16 +69,16 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx Applicability::MachineApplicable, ); - diag.span_suggestion( - expr.span, - "replace with coercion", - format!( - "({}) as {}", - snippet_block(ctx, check.span, "..", None), - lit_type, - ), - Applicability::MachineApplicable, - ); + // diag.span_suggestion( + // expr.span, + // "replace with coercion", + // format!( + // "({}) as {}", + // snippet_block(ctx, check.span, "..", None), + // lit_type, + // ), + // Applicability::MaybeIncorrect, + // ); }); } ); From 3dec8dcbbd4dedb194d59f9de8a97a206e75e639 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:32:31 +0300 Subject: [PATCH 10/35] add run-rustfix --- tests/ui/bool_to_int_with_if.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index e6064c066362..b70dd3247b03 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::bool_to_int_with_if)] fn main() { From 595e2ea0f08e26a0185bfddcf17ef881b3e36b32 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:32:45 +0300 Subject: [PATCH 11/35] cargo dev bless --- tests/ui/bool_to_int_with_if.fixed | 35 +++++++++++++++++++++++++++++ tests/ui/bool_to_int_with_if.stderr | 28 +++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 tests/ui/bool_to_int_with_if.fixed create mode 100644 tests/ui/bool_to_int_with_if.stderr diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed new file mode 100644 index 000000000000..c7c9aae2a06a --- /dev/null +++ b/tests/ui/bool_to_int_with_if.fixed @@ -0,0 +1,35 @@ +// run-rustfix + +#![warn(clippy::bool_to_int_with_if)] + +fn main() { + let cond = true; + + // should warn + let x = i32::from(cond); + + let x = i32::from(cond ^ true); + + // it should be type aware + let x: u8 = u8::from(cond); + let x = i16::from(cond); + + // shouldn't + + let x = if cond { Some(1) } else { None }; + + let x = if cond { + side_effect(); + 1 + } else { + 0 + }; + + let x = if cond { 2 } else { 0 }; + + // questionable + // maybe `!cond as usize` is reasonable + let x = if cond { 0 } else { 1 }; +} + +fn side_effect() {} diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr new file mode 100644 index 000000000000..3c3866625692 --- /dev/null +++ b/tests/ui/bool_to_int_with_if.stderr @@ -0,0 +1,28 @@ +error: boolean to int conversion using if + --> $DIR/bool_to_int_with_if.rs:9:13 + | +LL | let x = if cond { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond)` + | + = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` + +error: boolean to int conversion using if + --> $DIR/bool_to_int_with_if.rs:11:13 + | +LL | let x = if cond ^ true { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond ^ true)` + +error: boolean to int conversion using if + --> $DIR/bool_to_int_with_if.rs:14:17 + | +LL | let x: u8 = if cond { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(cond)` + +error: boolean to int conversion using if + --> $DIR/bool_to_int_with_if.rs:15:13 + | +LL | let x = if cond { 1i16 } else { 0i16 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i16::from(cond)` + +error: aborting due to 4 previous errors + From fe0172f2156299fa40e0a768c690fae2f085f138 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:44:39 +0300 Subject: [PATCH 12/35] fix tests --- tests/ui/bool_to_int_with_if.fixed | 18 +++++++++--------- tests/ui/bool_to_int_with_if.rs | 18 +++++++++--------- tests/ui/bool_to_int_with_if.stderr | 24 ++++++++++++------------ 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index c7c9aae2a06a..9d94c2660950 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -6,30 +6,30 @@ fn main() { let cond = true; // should warn - let x = i32::from(cond); + let _x = i32::from(cond); - let x = i32::from(cond ^ true); + let _x = i32::from(cond ^ true); // it should be type aware - let x: u8 = u8::from(cond); - let x = i16::from(cond); + let _x: u8 = u8::from(cond); - // shouldn't + let _x = i16::from(cond); - let x = if cond { Some(1) } else { None }; + // shouldn't + let _x = if cond { Some(1) } else { None }; - let x = if cond { + let _x = if cond { side_effect(); 1 } else { 0 }; - let x = if cond { 2 } else { 0 }; + let _x = if cond { 2 } else { 0 }; // questionable // maybe `!cond as usize` is reasonable - let x = if cond { 0 } else { 1 }; + let _x = if cond { 0 } else { 1 }; } fn side_effect() {} diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index b70dd3247b03..c36598e3aa4c 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -6,30 +6,30 @@ fn main() { let cond = true; // should warn - let x = if cond { 1 } else { 0 }; + let _x = if cond { 1 } else { 0 }; - let x = if cond ^ true { 1 } else { 0 }; + let _x = if cond ^ true { 1 } else { 0 }; // it should be type aware - let x: u8 = if cond { 1 } else { 0 }; - let x = if cond { 1i16 } else { 0i16 }; + let _x: u8 = if cond { 1 } else { 0 }; - // shouldn't + let _x = if cond { 1i16 } else { 0i16 }; - let x = if cond { Some(1) } else { None }; + // shouldn't + let _x = if cond { Some(1) } else { None }; - let x = if cond { + let _x = if cond { side_effect(); 1 } else { 0 }; - let x = if cond { 2 } else { 0 }; + let _x = if cond { 2 } else { 0 }; // questionable // maybe `!cond as usize` is reasonable - let x = if cond { 0 } else { 1 }; + let _x = if cond { 0 } else { 1 }; } fn side_effect() {} diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index 3c3866625692..e39b3370e07e 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -1,28 +1,28 @@ error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:9:13 + --> $DIR/bool_to_int_with_if.rs:9:14 | -LL | let x = if cond { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond)` +LL | let _x = if cond { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond)` | = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:11:13 + --> $DIR/bool_to_int_with_if.rs:11:14 | -LL | let x = if cond ^ true { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond ^ true)` +LL | let _x = if cond ^ true { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond ^ true)` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:14:17 + --> $DIR/bool_to_int_with_if.rs:14:18 | -LL | let x: u8 = if cond { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(cond)` +LL | let _x: u8 = if cond { 1 } else { 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(cond)` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:15:13 + --> $DIR/bool_to_int_with_if.rs:16:14 | -LL | let x = if cond { 1i16 } else { 0i16 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i16::from(cond)` +LL | let _x = if cond { 1i16 } else { 0i16 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i16::from(cond)` error: aborting due to 4 previous errors From cc045ab3e66b8ca3355fdb23e8430782bef4b93e Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Thu, 30 Jun 2022 21:50:37 +0300 Subject: [PATCH 13/35] fix style --- clippy_lints/src/bool_to_int_with_if.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index caf4e34a50e9..d5fc5d70e934 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -1,5 +1,5 @@ use rustc_ast::LitKind; -use rustc_hir::*; +use rustc_hir::{Block, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -119,5 +119,5 @@ fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expecte } ); - return false; + false } From 0c2e07c56b69c307b62e81732ba29c17385f50d3 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 14:35:05 +0300 Subject: [PATCH 14/35] implement as version --- clippy_lints/src/bool_to_int_with_if.rs | 27 +++++----- tests/ui/bool_to_int_with_if.fixed | 51 +++++++++++-------- tests/ui/bool_to_int_with_if.rs | 65 ++++++++++++++++++------- tests/ui/bool_to_int_with_if.stderr | 58 +++++++++++++++++----- 4 files changed, 134 insertions(+), 67 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index d5fc5d70e934..0c3c4937d1c1 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -1,4 +1,4 @@ -use rustc_ast::LitKind; +use rustc_ast::{ExprPrecedence, LitKind}; use rustc_hir::{Block, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -57,28 +57,21 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx then { let lit_type = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + let need_parens = should_have_parentheses(check); + 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", format!( - "{}::from({})", - lit_type, - snippet_block(ctx, check.span, "..", None), + "{lbrace}{snippet}{rbrace} as {ty}", + lbrace=if need_parens {"("} else {""}, + rbrace=if need_parens {")"} else {""}, + ty=lit_type, + snippet=snippet_block(ctx, check.span, "..", None), ), Applicability::MachineApplicable, ); - - // diag.span_suggestion( - // expr.span, - // "replace with coercion", - // format!( - // "({}) as {}", - // snippet_block(ctx, check.span, "..", None), - // lit_type, - // ), - // Applicability::MaybeIncorrect, - // ); }); } ); @@ -121,3 +114,7 @@ fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expecte false } + +fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool { + check.precedence().order() < ExprPrecedence::Cast.order() +} diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index 9d94c2660950..541d45be530d 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -1,35 +1,44 @@ // run-rustfix #![warn(clippy::bool_to_int_with_if)] +#[allow(unused)] fn main() { - let cond = true; - - // should warn - let _x = i32::from(cond); - - let _x = i32::from(cond ^ true); - - // it should be type aware - let _x: u8 = u8::from(cond); - - let _x = i16::from(cond); - - // shouldn't - let _x = if cond { Some(1) } else { None }; - - let _x = if cond { + let a = true; + let b = false; + + let x = 1; + let y = 2; + + /// Should lint + // precedence + a as i32; + !a as i32; + (a || b) as i32; + cond(a, b) as i32; + (x + y < 4) as i32; + + /// Shouldn't lint + if a { + 3 + } else { + 0 + }; + if a { side_effect(); 1 } else { 0 }; +} - let _x = if cond { 2 } else { 0 }; - - // questionable - // maybe `!cond as usize` is reasonable - let _x = if cond { 0 } else { 1 }; +// Lint returns and type inference +fn some_fn(a: bool) -> u8 { + a as u8 } fn side_effect() {} + +fn cond(a: bool, b: bool) -> bool { + a || b +} diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index c36598e3aa4c..7abc3b59c2fa 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,35 +1,64 @@ // run-rustfix #![warn(clippy::bool_to_int_with_if)] +#[allow(unused)] fn main() { - let cond = true; + let a = true; + let b = false; - // should warn - let _x = if cond { 1 } else { 0 }; + let x = 1; + let y = 2; - let _x = if cond ^ true { 1 } else { 0 }; - - // it should be type aware - let _x: u8 = if cond { 1 } else { 0 }; - - let _x = if cond { 1i16 } else { 0i16 }; - - // shouldn't - let _x = if cond { Some(1) } else { None }; + /// 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 + }; - let _x = if cond { + /// Shouldn't lint + if a { + 3 + } else { + 0 + }; + if a { side_effect(); 1 } else { 0 }; +} - let _x = if cond { 2 } else { 0 }; - - // questionable - // maybe `!cond as usize` is reasonable - let _x = if cond { 0 } else { 1 }; +// 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 +} diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index e39b3370e07e..53c9ac767451 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -1,28 +1,60 @@ error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:9:14 + --> $DIR/bool_to_int_with_if.rs:15:5 | -LL | let _x = if cond { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond)` +LL | / if a { +LL | | 1 +LL | | } else { +LL | | 0 +LL | | }; + | |_____^ help: replace with from: `a as i32` | = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:11:14 + --> $DIR/bool_to_int_with_if.rs:20:5 | -LL | let _x = if cond ^ true { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i32::from(cond ^ true)` +LL | / if !a { +LL | | 1 +LL | | } else { +LL | | 0 +LL | | }; + | |_____^ help: replace with from: `!a as i32` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:14:18 + --> $DIR/bool_to_int_with_if.rs:25:5 | -LL | let _x: u8 = if cond { 1 } else { 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(cond)` +LL | / if a || b { +LL | | 1 +LL | | } else { +LL | | 0 +LL | | }; + | |_____^ help: replace with from: `(a || b) as i32` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:16:14 + --> $DIR/bool_to_int_with_if.rs:30:5 | -LL | let _x = if cond { 1i16 } else { 0i16 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `i16::from(cond)` +LL | / if cond(a, b) { +LL | | 1 +LL | | } else { +LL | | 0 +LL | | }; + | |_____^ help: replace with from: `cond(a, b) as i32` -error: aborting due to 4 previous errors +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: `(x + y < 4) as i32` + +error: boolean to int conversion using if + --> $DIR/bool_to_int_with_if.rs:57:5 + | +LL | if a { 1 } else { 0 } + | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `a as u8` + +error: aborting due to 6 previous errors From 74a65cda6f2a8c27667e191e29903f755eced79b Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 14:44:01 +0300 Subject: [PATCH 15/35] Add note --- clippy_lints/src/bool_to_int_with_if.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 0c3c4937d1c1..2d3f06bc182a 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -55,23 +55,22 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx if check_int_literal_equals_val(else_lit, 0); then { - let lit_type = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type let need_parens = should_have_parentheses(check); + let snippet = format!("{lbrace}{snippet}{rbrace}", snippet=snippet_block(ctx, check.span, "..", None), lbrace=if need_parens {"("} else {""}, rbrace=if need_parens {")"} else {""}); 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", format!( - "{lbrace}{snippet}{rbrace} as {ty}", - lbrace=if need_parens {"("} else {""}, - rbrace=if need_parens {")"} else {""}, - ty=lit_type, - snippet=snippet_block(ctx, check.span, "..", None), + "{snippet} as {ty}", ), Applicability::MachineApplicable, ); + + diag.note(format!("`{ty}::from({snippet})` or `{snippet}.into()` can also be valid options")); }); } ); From e905ec26299b44a650105d87f2991b215ec2f13a Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 14:45:32 +0300 Subject: [PATCH 16/35] cargo dev bless --- tests/ui/bool_to_int_with_if.stderr | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index 53c9ac767451..859da4dde0e3 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -9,6 +9,7 @@ LL | | }; | |_____^ help: replace with from: `a as i32` | = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` + = note: `i32::from(a)` or `a.into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:20:5 @@ -19,6 +20,8 @@ LL | | } else { LL | | 0 LL | | }; | |_____^ help: replace with from: `!a as i32` + | + = note: `i32::from(!a)` or `!a.into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:25:5 @@ -29,6 +32,8 @@ LL | | } else { LL | | 0 LL | | }; | |_____^ help: replace with from: `(a || b) as i32` + | + = note: `i32::from((a || b))` 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 @@ -39,6 +44,8 @@ LL | | } else { LL | | 0 LL | | }; | |_____^ help: replace with from: `cond(a, b) as i32` + | + = note: `i32::from(cond(a, b))` 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 @@ -49,12 +56,16 @@ LL | | } else { LL | | 0 LL | | }; | |_____^ help: replace with from: `(x + y < 4) as i32` + | + = note: `i32::from((x + y < 4))` or `(x + y < 4).into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:57:5 | LL | if a { 1 } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `a as u8` + | + = note: `u8::from(a)` or `a.into()` can also be valid options error: aborting due to 6 previous errors From ecf9a76d213b7ac3b90557eaca753be04debb216 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 18:33:17 +0300 Subject: [PATCH 17/35] fix tests --- tests/ui/author/struct.rs | 6 ++---- tests/ui/author/struct.stdout | 15 +++------------ tests/ui/bool_to_int_with_if.fixed | 8 +++++--- tests/ui/bool_to_int_with_if.rs | 8 +++++--- tests/ui/bool_to_int_with_if.stderr | 2 +- 5 files changed, 16 insertions(+), 23 deletions(-) diff --git a/tests/ui/author/struct.rs b/tests/ui/author/struct.rs index 5fdf3433a370..e4ac5a070ef3 100644 --- a/tests/ui/author/struct.rs +++ b/tests/ui/author/struct.rs @@ -1,13 +1,11 @@ -#[allow(clippy::unnecessary_operation, clippy::single_match)] +#![allow(clippy::unnecessary_operation, clippy::single_match, clippy::no_effect)] fn main() { struct Test { field: u32, } #[clippy::author] - Test { - field: if true { 1 } else { 0 }, - }; + Test { field: true as u32 }; let test = Test { field: 1 }; diff --git a/tests/ui/author/struct.stdout b/tests/ui/author/struct.stdout index 5e78b7c9de7e..8c9ec90a74ea 100644 --- a/tests/ui/author/struct.stdout +++ b/tests/ui/author/struct.stdout @@ -3,20 +3,11 @@ if_chain! { if match_qpath(qpath, &["Test"]); if fields.len() == 1; if fields[0].ident.as_str() == "field"; - if let ExprKind::If(cond, then, Some(else_expr)) = fields[0].expr.kind; - if let ExprKind::DropTemps(expr1) = cond.kind; + if let ExprKind::Cast(expr1, cast_ty) = fields[0].expr.kind; + if let TyKind::Path(ref qpath1) = cast_ty.kind; + if match_qpath(qpath1, &["u32"]); if let ExprKind::Lit(ref lit) = expr1.kind; if let LitKind::Bool(true) = lit.node; - if let ExprKind::Block(block, None) = then.kind; - if block.stmts.is_empty(); - if let Some(trailing_expr) = block.expr; - if let ExprKind::Lit(ref lit1) = trailing_expr.kind; - if let LitKind::Int(1, LitIntType::Unsuffixed) = lit1.node; - if let ExprKind::Block(block1, None) = else_expr.kind; - if block1.stmts.is_empty(); - if let Some(trailing_expr1) = block1.expr; - if let ExprKind::Lit(ref lit2) = trailing_expr1.kind; - if let LitKind::Int(0, LitIntType::Unsuffixed) = lit2.node; then { // report your lint here } diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index 541d45be530d..1bffcd9206b4 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::bool_to_int_with_if)] -#[allow(unused)] +#![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)] fn main() { let a = true; @@ -10,7 +10,7 @@ fn main() { let x = 1; let y = 2; - /// Should lint + // Should lint // precedence a as i32; !a as i32; @@ -18,7 +18,7 @@ fn main() { cond(a, b) as i32; (x + y < 4) as i32; - /// Shouldn't lint + // Shouldn't lint if a { 3 } else { @@ -30,6 +30,8 @@ fn main() { } else { 0 }; + + some_fn(a); } // Lint returns and type inference diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 7abc3b59c2fa..3576c485320d 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::bool_to_int_with_if)] -#[allow(unused)] +#![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)] fn main() { let a = true; @@ -10,7 +10,7 @@ fn main() { let x = 1; let y = 2; - /// Should lint + // Should lint // precedence if a { 1 @@ -38,7 +38,7 @@ fn main() { 0 }; - /// Shouldn't lint + // Shouldn't lint if a { 3 } else { @@ -50,6 +50,8 @@ fn main() { } else { 0 }; + + some_fn(a); } // Lint returns and type inference diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index 859da4dde0e3..c863562f29d1 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -60,7 +60,7 @@ LL | | }; = note: `i32::from((x + y < 4))` or `(x + y < 4).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:57:5 + --> $DIR/bool_to_int_with_if.rs:59:5 | LL | if a { 1 } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `a as u8` From ff3cda1e26799d39dffa7852d58b471549db2299 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 20:01:20 +0300 Subject: [PATCH 18/35] rollback to suggesting from --- clippy_lints/src/bool_to_int_with_if.rs | 7 ++++--- tests/ui/bool_to_int_with_if.fixed | 12 ++++++------ tests/ui/bool_to_int_with_if.stderr | 24 ++++++++++++------------ 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 2d3f06bc182a..d8e730df50cb 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -58,19 +58,20 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type let need_parens = should_have_parentheses(check); - let snippet = format!("{lbrace}{snippet}{rbrace}", snippet=snippet_block(ctx, check.span, "..", None), lbrace=if need_parens {"("} else {""}, rbrace=if need_parens {")"} else {""}); + let snippet = snippet_block(ctx, check.span, "..", None); + let snippet_with_braces = format!("{lbrace}{snippet}{rbrace}", lbrace=if need_parens {"("} else {""}, rbrace=if need_parens {")"} else {""}); 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", format!( - "{snippet} as {ty}", + "{ty}::from({snippet})" ), Applicability::MachineApplicable, ); - diag.note(format!("`{ty}::from({snippet})` or `{snippet}.into()` can also be valid options")); + diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options")); }); } ); diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index 1bffcd9206b4..3c6be843292b 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -12,11 +12,11 @@ fn main() { // Should lint // precedence - a as i32; - !a as i32; - (a || b) as i32; - cond(a, b) as i32; - (x + y < 4) as i32; + i32::from(a); + i32::from(!a); + i32::from(a || b); + i32::from(cond(a, b)); + i32::from(x + y < 4); // Shouldn't lint if a { @@ -36,7 +36,7 @@ fn main() { // Lint returns and type inference fn some_fn(a: bool) -> u8 { - a as u8 + u8::from(a) } fn side_effect() {} diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index c863562f29d1..d25df158fcc9 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -6,10 +6,10 @@ LL | | 1 LL | | } else { LL | | 0 LL | | }; - | |_____^ help: replace with from: `a as i32` + | |_____^ help: replace with from: `i32::from(a)` | = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` - = note: `i32::from(a)` or `a.into()` can also be valid options + = note: `a` or `a.into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:20:5 @@ -19,9 +19,9 @@ LL | | 1 LL | | } else { LL | | 0 LL | | }; - | |_____^ help: replace with from: `!a as i32` + | |_____^ help: replace with from: `i32::from(!a)` | - = note: `i32::from(!a)` or `!a.into()` can also be valid options + = note: `!a` or `!a.into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:25:5 @@ -31,9 +31,9 @@ LL | | 1 LL | | } else { LL | | 0 LL | | }; - | |_____^ help: replace with from: `(a || b) as i32` + | |_____^ help: replace with from: `i32::from(a || b)` | - = note: `i32::from((a || b))` or `(a || b).into()` can also be valid options + = note: `(a || b)` 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 @@ -43,9 +43,9 @@ LL | | 1 LL | | } else { LL | | 0 LL | | }; - | |_____^ help: replace with from: `cond(a, b) as i32` + | |_____^ help: replace with from: `i32::from(cond(a, b))` | - = note: `i32::from(cond(a, b))` or `cond(a, b).into()` can also be valid options + = note: `cond(a, b)` 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 @@ -55,17 +55,17 @@ LL | | 1 LL | | } else { LL | | 0 LL | | }; - | |_____^ help: replace with from: `(x + y < 4) as i32` + | |_____^ help: replace with from: `i32::from(x + y < 4)` | - = note: `i32::from((x + y < 4))` or `(x + y < 4).into()` can also be valid options + = note: `(x + y < 4)` or `(x + y < 4).into()` can also be valid options error: boolean to int conversion using if --> $DIR/bool_to_int_with_if.rs:59:5 | LL | if a { 1 } else { 0 } - | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `a as u8` + | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(a)` | - = note: `u8::from(a)` or `a.into()` can also be valid options + = note: `a` or `a.into()` can also be valid options error: aborting due to 6 previous errors From d55afc09086dcc85d0dfa6f08c0574366a64cbd0 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 20:01:35 +0300 Subject: [PATCH 19/35] fix dogfood --- clippy_lints/src/matches/single_match.rs | 8 ++------ clippy_lints/src/only_used_in_recursion.rs | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index 92091a0c3395..52632e88b1bd 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -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); diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index 677ac998b568..d2ac1d5b9181 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -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(); From 1d5aa040cdab99e9ad290812d069457dcd1a54a8 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 20:04:31 +0300 Subject: [PATCH 20/35] cargo dev bless --- tests/ui/bool_to_int_with_if.stderr | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index d25df158fcc9..002699371121 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -9,7 +9,7 @@ LL | | }; | |_____^ help: replace with from: `i32::from(a)` | = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` - = note: `a` or `a.into()` can also be valid options + = 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 @@ -21,7 +21,7 @@ LL | | 0 LL | | }; | |_____^ help: replace with from: `i32::from(!a)` | - = note: `!a` or `!a.into()` can also be valid options + = 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 @@ -33,7 +33,7 @@ LL | | 0 LL | | }; | |_____^ help: replace with from: `i32::from(a || b)` | - = note: `(a || b)` or `(a || b).into()` can also be valid options + = 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 @@ -45,7 +45,7 @@ LL | | 0 LL | | }; | |_____^ help: replace with from: `i32::from(cond(a, b))` | - = note: `cond(a, b)` or `cond(a, b).into()` can also be valid options + = 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 @@ -57,7 +57,7 @@ LL | | 0 LL | | }; | |_____^ help: replace with from: `i32::from(x + y < 4)` | - = note: `(x + y < 4)` or `(x + y < 4).into()` can also be valid options + = 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:59:5 @@ -65,7 +65,7 @@ error: boolean to int conversion using if LL | if a { 1 } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(a)` | - = note: `a` or `a.into()` can also be valid options + = note: `a as u8` or `a.into()` can also be valid options error: aborting due to 6 previous errors From 894363e46e10c597409cc62f87607c1d52771061 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 20:28:09 +0300 Subject: [PATCH 21/35] add lint description --- clippy_lints/src/bool_to_int_with_if.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index d8e730df50cb..64f77c4179ea 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -32,7 +32,7 @@ declare_clippy_lint! { #[clippy::version = "1.64.0"] pub BOOL_TO_INT_WITH_IF, style, - "default lint description" + "using if to convert bool to int" } declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]); From 75c93fdaea1b3e716eb74aa43e586d16e44106af Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 20:48:28 +0300 Subject: [PATCH 22/35] fix doctest? --- clippy_lints/src/bool_to_int_with_if.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 64f77c4179ea..bdb6193366ab 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -15,19 +15,22 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// let condition = false; /// if condition { /// 1_i64 /// } else { /// 0 - /// } + /// }; /// ``` /// Use instead: /// ```rust - /// i64::from(condition) + /// let condition = false; + /// i64::from(condition); /// ``` /// or /// ```rust - /// condition as i64 + /// let condition = false; + /// condition as i64; /// ``` #[clippy::version = "1.64.0"] pub BOOL_TO_INT_WITH_IF, From 26468a814936662a14ea1500dea4ba970def0646 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 1 Jul 2022 22:23:50 +0300 Subject: [PATCH 23/35] restore author\struct test --- tests/ui/author/struct.rs | 11 +++++++++-- tests/ui/author/struct.stdout | 15 ++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/ui/author/struct.rs b/tests/ui/author/struct.rs index e4ac5a070ef3..a99bdfc13137 100644 --- a/tests/ui/author/struct.rs +++ b/tests/ui/author/struct.rs @@ -1,11 +1,18 @@ -#![allow(clippy::unnecessary_operation, clippy::single_match, clippy::no_effect)] +#![allow( + clippy::unnecessary_operation, + clippy::single_match, + clippy::no_effect, + clippy::bool_to_int_with_if +)] fn main() { struct Test { field: u32, } #[clippy::author] - Test { field: true as u32 }; + Test { + field: if true { 1 } else { 0 }, + }; let test = Test { field: 1 }; diff --git a/tests/ui/author/struct.stdout b/tests/ui/author/struct.stdout index 8c9ec90a74ea..5e78b7c9de7e 100644 --- a/tests/ui/author/struct.stdout +++ b/tests/ui/author/struct.stdout @@ -3,11 +3,20 @@ if_chain! { if match_qpath(qpath, &["Test"]); if fields.len() == 1; if fields[0].ident.as_str() == "field"; - if let ExprKind::Cast(expr1, cast_ty) = fields[0].expr.kind; - if let TyKind::Path(ref qpath1) = cast_ty.kind; - if match_qpath(qpath1, &["u32"]); + if let ExprKind::If(cond, then, Some(else_expr)) = fields[0].expr.kind; + if let ExprKind::DropTemps(expr1) = cond.kind; if let ExprKind::Lit(ref lit) = expr1.kind; if let LitKind::Bool(true) = lit.node; + if let ExprKind::Block(block, None) = then.kind; + if block.stmts.is_empty(); + if let Some(trailing_expr) = block.expr; + if let ExprKind::Lit(ref lit1) = trailing_expr.kind; + if let LitKind::Int(1, LitIntType::Unsuffixed) = lit1.node; + if let ExprKind::Block(block1, None) = else_expr.kind; + if block1.stmts.is_empty(); + if let Some(trailing_expr1) = block1.expr; + if let ExprKind::Lit(ref lit2) = trailing_expr1.kind; + if let LitKind::Int(0, LitIntType::Unsuffixed) = lit2.node; then { // report your lint here } From c599291596005b96ec404fd9c2af048cde781e52 Mon Sep 17 00:00:00 2001 From: jst-r <58705474+jst-r@users.noreply.github.com> Date: Sun, 3 Jul 2022 23:15:15 +0300 Subject: [PATCH 24/35] Update clippy_lints/src/bool_to_int_with_if.rs Co-authored-by: Fridtjof Stoldt --- clippy_lints/src/bool_to_int_with_if.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index bdb6193366ab..2f3d987c8adb 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -87,10 +87,7 @@ fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hi if let Block { stmts: [], // Shouldn't lint if statements with side effects expr: Some(expr), - hir_id: _, - rules: _, - span: _, - targeted_by_break: _, + .. } = block; if let ExprKind::Lit(lit) = &expr.kind; From e3519e04d92b228c1aa6267601e2f5634318ddca Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Mon, 4 Jul 2022 13:26:18 +0300 Subject: [PATCH 25/35] if_chain with curly braces --- clippy_lints/src/bool_to_int_with_if.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index bdb6193366ab..f24e49e3c64c 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -48,7 +48,7 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { } fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - if_chain!( + if_chain! { if let ExprKind::If(check, then, Some(else_)) = expr.kind; if let Some(then_lit) = int_literal(then); @@ -77,12 +77,12 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx 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_chain!( + if_chain! { if let ExprKind::Block(block, _) = expr.kind; if let Block { stmts: [], // Shouldn't lint if statements with side effects @@ -99,13 +99,13 @@ fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hi then { return Some(expr) } - ); + }; None } fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expected_value: u128) -> bool { - if_chain!( + if_chain! { if let ExprKind::Lit(lit) = &expr.kind; if let LitKind::Int(val, _) = lit.node; if val == expected_value; @@ -113,7 +113,7 @@ fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expecte then { return true; } - ); + }; false } From 70f8a0728932027585450968255b9427f77c6c9b Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Wed, 6 Jul 2022 00:02:47 +0300 Subject: [PATCH 26/35] add tests --- tests/ui/bool_to_int_with_if.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 3576c485320d..7416dbe2caf3 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -38,6 +38,31 @@ fn main() { 0 }; + // if else if + if a { + 123 + } else if b { + 1 + } else { + 0 + }; + + if a { + 1 + } else if b { + 0 + } else { + 3 + }; + + if a { + 1 + } else if b { + 1 + } else { + -2 + }; + // Shouldn't lint if a { 3 @@ -50,6 +75,12 @@ fn main() { } else { 0 }; + if a { + 1 + } else { + side_effect(); + 0 + }; some_fn(a); } From d857ce3109209ec6c16a9515821675a7e1cd211e Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Mon, 25 Jul 2022 21:19:05 +0300 Subject: [PATCH 27/35] if chains and format --- clippy_lints/src/bool_to_int_with_if.rs | 102 +++++++++++------------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 746b69a92021..3eec8cce56fd 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -48,71 +48,65 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { } fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - if_chain! { - if let ExprKind::If(check, then, Some(else_)) = expr.kind; - - if let Some(then_lit) = int_literal(then); - if let Some(else_lit) = int_literal(else_); - - if check_int_literal_equals_val(then_lit, 1); - if check_int_literal_equals_val(else_lit, 0); - - then { - let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type - - let need_parens = should_have_parentheses(check); - let snippet = snippet_block(ctx, check.span, "..", None); - let snippet_with_braces = format!("{lbrace}{snippet}{rbrace}", lbrace=if need_parens {"("} else {""}, rbrace=if need_parens {")"} else {""}); - - 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", - format!( - "{ty}::from({snippet})" - ), - Applicability::MachineApplicable, - ); - - diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options")); - }); - } + 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 ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + let need_parens = should_have_parentheses(check); + let snippet = snippet_block(ctx, check.span, "..", None); + let snippet_with_braces = { + let lparen = if need_parens {"("} else {""}; + let rparen = if need_parens {")"} else {""}; + format!("{lparen}{snippet}{rparen}") + }; + 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", + format!( + "{ty}::from({snippet})" + ), + Applicability::MachineApplicable, + ); + 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_chain! { - if let ExprKind::Block(block, _) = expr.kind; - if let Block { + if let ExprKind::Block(block, _) = expr.kind && + let Block { stmts: [], // Shouldn't lint if statements with side effects expr: Some(expr), .. - } = block; - if let ExprKind::Lit(lit) = &expr.kind; - - if let LitKind::Int(_, _) = lit.node; - - then { - return Some(expr) - } - }; - - None + } = 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_chain! { - if let ExprKind::Lit(lit) = &expr.kind; - if let LitKind::Int(val, _) = lit.node; - if val == expected_value; - - then { - return true; - } - }; - - false + 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 { From ddc16e21f7fd208bee05d4c5df26c3060ae2bd9a Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Mon, 25 Jul 2022 21:34:42 +0300 Subject: [PATCH 28/35] cleaner code snippets --- clippy_lints/src/bool_to_int_with_if.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 3eec8cce56fd..64e978cf843a 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -15,7 +15,7 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let condition = false; + /// # let condition = false; /// if condition { /// 1_i64 /// } else { @@ -24,12 +24,12 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust - /// let condition = false; + /// # let condition = false; /// i64::from(condition); /// ``` /// or /// ```rust - /// let condition = false; + /// # let condition = false; /// condition as i64; /// ``` #[clippy::version = "1.64.0"] From a673d6c8493fe4bc7e04638f247743db8f7946ef Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Mon, 25 Jul 2022 21:36:02 +0300 Subject: [PATCH 29/35] update description --- clippy_lints/src/bool_to_int_with_if.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 64e978cf843a..22e332bf4d97 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -8,10 +8,13 @@ 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 `as` coercion or a `from()` function. + /// 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 + /// 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 From 85b9c053154a449193b6e3dff9e3fcb35365c5f3 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Mon, 25 Jul 2022 21:44:13 +0300 Subject: [PATCH 30/35] handle possibly missing snippet --- clippy_lints/src/bool_to_int_with_if.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 22e332bf4d97..195f5ca446e1 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -3,7 +3,7 @@ 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, source::snippet_block}; +use clippy_utils::{diagnostics::span_lint_and_then, source::snippet_block_with_applicability}; use rustc_errors::Applicability; declare_clippy_lint! { @@ -53,18 +53,21 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { 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_) && + let Some(else_lit) = int_literal(else_) && check_int_literal_equals_val(then_lit, 1) && check_int_literal_equals_val(else_lit, 0) { - let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type - let need_parens = should_have_parentheses(check); - let snippet = snippet_block(ctx, check.span, "..", None); + 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 lparen = if need_parens {"("} else {""}; let rparen = if need_parens {")"} else {""}; format!("{lparen}{snippet}{rparen}") }; + + let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + span_lint_and_then(ctx, BOOL_TO_INT_WITH_IF, expr.span, @@ -76,7 +79,7 @@ let Some(else_lit) = int_literal(else_) && format!( "{ty}::from({snippet})" ), - Applicability::MachineApplicable, + applicability, ); diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options")); }); From 5cda29332278778fc0029527931d0709fb721f9d Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Sat, 30 Jul 2022 02:00:12 +0300 Subject: [PATCH 31/35] handle else if --- clippy_lints/src/bool_to_int_with_if.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 195f5ca446e1..eab42ea92199 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -3,7 +3,7 @@ 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, source::snippet_block_with_applicability}; +use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, source::snippet_block_with_applicability}; use rustc_errors::Applicability; declare_clippy_lint! { @@ -61,13 +61,20 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx let snippet = snippet_block_with_applicability(ctx, check.span, "..", None, &mut applicability); let snippet_with_braces = { let need_parens = should_have_parentheses(check); - let lparen = if need_parens {"("} else {""}; - let rparen = if need_parens {")"} else {""}; - format!("{lparen}{snippet}{rparen}") + 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, @@ -76,9 +83,7 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx diag.span_suggestion( expr.span, "replace with from", - format!( - "{ty}::from({snippet})" - ), + suggestion, applicability, ); diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options")); From 41929d1a69c718ff92d8623854331ca38cbf8150 Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Sat, 30 Jul 2022 02:00:23 +0300 Subject: [PATCH 32/35] update tests --- tests/ui/bool_to_int_with_if.fixed | 39 +++++++++++++++++++++++++++++ tests/ui/bool_to_int_with_if.rs | 16 ++++++++++-- tests/ui/bool_to_int_with_if.stderr | 17 +++++++++++-- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index 3c6be843292b..9c1098dc4c17 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -18,7 +18,29 @@ fn main() { 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 { @@ -30,6 +52,23 @@ fn main() { } 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); } diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 7416dbe2caf3..0c967dac6e2d 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -47,6 +47,8 @@ fn main() { 0 }; + // Shouldn't lint + if a { 1 } else if b { @@ -56,14 +58,13 @@ fn main() { }; if a { - 1 + 3 } else if b { 1 } else { -2 }; - // Shouldn't lint if a { 3 } else { @@ -82,6 +83,17 @@ fn main() { 0 }; + // multiple else ifs + if a { + 123 + } else if b { + 1 + } else if a | b { + 0 + } else { + 123 + }; + some_fn(a); } diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index 002699371121..8647a9cffbed 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -60,12 +60,25 @@ LL | | }; = 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:59:5 + --> $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 6 previous errors +error: aborting due to 7 previous errors From daa005bbc035e876f87c94df1291ae3b67fcba4d Mon Sep 17 00:00:00 2001 From: jst-r <58705474+jst-r@users.noreply.github.com> Date: Fri, 5 Aug 2022 21:40:36 +0300 Subject: [PATCH 33/35] Update clippy_lints/src/bool_to_int_with_if.rs Co-authored-by: Fridtjof Stoldt --- clippy_lints/src/bool_to_int_with_if.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index eab42ea92199..2dc604eaa861 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -10,6 +10,7 @@ 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. From f139c59ea3d7e08e9de6b462bd296071d9f27710 Mon Sep 17 00:00:00 2001 From: jst-r <58705474+jst-r@users.noreply.github.com> Date: Fri, 5 Aug 2022 21:43:23 +0300 Subject: [PATCH 34/35] Update clippy_lints/src/bool_to_int_with_if.rs Co-authored-by: Fridtjof Stoldt --- clippy_lints/src/bool_to_int_with_if.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 2dc604eaa861..cfa4532668a7 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -36,7 +36,7 @@ declare_clippy_lint! { /// # let condition = false; /// condition as i64; /// ``` - #[clippy::version = "1.64.0"] + #[clippy::version = "1.65.0"] pub BOOL_TO_INT_WITH_IF, style, "using if to convert bool to int" From c1961d37e8262e55c5e399ff522f8a2962e4319b Mon Sep 17 00:00:00 2001 From: Dmitrii Lavrov Date: Fri, 5 Aug 2022 21:53:23 +0300 Subject: [PATCH 35/35] formatting --- clippy_lints/src/bool_to_int_with_if.rs | 53 ++++++++++++------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index eab42ea92199..424d94132bb1 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -51,29 +51,29 @@ impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { } 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) + 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 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 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 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 + 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, @@ -93,15 +93,14 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx // 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 { + 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 + } = block + && let ExprKind::Lit(lit) = &expr.kind + && let LitKind::Int(_, _) = lit.node { Some(expr) } else { @@ -110,9 +109,9 @@ fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hi } 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 + if let ExprKind::Lit(lit) = &expr.kind + && let LitKind::Int(val, _) = lit.node + && val == expected_value { true } else {