about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-09-14 19:56:12 +0000
committerbors <bors@rust-lang.org>2022-09-14 19:56:12 +0000
commitbae4699a9f6afc1c3c2e295e2ffa7d309f96f84d (patch)
treec3434ddecda069bc3abcb7c2efd48678df97eb32
parent2ddbc86bef837b1072159c020c35940ce52ae696 (diff)
parentdd97c1ed205f47d13dc5bc482a73c0c76383d159 (diff)
downloadrust-bae4699a9f6afc1c3c2e295e2ffa7d309f96f84d.tar.gz
rust-bae4699a9f6afc1c3c2e295e2ffa7d309f96f84d.zip
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 }`
-rw-r--r--clippy_lints/src/bool_to_int_with_if.rs47
-rw-r--r--clippy_lints/src/dereference.rs7
-rw-r--r--clippy_lints/src/methods/unnecessary_to_owned.rs2
-rw-r--r--clippy_utils/src/sugg.rs2
-rw-r--r--tests/ui/bool_to_int_with_if.fixed8
-rw-r--r--tests/ui/bool_to_int_with_if.rs14
-rw-r--r--tests/ui/bool_to_int_with_if.stderr41
-rw-r--r--tests/ui/len_without_is_empty.rs2
8 files changed, 88 insertions, 35 deletions
diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs
index a4b8cbb0d82..51e98cda845 100644
--- a/clippy_lints/src/bool_to_int_with_if.rs
+++ b/clippy_lints/src/bool_to_int_with_if.rs
@@ -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! {
@@ -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,
@@ -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"));
         });
     };
 }
@@ -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()
-}
diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs
index 88e28018e5d..d1b25a0f1b5 100644
--- a/clippy_lints/src/dereference.rs
+++ b/clippy_lints/src/dereference.rs
@@ -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,
                             },
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
index 763bfafecef..b32e5d8d421 100644
--- a/clippy_lints/src/methods/unnecessary_to_owned.rs
+++ b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -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(
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
index d28f2bd8c17..f08275a4ac7 100644
--- a/clippy_utils/src/sugg.rs
+++ b/clippy_utils/src/sugg.rs
@@ -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))
             },
diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed
index 9c1098dc4c1..2c8339cdd7f 100644
--- a/tests/ui/bool_to_int_with_if.fixed
+++ b/tests/ui/bool_to_int_with_if.fixed
@@ -14,6 +14,7 @@ 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);
@@ -21,7 +22,12 @@ fn main() {
     // 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
 
diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs
index 0c967dac6e2..5d9496f0177 100644
--- a/tests/ui/bool_to_int_with_if.rs
+++ b/tests/ui/bool_to_int_with_if.rs
@@ -17,6 +17,11 @@ fn main() {
     } else {
         0
     };
+    if a {
+        0
+    } else {
+        1
+    };
     if !a {
         1
     } else {
@@ -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 {
diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr
index 8647a9cffbe..e695440f668 100644
--- a/tests/ui/bool_to_int_with_if.stderr
+++ b/tests/ui/bool_to_int_with_if.stderr
@@ -14,6 +14,18 @@ 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 {
@@ -21,10 +33,10 @@ 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
@@ -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
@@ -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
@@ -60,7 +72,7 @@ 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 {
    |  ____________^
@@ -68,17 +80,30 @@ 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
 
diff --git a/tests/ui/len_without_is_empty.rs b/tests/ui/len_without_is_empty.rs
index 1e938e72b57..78397c2af34 100644
--- a/tests/ui/len_without_is_empty.rs
+++ b/tests/ui/len_without_is_empty.rs
@@ -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 {