From 6a14dc7fd45dc620f3b873f939cff4205585342d Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Tue, 1 Nov 2016 17:48:32 -0700 Subject: [PATCH] Remove false positives from `get_unwrap` lint HashMap and BTreeMap don't implement `IndexMut`, so we shouldn't lint for use of `get_mut().unwrap()` for those types. --- clippy_lints/src/methods.rs | 6 ++++-- tests/compile-fail/methods.rs | 11 +++-------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index f73f00732714..32d230bd73d5 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -854,6 +854,8 @@ fn lint_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &MethodArgs, is_ } fn lint_get_unwrap(cx: &LateContext, expr: &hir::Expr, get_args: &MethodArgs, is_mut: bool) { + // Note: we don't want to lint `get_mut().unwrap` for HashMap or BTreeMap, + // because they do not implement `IndexMut` let expr_ty = cx.tcx.expr_ty(&get_args[0]); let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() { "slice" @@ -861,9 +863,9 @@ fn lint_get_unwrap(cx: &LateContext, expr: &hir::Expr, get_args: &MethodArgs, is "Vec" } else if match_type(cx, expr_ty, &paths::VEC_DEQUE) { "VecDeque" - } else if match_type(cx, expr_ty, &paths::HASHMAP) { + } else if !is_mut && match_type(cx, expr_ty, &paths::HASHMAP) { "HashMap" - } else if match_type(cx, expr_ty, &paths::BTREEMAP) { + } else if !is_mut && match_type(cx, expr_ty, &paths::BTREEMAP) { "BTreeMap" } else { return; // caller is not a type that we want to lint diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 2e6072b92fbb..9d430e7ac7ca 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -445,15 +445,10 @@ fn get_unwrap() { //~^ERROR called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise //~|HELP try this //~|SUGGESTION &mut some_vecdeque[0] - *some_hashmap.get_mut(&1).unwrap() = 'b'; - //~^ERROR called `.get_mut().unwrap()` on a HashMap. Using `[]` is more clear and more concise - //~|HELP try this - //~|SUGGESTION &mut some_hashmap[&1] - *some_btreemap.get_mut(&1).unwrap() = 'b'; - //~^ERROR called `.get_mut().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise - //~|HELP try this - //~|SUGGESTION &mut some_btreemap[&1] + // Check false positives + *some_hashmap.get_mut(&1).unwrap() = 'b'; + *some_btreemap.get_mut(&1).unwrap() = 'b'; *false_positive.get_mut(0).unwrap() = 1; } }