From e4cd8e7961b553cac44671d63bc6dfc2223ea66b Mon Sep 17 00:00:00 2001
From: Eduardo Broto <ebroto@tutanota.com>
Date: Tue, 12 May 2020 22:05:56 +0200
Subject: [PATCH 1/2] Fix ICE caused in unwrap module

---
 clippy_lints/src/unwrap.rs                    | 12 +++++++++--
 .../ui/checked_unwrap/simple_conditionals.rs  | 21 +++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs
index f3844c7d3b6..8b971e7064b 100644
--- a/clippy_lints/src/unwrap.rs
+++ b/clippy_lints/src/unwrap.rs
@@ -8,6 +8,7 @@ use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Path, QPath, UnO
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::Ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 
@@ -90,6 +91,14 @@ fn collect_unwrap_info<'a, 'tcx>(
     branch: &'tcx Expr<'_>,
     invert: bool,
 ) -> Vec<UnwrapInfo<'tcx>> {
+    fn is_relevant_option_call(cx: &LateContext<'_, '_>, ty: Ty<'_>, method_name: &str) -> bool {
+        is_type_diagnostic_item(cx, ty, sym!(option_type)) && ["is_some", "is_none"].contains(&method_name)
+    }
+
+    fn is_relevant_result_call(cx: &LateContext<'_, '_>, ty: Ty<'_>, method_name: &str) -> bool {
+        is_type_diagnostic_item(cx, ty, sym!(result_type)) && ["is_ok", "is_err"].contains(&method_name)
+    }
+
     if let ExprKind::Binary(op, left, right) = &expr.kind {
         match (invert, op.node) {
             (false, BinOpKind::And) | (false, BinOpKind::BitAnd) | (true, BinOpKind::Or) | (true, BinOpKind::BitOr) => {
@@ -106,9 +115,8 @@ fn collect_unwrap_info<'a, 'tcx>(
             if let ExprKind::MethodCall(method_name, _, args) = &expr.kind;
             if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind;
             let ty = cx.tables.expr_ty(&args[0]);
-            if is_type_diagnostic_item(cx, ty, sym!(option_type)) || is_type_diagnostic_item(cx, ty, sym!(result_type));
             let name = method_name.ident.as_str();
-            if ["is_some", "is_none", "is_ok", "is_err"].contains(&&*name);
+            if is_relevant_option_call(cx, ty, &name) || is_relevant_result_call(cx, ty, &name);
             then {
                 assert!(args.len() == 1);
                 let unwrappable = match name.as_ref() {
diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs
index 3e7b4b390ba..49794e0c241 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.rs
+++ b/tests/ui/checked_unwrap/simple_conditionals.rs
@@ -78,3 +78,24 @@ fn main() {
 
     assert!(x.is_ok(), "{:?}", x.unwrap_err()); // ok, it's a common test pattern
 }
+
+mod issue_5579 {
+    trait IsErr {
+        fn is_err(&self, err: &str) -> bool;
+    }
+
+    impl<T> IsErr for Option<T> {
+        fn is_err(&self, _err: &str) -> bool {
+            true
+        }
+    }
+
+    #[allow(unused)]
+    fn boom() {
+        let t = Some(1);
+
+        if t.is_err("") {
+            t.unwrap();
+        }
+    }
+}

From 8d1029d3ca013687422b58d0e99084a4e3421089 Mon Sep 17 00:00:00 2001
From: Eduardo Broto <ebroto@tutanota.com>
Date: Wed, 13 May 2020 20:47:44 +0200
Subject: [PATCH 2/2] Move test for issue 5579 under tests/ui/crashes

---
 .../ui/checked_unwrap/simple_conditionals.rs  | 21 -------------------
 tests/ui/crashes/ice-5579.rs                  | 17 +++++++++++++++
 2 files changed, 17 insertions(+), 21 deletions(-)
 create mode 100644 tests/ui/crashes/ice-5579.rs

diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs
index 49794e0c241..3e7b4b390ba 100644
--- a/tests/ui/checked_unwrap/simple_conditionals.rs
+++ b/tests/ui/checked_unwrap/simple_conditionals.rs
@@ -78,24 +78,3 @@ fn main() {
 
     assert!(x.is_ok(), "{:?}", x.unwrap_err()); // ok, it's a common test pattern
 }
-
-mod issue_5579 {
-    trait IsErr {
-        fn is_err(&self, err: &str) -> bool;
-    }
-
-    impl<T> IsErr for Option<T> {
-        fn is_err(&self, _err: &str) -> bool {
-            true
-        }
-    }
-
-    #[allow(unused)]
-    fn boom() {
-        let t = Some(1);
-
-        if t.is_err("") {
-            t.unwrap();
-        }
-    }
-}
diff --git a/tests/ui/crashes/ice-5579.rs b/tests/ui/crashes/ice-5579.rs
new file mode 100644
index 00000000000..e1842c73f0e
--- /dev/null
+++ b/tests/ui/crashes/ice-5579.rs
@@ -0,0 +1,17 @@
+trait IsErr {
+    fn is_err(&self, err: &str) -> bool;
+}
+
+impl<T> IsErr for Option<T> {
+    fn is_err(&self, _err: &str) -> bool {
+        true
+    }
+}
+
+fn main() {
+    let t = Some(1);
+
+    if t.is_err("") {
+        t.unwrap();
+    }
+}