Clean up `get_unwrap` code

This commit is contained in:
Devon Hollowood 2016-11-01 17:33:43 -07:00
parent 1187d333ec
commit 4e0d182d1d
2 changed files with 19 additions and 14 deletions

View File

@ -519,7 +519,9 @@ impl LintPass for Pass {
} }
impl LateLintPass for Pass { impl LateLintPass for Pass {
#[allow(cyclomatic_complexity)] #[allow(unused_attributes)]
// ^ required because `cyclomatic_complexity` attribute shows up as unused
#[cyclomatic_complexity = "30"]
fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) { fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) {
if in_macro(cx, expr.span) { if in_macro(cx, expr.span) {
return; return;
@ -530,7 +532,7 @@ impl LateLintPass for Pass {
// Chain calls // Chain calls
// GET_UNWRAP needs to be checked before general `UNWRAP` lints // GET_UNWRAP needs to be checked before general `UNWRAP` lints
if let Some(arglists) = method_chain_args(expr, &["get", "unwrap"]) { if let Some(arglists) = method_chain_args(expr, &["get", "unwrap"]) {
lint_get_unwrap(cx, expr, arglists[0], false); lint_get_unwrap(cx, expr, arglists[0], false);
} else if let Some(arglists) = method_chain_args(expr, &["get_mut", "unwrap"]) { } else if let Some(arglists) = method_chain_args(expr, &["get_mut", "unwrap"]) {
lint_get_unwrap(cx, expr, arglists[0], true); lint_get_unwrap(cx, expr, arglists[0], true);
} else if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { } else if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
@ -852,21 +854,23 @@ 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) { fn lint_get_unwrap(cx: &LateContext, expr: &hir::Expr, get_args: &MethodArgs, is_mut: bool) {
let mut_str = if is_mut { "_mut" } else {""}; let expr_ty = cx.tcx.expr_ty(&get_args[0]);
let caller_type = if derefs_to_slice(cx, &get_args[0], cx.tcx.expr_ty(&get_args[0])).is_some() { let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
"slice" "slice"
} else if match_type(cx, cx.tcx.expr_ty(&get_args[0]), &paths::VEC) { } else if match_type(cx, expr_ty, &paths::VEC) {
"Vec" "Vec"
} else if match_type(cx, cx.tcx.expr_ty(&get_args[0]), &paths::VEC_DEQUE) { } else if match_type(cx, expr_ty, &paths::VEC_DEQUE) {
"VecDeque" "VecDeque"
} else if match_type(cx, cx.tcx.expr_ty(&get_args[0]), &paths::HASHMAP) { } else if match_type(cx, expr_ty, &paths::HASHMAP) {
"HashMap" "HashMap"
} else if match_type(cx, cx.tcx.expr_ty(&get_args[0]), &paths::BTREEMAP) { } else if match_type(cx, expr_ty, &paths::BTREEMAP) {
"BTreeMap" "BTreeMap"
} else { } else {
return; // caller is not a type that we want to lint 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 {""};
span_lint_and_then( span_lint_and_then(
cx, cx,
GET_UNWRAP, GET_UNWRAP,
@ -877,7 +881,8 @@ fn lint_get_unwrap(cx: &LateContext, expr: &hir::Expr, get_args: &MethodArgs, is
db.span_suggestion( db.span_suggestion(
expr.span, expr.span,
"try this", "try this",
format!("{}[{}]", snippet(cx, get_args[0].span, "_"), snippet(cx, get_args[1].span, "_")) format!("{}{}[{}]", borrow_str, snippet(cx, get_args[0].span, "_"),
snippet(cx, get_args[1].span, "_"))
); );
} }
); );

View File

@ -436,23 +436,23 @@ fn get_unwrap() {
*some_slice.get_mut(0).unwrap() = 1; *some_slice.get_mut(0).unwrap() = 1;
//~^ERROR called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise //~^ERROR called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
//~|HELP try this //~|HELP try this
//~|SUGGESTION some_slice[0] //~|SUGGESTION &mut some_slice[0]
*some_vec.get_mut(0).unwrap() = 1; *some_vec.get_mut(0).unwrap() = 1;
//~^ERROR called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise //~^ERROR called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
//~|HELP try this //~|HELP try this
//~|SUGGESTION some_vec[0] //~|SUGGESTION &mut some_vec[0]
*some_vecdeque.get_mut(0).unwrap() = 1; *some_vecdeque.get_mut(0).unwrap() = 1;
//~^ERROR called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise //~^ERROR called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
//~|HELP try this //~|HELP try this
//~|SUGGESTION some_vecdeque[0] //~|SUGGESTION &mut some_vecdeque[0]
*some_hashmap.get_mut(&1).unwrap() = 'b'; *some_hashmap.get_mut(&1).unwrap() = 'b';
//~^ERROR called `.get_mut().unwrap()` on a HashMap. Using `[]` is more clear and more concise //~^ERROR called `.get_mut().unwrap()` on a HashMap. Using `[]` is more clear and more concise
//~|HELP try this //~|HELP try this
//~|SUGGESTION some_hashmap[&1] //~|SUGGESTION &mut some_hashmap[&1]
*some_btreemap.get_mut(&1).unwrap() = 'b'; *some_btreemap.get_mut(&1).unwrap() = 'b';
//~^ERROR called `.get_mut().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise //~^ERROR called `.get_mut().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise
//~|HELP try this //~|HELP try this
//~|SUGGESTION some_btreemap[&1] //~|SUGGESTION &mut some_btreemap[&1]
*false_positive.get_mut(0).unwrap() = 1; *false_positive.get_mut(0).unwrap() = 1;
} }