OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Implement lint checking for unwraps that will always panic.

Browse Source
This commit is contained in:
Fabian Zaiser 2018-06-08 18:12:01 +02:00
parent 8f0edba6e9
commit 23404287fc
3 changed files with 340 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
clippy_lints/src/unwrap.rs
View File

@ -32,6 +32,27 @@ declare_clippy_lint! {
"checks for calls of unwrap[_err]() that cannot fail"
}
/// **What it does:** Checks for calls of unwrap[_err]() that will always fail.
///
/// **Why is this bad?** If panicking is desired, an explicit `panic!()` should be used.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// if option.is_none() {
/// do_something_with(option.unwrap())
/// }
/// ```
///
/// This code will always panic. The if condition should probably be inverted.
/// ```
declare_clippy_lint! {
pub PANICKING_UNWRAP,
nursery,
"checks for calls of unwrap[_err]() that will always fail"
}
pub struct Pass;
/// Visitor that keeps track of which variables are unwrappable.
@ -124,17 +145,28 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
if ["unwrap", "unwrap_err"].contains(&&*method_name.name.as_str());
let call_to_unwrap = method_name.name == "unwrap";
if let Some(unwrappable) = self.unwrappables.iter()
.find(|u| u.ident.def == path.def && call_to_unwrap == u.safe_to_unwrap);
.find(|u| u.ident.def == path.def);
then {
span_lint_and_then(
self.cx,
UNNECESSARY_UNWRAP,
expr.span,
&format!("You checked before that `{}()` cannot fail. \
Instead of checking and unwrapping, it's better to use `if let` or `match`.",
method_name.name),
|db| { db.span_label(unwrappable.check.span, "the check is happening here"); },
);
if call_to_unwrap == unwrappable.safe_to_unwrap {
span_lint_and_then(
self.cx,
UNNECESSARY_UNWRAP,
expr.span,
&format!("You checked before that `{}()` cannot fail. \
Instead of checking and unwrapping, it's better to use `if let` or `match`.",
method_name.name),
|db| { db.span_label(unwrappable.check.span, "the check is happening here"); },
);
} else {
span_lint_and_then(
self.cx,
UNNECESSARY_UNWRAP,
expr.span,
&format!("This call to `{}()` will always panic.",
method_name.name),
|db| { db.span_label(unwrappable.check.span, "because of this check"); },
);
}
}
}
walk_expr(self, expr);

72
tests/ui/checked_unwrap.rs
View File

@ -1,32 +1,41 @@
#![deny(unnecessary_unwrap)]
#![allow(if_same_then_else)]
fn main() {
let x = Some(());
if x.is_some() {
x.unwrap();
x.unwrap(); // unnecessary
} else {
x.unwrap(); // will panic
}
if x.is_none() {
// nothing to do here
x.unwrap(); // will panic
} else {
x.unwrap();
x.unwrap(); // unnecessary
}
let mut x: Result<(), ()> = Ok(());
if x.is_ok() {
x.unwrap();
x.unwrap(); // unnecessary
x.unwrap_err(); // will panic
} else {
x.unwrap_err();
x.unwrap(); // will panic
x.unwrap_err(); // unnecessary
}
if x.is_err() {
x.unwrap_err();
x.unwrap(); // will panic
x.unwrap_err(); // unnecessary
} else {
x.unwrap();
x.unwrap(); // unnecessary
x.unwrap_err(); // will panic
}
if x.is_ok() {
x = Err(());
x.unwrap();
x.unwrap(); // not unnecessary because of mutation of x
// it will always panic but the lint is not smart enoguh to see this (it only checks if conditions).
} else {
x = Ok(());
x.unwrap_err();
x.unwrap_err(); // not unnecessary because of mutation of x
// it will always panic but the lint is not smart enoguh to see this (it only checks if conditions).
}
}
@ -34,34 +43,49 @@ fn test_complex_conditions() {
let x: Result<(), ()> = Ok(());
let y: Result<(), ()> = Ok(());
if x.is_ok() && y.is_err() {
x.unwrap();
y.unwrap_err();
x.unwrap(); // unnecessary
x.unwrap_err(); // will panic
y.unwrap(); // will panic
y.unwrap_err(); // unnecessary
} else {
// not clear whether unwrappable:
// not statically determinable whether any of the following will always succeed or always fail:
x.unwrap();
x.unwrap_err();
y.unwrap();
y.unwrap_err();
}
if x.is_ok() || y.is_ok() {
// not clear whether unwrappable:
// not statically determinable whether any of the following will always succeed or always fail:
x.unwrap();
y.unwrap();
} else {
x.unwrap_err();
y.unwrap_err();
x.unwrap(); // will panic
x.unwrap_err(); // unnecessary
y.unwrap(); // will panic
y.unwrap_err(); // unnecessary
}
let z: Result<(), ()> = Ok(());
if x.is_ok() && !(y.is_ok() || z.is_err()) {
x.unwrap();
y.unwrap_err();
z.unwrap();
x.unwrap(); // unnecessary
x.unwrap_err(); // will panic
y.unwrap(); // will panic
y.unwrap_err(); // unnecessary
z.unwrap(); // unnecessary
z.unwrap_err(); // will panic
}
if x.is_ok() || !(y.is_ok() && z.is_err()) {
// not clear what's unwrappable
} else {
x.unwrap_err();
// not statically determinable whether any of the following will always succeed or always fail:
x.unwrap();
y.unwrap();
z.unwrap_err();
z.unwrap();
} else {
x.unwrap(); // will panic
x.unwrap_err(); // unnecessary
y.unwrap(); // unnecessary
y.unwrap_err(); // will panic
z.unwrap(); // will panic
z.unwrap_err(); // unnecessary
}
}
@ -69,7 +93,9 @@ fn test_nested() {
fn nested() {
let x = Some(());
if x.is_some() {
x.unwrap();
x.unwrap(); // unnecessary
} else {
x.unwrap(); // will panic
}
}
}

346
tests/ui/checked_unwrap.stderr
View File

@ -1,9 +1,9 @@
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:6:9
--> $DIR/checked_unwrap.rs:7:9
|
5 | if x.is_some() {
6 | if x.is_some() {
| ----------- the check is happening here
6 | x.unwrap();
7 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
|
note: lint level defined here
@ -12,144 +12,296 @@ note: lint level defined here
1 | #![deny(unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:11:9
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:9:9
|
6 | if x.is_some() {
| ----------- because of this check
...
9 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:12:9
|
8 | if x.is_none() {
11 | if x.is_none() {
| ----------- because of this check
12 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:14:9
|
11 | if x.is_none() {
| ----------- the check is happening here
...
11 | x.unwrap();
14 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:15:9
--> $DIR/checked_unwrap.rs:18:9
|
14 | if x.is_ok() {
17 | if x.is_ok() {
| --------- the check is happening here
15 | x.unwrap();
18 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:17:9
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:19:9
|
14 | if x.is_ok() {
| --------- the check is happening here
17 | if x.is_ok() {
| --------- because of this check
18 | x.unwrap(); // unnecessary
19 | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:21:9
|
17 | if x.is_ok() {
| --------- because of this check
...
17 | x.unwrap_err();
| ^^^^^^^^^^^^^^
21 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:20:9
|
19 | if x.is_err() {
| ---------- the check is happening here
20 | x.unwrap_err();
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:22:9
|
19 | if x.is_err() {
17 | if x.is_ok() {
| --------- the check is happening here
...
22 | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:25:9
|
24 | if x.is_err() {
| ---------- because of this check
25 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:26:9
|
24 | if x.is_err() {
| ---------- the check is happening here
25 | x.unwrap(); // will panic
26 | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:28:9
|
24 | if x.is_err() {
| ---------- the check is happening here
...
22 | x.unwrap();
28 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:37:9
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:29:9
|
36 | if x.is_ok() && y.is_err() {
24 | if x.is_err() {
| ---------- because of this check
...
29 | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:46:9
|
45 | if x.is_ok() && y.is_err() {
| --------- the check is happening here
37 | x.unwrap();
46 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:47:9
|
45 | if x.is_ok() && y.is_err() {
| --------- because of this check
46 | x.unwrap(); // unnecessary
47 | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:48:9
|
45 | if x.is_ok() && y.is_err() {
| ---------- because of this check
...
48 | y.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:38:9
--> $DIR/checked_unwrap.rs:49:9
|
36 | if x.is_ok() && y.is_err() {
45 | if x.is_ok() && y.is_err() {
| ---------- the check is happening here
37 | x.unwrap();
38 | y.unwrap_err();
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:50:9
|
45 | if x.is_ok() || y.is_ok() {
| --------- the check is happening here
...
50 | x.unwrap_err();
49 | y.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:51:9
|
45 | if x.is_ok() || y.is_ok() {
| --------- the check is happening here
...
51 | y.unwrap_err();
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:55:9
|
54 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- the check is happening here
55 | x.unwrap();
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:56:9
|
54 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- the check is happening here
55 | x.unwrap();
56 | y.unwrap_err();
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:57:9
|
54 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| ---------- the check is happening here
...
57 | z.unwrap();
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:62:9
|
59 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- the check is happening here
...
62 | x.unwrap_err();
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:63:9
|
59 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- the check is happening here
58 | if x.is_ok() || y.is_ok() {
| --------- because of this check
...
63 | y.unwrap();
63 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:64:9
|
59 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| ---------- the check is happening here
58 | if x.is_ok() || y.is_ok() {
| --------- the check is happening here
...
64 | z.unwrap_err();
64 | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:65:9
|
58 | if x.is_ok() || y.is_ok() {
| --------- because of this check
...
65 | y.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:66:9
|
58 | if x.is_ok() || y.is_ok() {
| --------- the check is happening here
...
66 | y.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:72:13
--> $DIR/checked_unwrap.rs:70:9
|
71 | if x.is_some() {
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- the check is happening here
70 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:71:9
|
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- because of this check
70 | x.unwrap(); // unnecessary
71 | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:72:9
|
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- because of this check
...
72 | y.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:73:9
|
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| --------- the check is happening here
...
73 | y.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:74:9
|
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| ---------- the check is happening here
...
74 | z.unwrap(); // unnecessary
| ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:75:9
|
69 | if x.is_ok() && !(y.is_ok() || z.is_err()) {
| ---------- because of this check
...
75 | z.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:83:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- because of this check
...
83 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:84:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- the check is happening here
...
84 | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:85:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- the check is happening here
...
85 | y.unwrap(); // unnecessary
| ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic.
--> $DIR/checked_unwrap.rs:86:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| --------- because of this check
...
86 | y.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:87:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| ---------- because of this check
...
87 | z.unwrap(); // will panic
| ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:88:9
|
77 | if x.is_ok() || !(y.is_ok() && z.is_err()) {
| ---------- the check is happening here
...
88 | z.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/checked_unwrap.rs:96:13
|
95 | if x.is_some() {
| ----------- the check is happening here
72 | x.unwrap();
96 | x.unwrap(); // unnecessary
| ^^^^^^^^^^
error: aborting due to 17 previous errors
error: This call to `unwrap()` will always panic.
--> $DIR/checked_unwrap.rs:98:13
|
95 | if x.is_some() {
| ----------- because of this check
...
98 | x.unwrap(); // will panic
| ^^^^^^^^^^
error: aborting due to 34 previous errors