use let chains, move assignments out of block, add tests

4 files changed, 95 insertions, 69 deletions

diff --git a/clippy_lints/src/methods/get_unwrap.rs b/clippy_lints/src/methods/get_unwrap.rs
index 209d42ebabd..ba5aae28bd6 100644
--- a/clippy_lints/src/methods/get_unwrap.rs
+++ b/clippy_lints/src/methods/get_unwrap.rs
@@ -3,7 +3,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::get_parent_expr;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::is_type_diagnostic_item;
-use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
@@ -23,21 +22,16 @@ pub(super) fn check<'tcx>(
     let mut applicability = Applicability::MachineApplicable;
     let expr_ty = cx.typeck_results().expr_ty(recv);
     let get_args_str = snippet_with_applicability(cx, get_arg.span, "..", &mut applicability);
-    let mut needs_ref;
+    let mut needs_ref = true;
     let caller_type = if derefs_to_slice(cx, recv, expr_ty).is_some() {
-        needs_ref = true;
         "slice"
     } else if is_type_diagnostic_item(cx, expr_ty, sym::Vec) {
-        needs_ref = true;
         "Vec"
     } else if is_type_diagnostic_item(cx, expr_ty, sym::VecDeque) {
-        needs_ref = true;
         "VecDeque"
     } else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::HashMap) {
-        needs_ref = true;
         "HashMap"
     } else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::BTreeMap) {
-        needs_ref = true;
         "BTreeMap"
     } else {
         return; // caller is not a type that we want to lint
@@ -48,21 +42,19 @@ pub(super) fn check<'tcx>(
     // Handle the case where the result is immediately dereferenced,
     // either directly be the user, or as a result of a method call or the like
     // by not requiring an explicit reference
-    if_chain! {
-        if needs_ref;
-        if let Some(parent) = get_parent_expr(cx, expr);
-        if let hir::ExprKind::Unary(hir::UnOp::Deref, _)
+    if needs_ref
+        && let Some(parent) = get_parent_expr(cx, expr)
+        && let hir::ExprKind::Unary(hir::UnOp::Deref, _)
             | hir::ExprKind::MethodCall(..)
             | hir::ExprKind::Field(..)
-            | hir::ExprKind::Index(..) = parent.kind;
-        then {
-            if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind {
-                // if the user explicitly dereferences the result, we can adjust
-                // the span to also include the deref part
-                span = parent.span;
-            }
-            needs_ref = false;
+            | hir::ExprKind::Index(..) = parent.kind
+    {
+        if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind {
+            // if the user explicitly dereferences the result, we can adjust
+            // the span to also include the deref part
+            span = parent.span;
         }
+        needs_ref = false;
     }
 
     let mut_str = if is_mut { "_mut" } else { "" };
diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed
index 3e4c2b499e4..f0d28dd9349 100644
--- a/tests/ui/get_unwrap.fixed
+++ b/tests/ui/get_unwrap.fixed
@@ -69,17 +69,43 @@ fn main() {
         let _ = some_vec[0..1].to_vec();
         let _ = some_vec[0..1].to_vec();
     }
-    issue9909();
 }
-fn issue9909() {
-    let f = &[1, 2, 3];
+mod issue9909 {
+    #![allow(clippy::identity_op, clippy::unwrap_used, dead_code)]
 
-    let _x: &i32 = &f[1 + 2];
-    // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
+    fn reduced() {
+        let f = &[1, 2, 3];
 
-    let _x = f[1 + 2].to_string();
-    // ^ don't include a borrow here
+        let _x: &i32 = &f[1 + 2];
+        // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
 
-    let _x = f[1 + 2].abs();
-    // ^ don't include a borrow here
+        let _x = f[1 + 2].to_string();
+        // ^ don't include a borrow here
+
+        let _x = f[1 + 2].abs();
+        // ^ don't include a borrow here
+    }
+
+    // original code:
+    fn linidx(row: usize, col: usize) -> usize {
+        row * 1 + col * 3
+    }
+
+    fn main_() {
+        let mut mat = [1.0f32, 5.0, 9.0, 2.0, 6.0, 10.0, 3.0, 7.0, 11.0, 4.0, 8.0, 12.0];
+
+        for i in 0..2 {
+            for j in i + 1..3 {
+                if mat[linidx(j, 3)] > mat[linidx(i, 3)] {
+                    for k in 0..4 {
+                        let (x, rest) = mat.split_at_mut(linidx(i, k) + 1);
+                        let a = x.last_mut().unwrap();
+                        let b = &mut rest[linidx(j, k) - linidx(i, k) - 1];
+                        ::std::mem::swap(a, b);
+                    }
+                }
+            }
+        }
+        assert_eq!([9.0, 5.0, 1.0, 10.0, 6.0, 2.0, 11.0, 7.0, 3.0, 12.0, 8.0, 4.0], mat);
+    }
 }
diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs
index a967971a7dc..21c1ecb0af2 100644
--- a/tests/ui/get_unwrap.rs
+++ b/tests/ui/get_unwrap.rs
@@ -69,17 +69,43 @@ fn main() {
         let _ = some_vec.get(0..1).unwrap().to_vec();
         let _ = some_vec.get_mut(0..1).unwrap().to_vec();
     }
-    issue9909();
 }
-fn issue9909() {
-    let f = &[1, 2, 3];
+mod issue9909 {
+    #![allow(clippy::identity_op, clippy::unwrap_used, dead_code)]
 
-    let _x: &i32 = f.get(1 + 2).unwrap();
-    // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
+    fn reduced() {
+        let f = &[1, 2, 3];
 
-    let _x = f.get(1 + 2).unwrap().to_string();
-    // ^ don't include a borrow here
+        let _x: &i32 = f.get(1 + 2).unwrap();
+        // ^ include a borrow in the suggestion, even if the argument is not just a numeric literal
 
-    let _x = f.get(1 + 2).unwrap().abs();
-    // ^ don't include a borrow here
+        let _x = f.get(1 + 2).unwrap().to_string();
+        // ^ don't include a borrow here
+
+        let _x = f.get(1 + 2).unwrap().abs();
+        // ^ don't include a borrow here
+    }
+
+    // original code:
+    fn linidx(row: usize, col: usize) -> usize {
+        row * 1 + col * 3
+    }
+
+    fn main_() {
+        let mut mat = [1.0f32, 5.0, 9.0, 2.0, 6.0, 10.0, 3.0, 7.0, 11.0, 4.0, 8.0, 12.0];
+
+        for i in 0..2 {
+            for j in i + 1..3 {
+                if mat[linidx(j, 3)] > mat[linidx(i, 3)] {
+                    for k in 0..4 {
+                        let (x, rest) = mat.split_at_mut(linidx(i, k) + 1);
+                        let a = x.last_mut().unwrap();
+                        let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap();
+                        ::std::mem::swap(a, b);
+                    }
+                }
+            }
+        }
+        assert_eq!([9.0, 5.0, 1.0, 10.0, 6.0, 2.0, 11.0, 7.0, 3.0, 12.0, 8.0, 4.0], mat);
+    }
 }
diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr
index 6221c236830..154083ea654 100644
--- a/tests/ui/get_unwrap.stderr
+++ b/tests/ui/get_unwrap.stderr
@@ -188,46 +188,28 @@ LL |         let _ = some_vec.get_mut(0..1).unwrap().to_vec();
    = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
 
 error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
-  --> $DIR/get_unwrap.rs:77:20
+  --> $DIR/get_unwrap.rs:79:24
    |
-LL |     let _x: &i32 = f.get(1 + 2).unwrap();
-   |                    ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]`
-
-error: used `unwrap()` on an `Option` value
-  --> $DIR/get_unwrap.rs:77:20
-   |
-LL |     let _x: &i32 = f.get(1 + 2).unwrap();
-   |                    ^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
+LL |         let _x: &i32 = f.get(1 + 2).unwrap();
+   |                        ^^^^^^^^^^^^^^^^^^^^^ help: try this: `&f[1 + 2]`
 
 error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
-  --> $DIR/get_unwrap.rs:80:14
-   |
-LL |     let _x = f.get(1 + 2).unwrap().to_string();
-   |              ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
-
-error: used `unwrap()` on an `Option` value
-  --> $DIR/get_unwrap.rs:80:14
-   |
-LL |     let _x = f.get(1 + 2).unwrap().to_string();
-   |              ^^^^^^^^^^^^^^^^^^^^^
+  --> $DIR/get_unwrap.rs:82:18
    |
-   = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
+LL |         let _x = f.get(1 + 2).unwrap().to_string();
+   |                  ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
 
 error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
-  --> $DIR/get_unwrap.rs:83:14
+  --> $DIR/get_unwrap.rs:85:18
    |
-LL |     let _x = f.get(1 + 2).unwrap().abs();
-   |              ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
+LL |         let _x = f.get(1 + 2).unwrap().abs();
+   |                  ^^^^^^^^^^^^^^^^^^^^^ help: try this: `f[1 + 2]`
 
-error: used `unwrap()` on an `Option` value
-  --> $DIR/get_unwrap.rs:83:14
-   |
-LL |     let _x = f.get(1 + 2).unwrap().abs();
-   |              ^^^^^^^^^^^^^^^^^^^^^
+error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
+  --> $DIR/get_unwrap.rs:103:33
    |
-   = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message
+LL |                         let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap();
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut rest[linidx(j, k) - linidx(i, k) - 1]`
 
-error: aborting due to 32 previous errors
+error: aborting due to 30 previous errors