Merge pull request #3178 from ms2300/bad_unwrap

Fix for bad get unwrap suggestion

clippy_lints/src/methods.rs

@@ -1426,22 +1426,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,
@@ -1456,7 +1467,7 @@ fn lint_get_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, get_args: &[hir::
  "{}{}[{}]",
  borrow_str,
  snippet(cx, get_args[0].span, "_"),
- snippet(cx, get_args[1].span, "_")
+ get_args_str
  ),
  );
 }

tests/ui/get_unwrap.rs

@@ -43,4 +43,9 @@ fn main() {
  *some_btreemap.get_mut(&1).unwrap() = 'b';
  *false_positive.get_mut(0).unwrap() = 1;
  }
+
+ { // Test `get().unwrap().foo()` and `get_mut().unwrap().bar()`
+ let _ = some_vec.get(0..1).unwrap().to_vec();
+ let _ = some_vec.get_mut(0..1).unwrap().to_vec();
+ }
 }

tests/ui/get_unwrap.stderr

@@ -60,5 +60,17 @@ error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and
 40 | *some_vecdeque.get_mut(0).unwrap() = 1;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut some_vecdeque[0]`
 
-error: aborting due to 10 previous errors
+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]`
+
+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: `some_vec[0..1]`
+
+error: aborting due to 12 previous errors