Skip to content

Commit

Permalink
Auto merge of #8372 - tamaroning:unwrap_used, r=llogiq
Browse files Browse the repository at this point in the history
make unwrap_used also trigger on .get().unwrap()

fixes #8124
changelog: make the [unwrap_used] lint trigger for code of the form such as `.get(i).unwrap()` and `.get_mut(i).unwrap()`
  • Loading branch information
bors committed Feb 5, 2022
2 parents 699ee5e + 447ed5c commit 29cc0d8
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 25 deletions.
15 changes: 11 additions & 4 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2402,10 +2402,17 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
implicit_clone::check(cx, name, expr, recv);
},
("unwrap", []) => match method_call(recv) {
Some(("get", [recv, get_arg], _)) => get_unwrap::check(cx, expr, recv, get_arg, false),
Some(("get_mut", [recv, get_arg], _)) => get_unwrap::check(cx, expr, recv, get_arg, true),
_ => unwrap_used::check(cx, expr, recv),
("unwrap", []) => {
match method_call(recv) {
Some(("get", [recv, get_arg], _)) => {
get_unwrap::check(cx, expr, recv, get_arg, false);
},
Some(("get_mut", [recv, get_arg], _)) => {
get_unwrap::check(cx, expr, recv, get_arg, true);
},
_ => {},
}
unwrap_used::check(cx, expr, recv);
},
("unwrap_or", [u_arg]) => match method_call(recv) {
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), [lhs, rhs], _)) => {
Expand Down
12 changes: 9 additions & 3 deletions tests/ui/get_unwrap.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// run-rustfix

#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
#![warn(clippy::unwrap_used)]
#![deny(clippy::get_unwrap)]

use std::collections::BTreeMap;
Expand Down Expand Up @@ -37,6 +39,7 @@ fn main() {
let _ = &some_vecdeque[0];
let _ = &some_hashmap[&1];
let _ = &some_btreemap[&1];
#[allow(clippy::unwrap_used)]
let _ = false_positive.get(0).unwrap();
// Test with deref
let _: u8 = boxed_slice[1];
Expand All @@ -49,9 +52,12 @@ fn main() {
some_vec[0] = 1;
some_vecdeque[0] = 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;
#[allow(clippy::unwrap_used)]
{
*some_hashmap.get_mut(&1).unwrap() = 'b';
*some_btreemap.get_mut(&1).unwrap() = 'b';
*false_positive.get_mut(0).unwrap() = 1;
}
}

{
Expand Down
12 changes: 9 additions & 3 deletions tests/ui/get_unwrap.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// run-rustfix

#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
#![warn(clippy::unwrap_used)]
#![deny(clippy::get_unwrap)]

use std::collections::BTreeMap;
Expand Down Expand Up @@ -37,6 +39,7 @@ fn main() {
let _ = some_vecdeque.get(0).unwrap();
let _ = some_hashmap.get(&1).unwrap();
let _ = some_btreemap.get(&1).unwrap();
#[allow(clippy::unwrap_used)]
let _ = false_positive.get(0).unwrap();
// Test with deref
let _: u8 = *boxed_slice.get(1).unwrap();
Expand All @@ -49,9 +52,12 @@ fn main() {
*some_vec.get_mut(0).unwrap() = 1;
*some_vecdeque.get_mut(0).unwrap() = 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;
#[allow(clippy::unwrap_used)]
{
*some_hashmap.get_mut(&1).unwrap() = 'b';
*some_btreemap.get_mut(&1).unwrap() = 'b';
*false_positive.get_mut(0).unwrap() = 1;
}
}

{
Expand Down
135 changes: 120 additions & 15 deletions tests/ui/get_unwrap.stderr
Original file line number Diff line number Diff line change
@@ -1,86 +1,191 @@
error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:34:17
--> $DIR/get_unwrap.rs:36:17
|
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&boxed_slice[1]`
|
note: the lint level is defined here
--> $DIR/get_unwrap.rs:3:9
--> $DIR/get_unwrap.rs:5:9
|
LL | #![deny(clippy::get_unwrap)]
| ^^^^^^^^^^^^^^^^^^

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:36:17
|
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unwrap-used` implied by `-D warnings`
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:35:17
--> $DIR/get_unwrap.rs:37:17
|
LL | let _ = some_slice.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_slice[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:37:17
|
LL | let _ = some_slice.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:36:17
--> $DIR/get_unwrap.rs:38:17
|
LL | let _ = some_vec.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_vec[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:38:17
|
LL | let _ = some_vec.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:37:17
--> $DIR/get_unwrap.rs:39:17
|
LL | let _ = some_vecdeque.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_vecdeque[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:39:17
|
LL | let _ = some_vecdeque.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:38:17
--> $DIR/get_unwrap.rs:40:17
|
LL | let _ = some_hashmap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_hashmap[&1]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:40:17
|
LL | let _ = some_hashmap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:39:17
--> $DIR/get_unwrap.rs:41:17
|
LL | let _ = some_btreemap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&some_btreemap[&1]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:41:17
|
LL | let _ = some_btreemap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:42:21
--> $DIR/get_unwrap.rs:45:21
|
LL | let _: u8 = *boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `boxed_slice[1]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:45:22
|
LL | let _: u8 = *boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:47:9
--> $DIR/get_unwrap.rs:50:9
|
LL | *boxed_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `boxed_slice[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:50:10
|
LL | *boxed_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:48:9
--> $DIR/get_unwrap.rs:51:9
|
LL | *some_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_slice[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:51:10
|
LL | *some_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:49:9
--> $DIR/get_unwrap.rs:52:9
|
LL | *some_vec.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:52:10
|
LL | *some_vec.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:50:9
--> $DIR/get_unwrap.rs:53:9
|
LL | *some_vecdeque.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vecdeque[0]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:53:10
|
LL | *some_vecdeque.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:59:17
--> $DIR/get_unwrap.rs:65:17
|
LL | let _ = some_vec.get(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0..1]`

error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:65:17
|
LL | let _ = some_vec.get(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:60:17
--> $DIR/get_unwrap.rs:66:17
|
LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `some_vec[0..1]`

error: aborting due to 13 previous errors
error: used `unwrap()` on `an Option` value
--> $DIR/get_unwrap.rs:66:17
|
LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message

error: aborting due to 26 previous errors

0 comments on commit 29cc0d8

Please sign in to comment.