Skip to content

bool_to_int_with_if inverse case patch #9476

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions clippy_lints/src/bool_to_int_with_if.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use rustc_ast::{ExprPrecedence, LitKind};
use rustc_ast::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 clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, sugg::Sugg};
use rustc_errors::Applicability;

declare_clippy_lint! {
Expand Down Expand Up @@ -55,27 +55,42 @@ 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 inverted = if
check_int_literal_equals_val(then_lit, 1)
&& check_int_literal_equals_val(else_lit, 0) {
false
} else if
check_int_literal_equals_val(then_lit, 0)
&& check_int_literal_equals_val(else_lit, 1) {
true
} else {
// Expression isn't boolean, exit
return;
};
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 snippet = {
let mut sugg = Sugg::hir_with_applicability(ctx, check, "..", &mut applicability);
if inverted {
sugg = !sugg;
}
sugg
};

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}"
)
let mut s = Sugg::NonParen(format!("{ty}::from({snippet})").into());
if wrap_in_curly {
s = s.blockify();
}
s
}; // when used in else clause if statement should be wrapped in curly braces

let into_snippet = snippet.clone().maybe_par();
let as_snippet = snippet.as_ty(ty);

span_lint_and_then(ctx,
BOOL_TO_INT_WITH_IF,
expr.span,
Expand All @@ -87,7 +102,7 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx
suggestion,
applicability,
);
diag.note(format!("`{snippet_with_braces} as {ty}` or `{snippet_with_braces}.into()` can also be valid options"));
diag.note(format!("`{as_snippet}` or `{into_snippet}.into()` can also be valid options"));
});
};
}
Expand Down Expand Up @@ -119,7 +134,3 @@ 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()
}
7 changes: 2 additions & 5 deletions clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
&& position.lint_explicit_deref() =>
{
let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)));
self.state = Some((
State::DerefMethod {
ty_changed_count: if deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)) {
0
} else {
1
},
ty_changed_count,
is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
target_mut,
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/unnecessary_to_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ fn check_other_call_arg<'tcx>(
// We can't add an `&` when the trait is `Deref` because `Target = &T` won't match
// `Target = T`.
if n_refs > 0 || is_copy(cx, receiver_ty) || trait_predicate.def_id() != deref_trait_id;
let n_refs = max(n_refs, if is_copy(cx, receiver_ty) { 0 } else { 1 });
let n_refs = max(n_refs, usize::from(!is_copy(cx, receiver_ty)));
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
then {
span_lint_and_sugg(
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ impl<'a> Sugg<'a> {
| hir::ExprKind::Ret(..)
| hir::ExprKind::Struct(..)
| hir::ExprKind::Tup(..)
| hir::ExprKind::DropTemps(_)
| hir::ExprKind::Err => Sugg::NonParen(get_snippet(expr.span)),
hir::ExprKind::DropTemps(inner) => Self::hir_from_snippet(inner, get_snippet),
hir::ExprKind::Assign(lhs, rhs, _) => {
Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span))
},
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/bool_to_int_with_if.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@ fn main() {
// precedence
i32::from(a);
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)};
} else { i32::from(b) };

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

// Shouldn't lint

Expand Down
14 changes: 14 additions & 0 deletions tests/ui/bool_to_int_with_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ fn main() {
} else {
0
};
if a {
0
} else {
1
};
if !a {
1
} else {
Expand Down Expand Up @@ -47,6 +52,15 @@ fn main() {
0
};

// if else if inverted
if a {
123
} else if b {
0
} else {
1
};

// Shouldn't lint

if a {
Expand Down
41 changes: 33 additions & 8 deletions tests/ui/bool_to_int_with_if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,29 @@ LL | | };
error: boolean to int conversion using if
--> $DIR/bool_to_int_with_if.rs:20:5
|
LL | / if a {
LL | | 0
LL | | } else {
LL | | 1
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 {
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
= 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
--> $DIR/bool_to_int_with_if.rs:30:5
|
LL | / if a || b {
LL | | 1
Expand All @@ -36,7 +48,7 @@ LL | | };
= 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
--> $DIR/bool_to_int_with_if.rs:35:5
|
LL | / if cond(a, b) {
LL | | 1
Expand All @@ -48,7 +60,7 @@ LL | | };
= 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
--> $DIR/bool_to_int_with_if.rs:40:5
|
LL | / if x + y < 4 {
LL | | 1
Expand All @@ -60,25 +72,38 @@ 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:44:12
--> $DIR/bool_to_int_with_if.rs:49:12
|
LL | } else if b {
| ____________^
LL | | 1
LL | | } else {
LL | | 0
LL | | };
| |_____^ help: replace with from: `{i32::from(b)}`
| |_____^ 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
--> $DIR/bool_to_int_with_if.rs:58:12
|
LL | } else if b {
| ____________^
LL | | 0
LL | | } else {
LL | | 1
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:116: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
error: aborting due to 9 previous errors

2 changes: 1 addition & 1 deletion tests/ui/len_without_is_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ impl AsyncLen {
}

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

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