Full fix of get unwrap issue

2 files changed, 26 insertions, 15 deletions

diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index 32e6a83e535..6bf57383a7a 100644
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -1404,22 +1404,33 @@ fn lint_get_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, get_args: &[hir::
     // Note: we don't want to lint `get_mut().unwrap` for HashMap or BTreeMap,
     // because they do not implement `IndexMut`
     let expr_ty = cx.tables.expr_ty(&get_args[0]);
+    let get_args_str = if get_args.len() > 1 {
+        snippet(cx, get_args[1].span, "_")
+    } else {
+        return; // not linting on a .get().unwrap() chain or variant
+    };
+    let needs_ref;
     let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
+        needs_ref = get_args_str.parse::<usize>().is_ok();
         "slice"
     } else if match_type(cx, expr_ty, &paths::VEC) {
+        needs_ref = get_args_str.parse::<usize>().is_ok();
         "Vec"
     } else if match_type(cx, expr_ty, &paths::VEC_DEQUE) {
+        needs_ref = get_args_str.parse::<usize>().is_ok();
         "VecDeque"
     } else if !is_mut && match_type(cx, expr_ty, &paths::HASHMAP) {
+        needs_ref = true;
         "HashMap"
     } else if !is_mut && match_type(cx, expr_ty, &paths::BTREEMAP) {
+        needs_ref = true;
         "BTreeMap"
     } else {
         return; // caller is not a type that we want to lint
     };
 
     let mut_str = if is_mut { "_mut" } else { "" };
-    let borrow_str = if is_mut { "&mut " } else { "&" };
+    let borrow_str = if !needs_ref { "" } else if is_mut { "&mut " } else { "&" };
     span_lint_and_sugg(
         cx,
         GET_UNWRAP,
@@ -1431,10 +1442,10 @@ fn lint_get_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, get_args: &[hir::
         ),
         "try this",
         format!(
-            "({}{}[{}])",
+            "{}{}[{}]",
             borrow_str,
             snippet(cx, get_args[0].span, "_"),
-            snippet(cx, get_args[1].span, "_")
+            get_args_str
         ),
     );
 }
diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr
index 6b9a360f332..669903da190 100644
--- a/tests/ui/get_unwrap.stderr
+++ b/tests/ui/get_unwrap.stderr
@@ -2,7 +2,7 @@ error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more co
   --> $DIR/get_unwrap.rs:27:17
    |
 27 |         let _ = boxed_slice.get(1).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&boxed_slice[1])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&boxed_slice[1]`
    |
    = note: `-D clippy::get-unwrap` implied by `-D warnings`
 
@@ -10,67 +10,67 @@ error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more co
   --> $DIR/get_unwrap.rs:28:17
    |
 28 |         let _ = some_slice.get(0).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_slice[0])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_slice[0]`
 
 error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:29:17
    |
 29 |         let _ = some_vec.get(0).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_vec[0])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_vec[0]`
 
 error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:30:17
    |
 30 |         let _ = some_vecdeque.get(0).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_vecdeque[0])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_vecdeque[0]`
 
 error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:31:17
    |
 31 |         let _ = some_hashmap.get(&1).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_hashmap[&1])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_hashmap[&1]`
 
 error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:32:17
    |
 32 |         let _ = some_btreemap.get(&1).unwrap();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_btreemap[&1])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_btreemap[&1]`
 
 error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:37:10
    |
 37 |         *boxed_slice.get_mut(0).unwrap() = 1;
-   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&mut boxed_slice[0])`
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut boxed_slice[0]`
 
 error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:38:10
    |
 38 |         *some_slice.get_mut(0).unwrap() = 1;
-   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&mut some_slice[0])`
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut some_slice[0]`
 
 error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:39:10
    |
 39 |         *some_vec.get_mut(0).unwrap() = 1;
-   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&mut some_vec[0])`
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut some_vec[0]`
 
 error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:40:10
    |
 40 |         *some_vecdeque.get_mut(0).unwrap() = 1;
-   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&mut some_vecdeque[0])`
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut some_vecdeque[0]`
 
 error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:48:17
    |
 48 |         let _ = some_vec.get(0..1).unwrap().to_vec();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&some_vec[0..1])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0..1]`
 
 error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
   --> $DIR/get_unwrap.rs:49:17
    |
 49 |         let _ = some_vec.get_mut(0..1).unwrap().to_vec();
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&mut some_vec[0..1])`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0..1]`
 
 error: aborting due to 12 previous errors