From d2ea6355a8432e0042060796cd1b4e9060400c2f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Mon, 7 Jan 2019 06:22:39 +0200 Subject: [PATCH] Update `unwrap_get` code review suggestions --- clippy_lints/src/methods/mod.rs | 15 +++++++-------- tests/ui/get_unwrap.fixed | 2 ++ tests/ui/get_unwrap.rs | 2 ++ tests/ui/get_unwrap.stderr | 20 +++++++++++++------- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index afc875593ac..4473802da3b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1628,14 +1628,13 @@ fn lint_get_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, get_args: &[hir:: // Handle the case where the result is immedately dereferenced // by not requiring ref and pulling the dereference into the // suggestion. - if needs_ref { - if let Some(parent) = get_parent_expr(cx, expr) { - if let hir::ExprKind::Unary(op, _) = parent.node { - if op == hir::UnOp::UnDeref { - needs_ref = false; - span = parent.span; - } - } + if_chain! { + if needs_ref; + if let Some(parent) = get_parent_expr(cx, expr); + if let hir::ExprKind::Unary(hir::UnOp::UnDeref, _) = parent.node; + then { + needs_ref = false; + span = parent.span; } } diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed index 4badef1b803..021c0c2ff44 100644 --- a/tests/ui/get_unwrap.fixed +++ b/tests/ui/get_unwrap.fixed @@ -46,6 +46,8 @@ fn main() { let _ = &some_hashmap[&1]; let _ = &some_btreemap[&1]; let _ = false_positive.get(0).unwrap(); + // Test with deref + let _: u8 = boxed_slice[1]; } { diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs index d1a32dcbda3..b041ba7b7c7 100644 --- a/tests/ui/get_unwrap.rs +++ b/tests/ui/get_unwrap.rs @@ -46,6 +46,8 @@ fn main() { let _ = some_hashmap.get(&1).unwrap(); let _ = some_btreemap.get(&1).unwrap(); let _ = false_positive.get(0).unwrap(); + // Test with deref + let _: u8 = *boxed_slice.get(1).unwrap(); } { diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr index c947cf87d10..d4f699f5a72 100644 --- a/tests/ui/get_unwrap.stderr +++ b/tests/ui/get_unwrap.stderr @@ -36,41 +36,47 @@ error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more LL | let _ = some_btreemap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_btreemap[&1]` +error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise + --> $DIR/get_unwrap.rs:50:21 + | +LL | let _: u8 = *boxed_slice.get(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `boxed_slice[1]` + error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:53:9 + --> $DIR/get_unwrap.rs:55:9 | LL | *boxed_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `boxed_slice[0]` error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:54:9 + --> $DIR/get_unwrap.rs:56:9 | LL | *some_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_slice[0]` error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:55:9 + --> $DIR/get_unwrap.rs:57:9 | LL | *some_vec.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0]` error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:56:9 + --> $DIR/get_unwrap.rs:58:9 | LL | *some_vecdeque.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vecdeque[0]` error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:65:17 + --> $DIR/get_unwrap.rs:67:17 | LL | 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:66:17 + --> $DIR/get_unwrap.rs:68:17 | LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0..1]` -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors