Skip to content

Commit bae4699

Browse files
borsbors
committed
Auto merge of #9476 - Xaeroxe:bool-to-int-inverted, r=xFrednet
`bool_to_int_with_if` inverse case patch Enhances `bool_to_int_with_if` such that it can also catch an inverse bool int conversion scenario, and makes the right suggestion for converting to int with a prefixed negation operator. changelog: [`bool_to_int_with_if`]: Now correctly detects the inverse case, `if bool { 0 } else { 1 }`
2 parents 2ddbc86 + dd97c1e commit bae4699

8 files changed

+88
-35
lines changed

‎clippy_lints/src/bool_to_int_with_if.rs

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use rustc_ast::{ExprPrecedence, LitKind};
1+
use rustc_ast::LitKind;
22
use rustc_hir::{Block, ExprKind};
33
use rustc_lint::{LateContext, LateLintPass};
44
use rustc_session::{declare_lint_pass, declare_tool_lint};
55

6-
use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, source::snippet_block_with_applicability};
6+
use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, sugg::Sugg};
77
use rustc_errors::Applicability;
88

99
declare_clippy_lint! {
@@ -55,27 +55,42 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx
5555
if let ExprKind::If(check, then, Some(else_)) = expr.kind
5656
&& let Some(then_lit) = int_literal(then)
5757
&& let Some(else_lit) = int_literal(else_)
58-
&& check_int_literal_equals_val(then_lit, 1)
59-
&& check_int_literal_equals_val(else_lit, 0)
6058
{
59+
let inverted = if
60+
check_int_literal_equals_val(then_lit, 1)
61+
&& check_int_literal_equals_val(else_lit, 0) {
62+
false
63+
} else if
64+
check_int_literal_equals_val(then_lit, 0)
65+
&& check_int_literal_equals_val(else_lit, 1) {
66+
true
67+
} else {
68+
// Expression isn't boolean, exit
69+
return;
70+
};
6171
let mut applicability = Applicability::MachineApplicable;
62-
let snippet = snippet_block_with_applicability(ctx, check.span, "..", None, &mut applicability);
63-
let snippet_with_braces = {
64-
let need_parens = should_have_parentheses(check);
65-
let (left_paren, right_paren) = if need_parens {("(", ")")} else {("", "")};
66-
format!("{left_paren}{snippet}{right_paren}")
72+
let snippet = {
73+
let mut sugg = Sugg::hir_with_applicability(ctx, check, "..", &mut applicability);
74+
if inverted {
75+
sugg = !sugg;
76+
}
77+
sugg
6778
};
6879

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

7182
let suggestion = {
7283
let wrap_in_curly = is_else_clause(ctx.tcx, expr);
73-
let (left_curly, right_curly) = if wrap_in_curly {("{", "}")} else {("", "")};
74-
format!(
75-
"{left_curly}{ty}::from({snippet}){right_curly}"
76-
)
84+
let mut s = Sugg::NonParen(format!("{ty}::from({snippet})").into());
85+
if wrap_in_curly {
86+
s = s.blockify();
87+
}
88+
s
7789
}; // when used in else clause if statement should be wrapped in curly braces
7890

91+
let into_snippet = snippet.clone().maybe_par();
92+
let as_snippet = snippet.as_ty(ty);
93+
7994
span_lint_and_then(ctx,
8095
BOOL_TO_INT_WITH_IF,
8196
expr.span,
@@ -87,7 +102,7 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx
87102
suggestion,
88103
applicability,
89104
);
90-
diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options"));
105+
diag.note(format!("`{as_snippet}` or `{into_snippet}.into()` can also be valid options"));
91106
});
92107
};
93108
}
@@ -119,7 +134,3 @@ fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expecte
119134
false
120135
}
121136
}
122-
123-
fn should_have_parentheses<'tcx>(check: &'tcx rustc_hir::Expr<'tcx>) -> bool {
124-
check.precedence().order() < ExprPrecedence::Cast.order()
125-
}

‎clippy_lints/src/dereference.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
297297
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
298298
&& position.lint_explicit_deref() =>
299299
{
300+
let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)));
300301
self.state = Some((
301302
State::DerefMethod {
302-
ty_changed_count: if deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)) {
303-
0
304-
} else {
305-
1
306-
},
303+
ty_changed_count,
307304
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
308305
target_mut,
309306
},

‎clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ fn check_other_call_arg<'tcx>(
269269
// We can't add an `&` when the trait is `Deref` because `Target = &T` won't match
270270
// `Target = T`.
271271
if n_refs > 0 || is_copy(cx, receiver_ty) || trait_predicate.def_id() != deref_trait_id;
272-
let n_refs = max(n_refs, if is_copy(cx, receiver_ty) { 0 } else { 1 });
272+
let n_refs = max(n_refs, usize::from(!is_copy(cx, receiver_ty)));
273273
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
274274
then {
275275
span_lint_and_sugg(

‎clippy_utils/src/sugg.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ impl<'a> Sugg<'a> {
155155
| hir::ExprKind::Ret(..)
156156
| hir::ExprKind::Struct(..)
157157
| hir::ExprKind::Tup(..)
158-
| hir::ExprKind::DropTemps(_)
159158
| hir::ExprKind::Err => Sugg::NonParen(get_snippet(expr.span)),
159+
hir::ExprKind::DropTemps(inner) => Self::hir_from_snippet(inner, get_snippet),
160160
hir::ExprKind::Assign(lhs, rhs, _) => {
161161
Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span))
162162
},

‎tests/ui/bool_to_int_with_if.fixed

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@ fn main() {
1414
// precedence
1515
i32::from(a);
1616
i32::from(!a);
17+
i32::from(!a);
1718
i32::from(a || b);
1819
i32::from(cond(a, b));
1920
i32::from(x + y < 4);
2021

2122
// if else if
2223
if a {
2324
123
24-
} else {i32::from(b)};
25+
} else { i32::from(b) };
26+
27+
// if else if inverted
28+
if a {
29+
123
30+
} else { i32::from(!b) };
2531

2632
// Shouldn't lint
2733

‎tests/ui/bool_to_int_with_if.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ fn main() {
1717
} else {
1818
0
1919
};
20+
if a {
21+
0
22+
} else {
23+
1
24+
};
2025
if !a {
2126
1
2227
} else {
@@ -47,6 +52,15 @@ fn main() {
4752
0
4853
};
4954

55+
// if else if inverted
56+
if a {
57+
123
58+
} else if b {
59+
0
60+
} else {
61+
1
62+
};
63+
5064
// Shouldn't lint
5165

5266
if a {

‎tests/ui/bool_to_int_with_if.stderr

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,29 @@ LL | | };
1414
error: boolean to int conversion using if
1515
--> $DIR/bool_to_int_with_if.rs:20:5
1616
|
17+
LL | / if a {
18+
LL | | 0
19+
LL | | } else {
20+
LL | | 1
21+
LL | | };
22+
| |_____^ help: replace with from: `i32::from(!a)`
23+
|
24+
= note: `!a as i32` or `(!a).into()` can also be valid options
25+
26+
error: boolean to int conversion using if
27+
--> $DIR/bool_to_int_with_if.rs:25:5
28+
|
1729
LL | / if !a {
1830
LL | | 1
1931
LL | | } else {
2032
LL | | 0
2133
LL | | };
2234
| |_____^ help: replace with from: `i32::from(!a)`
2335
|
24-
= note: `!a as i32` or `!a.into()` can also be valid options
36+
= note: `!a as i32` or `(!a).into()` can also be valid options
2537

2638
error: boolean to int conversion using if
27-
--> $DIR/bool_to_int_with_if.rs:25:5
39+
--> $DIR/bool_to_int_with_if.rs:30:5
2840
|
2941
LL | / if a || b {
3042
LL | | 1
@@ -36,7 +48,7 @@ LL | | };
3648
= note: `(a || b) as i32` or `(a || b).into()` can also be valid options
3749

3850
error: boolean to int conversion using if
39-
--> $DIR/bool_to_int_with_if.rs:30:5
51+
--> $DIR/bool_to_int_with_if.rs:35:5
4052
|
4153
LL | / if cond(a, b) {
4254
LL | | 1
@@ -48,7 +60,7 @@ LL | | };
4860
= note: `cond(a, b) as i32` or `cond(a, b).into()` can also be valid options
4961

5062
error: boolean to int conversion using if
51-
--> $DIR/bool_to_int_with_if.rs:35:5
63+
--> $DIR/bool_to_int_with_if.rs:40:5
5264
|
5365
LL | / if x + y < 4 {
5466
LL | | 1
@@ -60,25 +72,38 @@ LL | | };
6072
= note: `(x + y < 4) as i32` or `(x + y < 4).into()` can also be valid options
6173

6274
error: boolean to int conversion using if
63-
--> $DIR/bool_to_int_with_if.rs:44:12
75+
--> $DIR/bool_to_int_with_if.rs:49:12
6476
|
6577
LL | } else if b {
6678
| ____________^
6779
LL | | 1
6880
LL | | } else {
6981
LL | | 0
7082
LL | | };
71-
| |_____^ help: replace with from: `{i32::from(b)}`
83+
| |_____^ help: replace with from: `{ i32::from(b) }`
7284
|
7385
= note: `b as i32` or `b.into()` can also be valid options
7486

7587
error: boolean to int conversion using if
76-
--> $DIR/bool_to_int_with_if.rs:102:5
88+
--> $DIR/bool_to_int_with_if.rs:58:12
89+
|
90+
LL | } else if b {
91+
| ____________^
92+
LL | | 0
93+
LL | | } else {
94+
LL | | 1
95+
LL | | };
96+
| |_____^ help: replace with from: `{ i32::from(!b) }`
97+
|
98+
= note: `!b as i32` or `(!b).into()` can also be valid options
99+
100+
error: boolean to int conversion using if
101+
--> $DIR/bool_to_int_with_if.rs:116:5
77102
|
78103
LL | if a { 1 } else { 0 }
79104
| ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(a)`
80105
|
81106
= note: `a as u8` or `a.into()` can also be valid options
82107

83-
error: aborting due to 7 previous errors
108+
error: aborting due to 9 previous errors
84109

‎tests/ui/len_without_is_empty.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ impl AsyncLen {
274274
}
275275

276276
pub async fn len(&self) -> usize {
277-
if self.async_task().await { 0 } else { 1 }
277+
usize::from(!self.async_task().await)
278278
}
279279

280280
pub async fn is_empty(&self) -> bool {

0 commit comments

Comments
 (0)