Skip to content

Commit 48ad9d8

Browse files
committed
do not apply [assertions_on_result_states] to unwrap unit type
Signed-off-by: tabokie <[email protected]>
1 parent e00ceb9 commit 48ad9d8

4 files changed

+29
-8
lines changed

clippy_lints/src/assertions_on_result_states.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,14 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
5353
if result_type_with_refs != result_type {
5454
return;
5555
} else if let Res::Local(binding_id) = path_res(cx, recv)
56-
&& local_used_after_expr(cx, binding_id, recv) {
56+
&& local_used_after_expr(cx, binding_id, recv)
57+
{
5758
return;
5859
}
5960
}
6061
let mut app = Applicability::MachineApplicable;
6162
match method_segment.ident.as_str() {
62-
"is_ok" if has_debug_impl(cx, substs.type_at(1)) => {
63+
"is_ok" if type_suitable_to_unwrap(cx, substs.type_at(1)) => {
6364
span_lint_and_sugg(
6465
cx,
6566
ASSERTIONS_ON_RESULT_STATES,
@@ -73,7 +74,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
7374
app,
7475
);
7576
}
76-
"is_err" if has_debug_impl(cx, substs.type_at(0)) => {
77+
"is_err" if type_suitable_to_unwrap(cx, substs.type_at(0)) => {
7778
span_lint_and_sugg(
7879
cx,
7980
ASSERTIONS_ON_RESULT_STATES,
@@ -99,3 +100,7 @@ fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
99100
.get_diagnostic_item(sym::Debug)
100101
.map_or(false, |debug| implements_trait(cx, ty, debug, &[]))
101102
}
103+
104+
fn type_suitable_to_unwrap<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
105+
has_debug_impl(cx, ty) && !ty.is_unit() && !ty.is_never()
106+
}

tests/ui/assertions_on_result_states.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ fn main() {
2727
let r: Result<Foo, Foo> = Ok(Foo);
2828
assert!(r.is_ok());
2929

30+
// test ok with some messages
31+
let r: Result<Foo, DebugFoo> = Ok(Foo);
32+
assert!(r.is_ok(), "oops");
33+
34+
// test ok with unit error
35+
let r: Result<Foo, ()> = Ok(Foo);
36+
assert!(r.is_ok());
37+
3038
// test temporary ok
3139
fn get_ok() -> Result<Foo, DebugFoo> {
3240
Ok(Foo)

tests/ui/assertions_on_result_states.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ fn main() {
2727
let r: Result<Foo, Foo> = Ok(Foo);
2828
assert!(r.is_ok());
2929

30+
// test ok with some messages
31+
let r: Result<Foo, DebugFoo> = Ok(Foo);
32+
assert!(r.is_ok(), "oops");
33+
34+
// test ok with unit error
35+
let r: Result<Foo, ()> = Ok(Foo);
36+
assert!(r.is_ok());
37+
3038
// test temporary ok
3139
fn get_ok() -> Result<Foo, DebugFoo> {
3240
Ok(Foo)

tests/ui/assertions_on_result_states.stderr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,31 @@ LL | assert!(r.is_ok());
77
= note: `-D clippy::assertions-on-result-states` implied by `-D warnings`
88

99
error: called `assert!` with `Result::is_ok`
10-
--> $DIR/assertions_on_result_states.rs:34:5
10+
--> $DIR/assertions_on_result_states.rs:42:5
1111
|
1212
LL | assert!(get_ok().is_ok());
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok().unwrap()`
1414

1515
error: called `assert!` with `Result::is_ok`
16-
--> $DIR/assertions_on_result_states.rs:37:5
16+
--> $DIR/assertions_on_result_states.rs:45:5
1717
|
1818
LL | assert!(get_ok_macro!().is_ok());
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok_macro!().unwrap()`
2020

2121
error: called `assert!` with `Result::is_ok`
22-
--> $DIR/assertions_on_result_states.rs:50:5
22+
--> $DIR/assertions_on_result_states.rs:58:5
2323
|
2424
LL | assert!(r.is_ok());
2525
| ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()`
2626

2727
error: called `assert!` with `Result::is_ok`
28-
--> $DIR/assertions_on_result_states.rs:56:9
28+
--> $DIR/assertions_on_result_states.rs:64:9
2929
|
3030
LL | assert!(r.is_ok());
3131
| ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()`
3232

3333
error: called `assert!` with `Result::is_err`
34-
--> $DIR/assertions_on_result_states.rs:64:5
34+
--> $DIR/assertions_on_result_states.rs:72:5
3535
|
3636
LL | assert!(r.is_err());
3737
| ^^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap_err()`

0 commit comments

Comments
 (0)