diff --git a/clippy_lints/src/assertions_on_result_states.rs b/clippy_lints/src/assertions_on_result_states.rs index 4caab623090..6a6554f968b 100644 --- a/clippy_lints/src/assertions_on_result_states.rs +++ b/clippy_lints/src/assertions_on_result_states.rs @@ -53,13 +53,14 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if result_type_with_refs != result_type { return; } else if let Res::Local(binding_id) = path_res(cx, recv) - && local_used_after_expr(cx, binding_id, recv) { + && local_used_after_expr(cx, binding_id, recv) + { return; } } let mut app = Applicability::MachineApplicable; match method_segment.ident.as_str() { - "is_ok" if has_debug_impl(cx, substs.type_at(1)) => { + "is_ok" if type_suitable_to_unwrap(cx, substs.type_at(1)) => { span_lint_and_sugg( cx, ASSERTIONS_ON_RESULT_STATES, @@ -73,7 +74,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { app, ); } - "is_err" if has_debug_impl(cx, substs.type_at(0)) => { + "is_err" if type_suitable_to_unwrap(cx, substs.type_at(0)) => { span_lint_and_sugg( cx, ASSERTIONS_ON_RESULT_STATES, @@ -99,3 +100,7 @@ fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { .get_diagnostic_item(sym::Debug) .map_or(false, |debug| implements_trait(cx, ty, debug, &[])) } + +fn type_suitable_to_unwrap<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + has_debug_impl(cx, ty) && !ty.is_unit() && !ty.is_never() +} diff --git a/tests/ui/assertions_on_result_states.fixed b/tests/ui/assertions_on_result_states.fixed index 7bde72e4b6b..795f435f24c 100644 --- a/tests/ui/assertions_on_result_states.fixed +++ b/tests/ui/assertions_on_result_states.fixed @@ -27,6 +27,14 @@ fn main() { let r: Result = Ok(Foo); assert!(r.is_ok()); + // test ok with some messages + let r: Result = Ok(Foo); + assert!(r.is_ok(), "oops"); + + // test ok with unit error + let r: Result = Ok(Foo); + assert!(r.is_ok()); + // test temporary ok fn get_ok() -> Result { Ok(Foo) diff --git a/tests/ui/assertions_on_result_states.rs b/tests/ui/assertions_on_result_states.rs index 4c5af81efc2..1101aec1e1b 100644 --- a/tests/ui/assertions_on_result_states.rs +++ b/tests/ui/assertions_on_result_states.rs @@ -27,6 +27,14 @@ fn main() { let r: Result = Ok(Foo); assert!(r.is_ok()); + // test ok with some messages + let r: Result = Ok(Foo); + assert!(r.is_ok(), "oops"); + + // test ok with unit error + let r: Result = Ok(Foo); + assert!(r.is_ok()); + // test temporary ok fn get_ok() -> Result { Ok(Foo) diff --git a/tests/ui/assertions_on_result_states.stderr b/tests/ui/assertions_on_result_states.stderr index 13c2dd877a9..97a5f3dfca4 100644 --- a/tests/ui/assertions_on_result_states.stderr +++ b/tests/ui/assertions_on_result_states.stderr @@ -7,31 +7,31 @@ LL | assert!(r.is_ok()); = note: `-D clippy::assertions-on-result-states` implied by `-D warnings` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:34:5 + --> $DIR/assertions_on_result_states.rs:42:5 | LL | assert!(get_ok().is_ok()); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok().unwrap()` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:37:5 + --> $DIR/assertions_on_result_states.rs:45:5 | LL | assert!(get_ok_macro!().is_ok()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok_macro!().unwrap()` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:50:5 + --> $DIR/assertions_on_result_states.rs:58:5 | LL | assert!(r.is_ok()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:56:9 + --> $DIR/assertions_on_result_states.rs:64:9 | LL | assert!(r.is_ok()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()` error: called `assert!` with `Result::is_err` - --> $DIR/assertions_on_result_states.rs:64:5 + --> $DIR/assertions_on_result_states.rs:72:5 | LL | assert!(r.is_err()); | ^^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap_err()`