fix issue rust-lang#7731

Make if_then_else handle situation of cond.kind = ExprKind::DropTemps(ExprKind::Binary(BinOpKind::And || BinOpKind::Or, left, right), ..) =
pineapplehunter · Sep 30, 2021 · 4babfae · 4babfae
1 parent 685b773
commit 4babfae
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 6 deletions.
diff --git a/clippy_lints/src/if_then_panic.rs b/clippy_lints/src/if_then_panic.rs
@@ -3,7 +3,7 @@ use clippy_utils::higher::PanicExpn;
 use clippy_utils::is_expn_of;
 use clippy_utils::source::snippet_with_applicability;
 use rustc_errors::Applicability;
-use rustc_hir::{Block, Expr, ExprKind, StmtKind, UnOp};
+use rustc_hir::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
@@ -74,10 +74,38 @@ impl LateLintPass<'_> for IfThenPanic {
                 };
                 let mut applicability = Applicability::MachineApplicable;
                 let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
-
-                let cond_sugg =
-                if let ExprKind::DropTemps(Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..}) = cond.kind {
-                    snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
+                //let mut cond_sugg = format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability));
+                let cond_sugg = if let ExprKind::DropTemps(e, ..) = cond.kind {
+                    if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = e {
+                        snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
+                    } else if let Expr{kind: ExprKind::Binary(op, left, right), ..} = e {//BinOp{BinOpKind::And, ..}
+                        match op.node {
+                            BinOpKind::And | BinOpKind::Or => {
+                                let left_span =  {
+                                    if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = left {
+                                        snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
+                                    } else {
+                                        format!("!{}", snippet_with_applicability(cx, left.span, "..", &mut applicability))
+                                    }
+                                };
+                                let right_span = {
+                                    if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = right {
+                                        snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
+                                    } else {
+                                        format!("!{}", snippet_with_applicability(cx, right.span, "..", &mut applicability))
+                                    }
+                                };
+                                if op.node == BinOpKind::And {
+                                  format!("{} || {}", left_span, right_span)
+                                } else  {
+                                  format!("{} && {}", left_span, right_span)
+                                }
+                            }
+                            _ => format!("!({})", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
+                        }
+                    } else {
+                        format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
+                    }
                 } else {
                     format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
                 };

diff --git a/tests/ui/if_then_panic.fixed b/tests/ui/if_then_panic.fixed
@@ -31,4 +31,10 @@ fn main() {
     } else {
         println!("qwq");
     }
+    let b = vec![1, 2, 3];
+    assert!(!b.is_empty(), "panic1");
+    assert!(!b.is_empty() || !a.is_empty(), "panic2");
+    assert!(!a.is_empty() || b.is_empty(), "panic3");
+    assert!(!b.is_empty() && !a.is_empty(), "panic4");
+    assert!(!a.is_empty() && b.is_empty(), "panic5");
 }
diff --git a/tests/ui/if_then_panic.rs b/tests/ui/if_then_panic.rs
@@ -35,4 +35,20 @@ fn main() {
     } else {
         println!("qwq");
     }
+    let b = vec![1, 2, 3];
+    if b.is_empty() {
+        panic!("panic1");
+    }
+    if b.is_empty() && a.is_empty() {
+        panic!("panic2");
+    }
+    if a.is_empty() && !b.is_empty() {
+        panic!("panic3");
+    }
+    if b.is_empty() || a.is_empty() {
+        panic!("panic4");
+    }
+    if a.is_empty() || !b.is_empty() {
+        panic!("panic5");
+    }
 }
diff --git a/tests/ui/if_then_panic.stderr b/tests/ui/if_then_panic.stderr
@@ -16,5 +16,45 @@ LL | |         panic!("qwqwq");
 LL | |     }
    | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
 
-error: aborting due to 2 previous errors
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:39:5
+   |
+LL | /     if b.is_empty() {
+LL | |         panic!("panic1");
+LL | |     }
+   | |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:42:5
+   |
+LL | /     if b.is_empty() && a.is_empty() {
+LL | |         panic!("panic2");
+LL | |     }
+   | |_____^ help: try: `assert!(!b.is_empty() || !a.is_empty(), "panic2");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:45:5
+   |
+LL | /     if a.is_empty() && !b.is_empty() {
+LL | |         panic!("panic3");
+LL | |     }
+   | |_____^ help: try: `assert!(!a.is_empty() || b.is_empty(), "panic3");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:48:5
+   |
+LL | /     if b.is_empty() || a.is_empty() {
+LL | |         panic!("panic4");
+LL | |     }
+   | |_____^ help: try: `assert!(!b.is_empty() && !a.is_empty(), "panic4");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:51:5
+   |
+LL | /     if a.is_empty() || !b.is_empty() {
+LL | |         panic!("panic5");
+LL | |     }
+   | |_____^ help: try: `assert!(!a.is_empty() && b.is_empty(), "panic5");`
+
+error: aborting due to 7 previous errors