Auto merge of #9453 - kraktus:a_on_re_state, r=Jarcho

[`assertions_on_result_states`]: Fix suggestion when `assert!` is not in a statement.

fix rust-lang/rust-clippy#9450

changelog: [`assertions_on_result_states`]: Fix suggestion when `assert!` is not in a statement.

Author: bors

clippy_lints/src/assertions_on_result_states.rs

@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn};
-use clippy_utils::path_res;
 use clippy_utils::source::snippet_with_context;
-use clippy_utils::ty::{implements_trait, is_copy, is_type_diagnostic_item};
+use clippy_utils::ty::{has_debug_impl, is_copy, is_type_diagnostic_item};
 use clippy_utils::usage::local_used_after_expr;
+use clippy_utils::{is_expr_final_block_expr, path_res};
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::{Expr, ExprKind};
@@ -58,6 +58,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
                     return;
                 }
             }
+            let semicolon = if is_expr_final_block_expr(cx.tcx, e) {";"} else {""};
             let mut app = Applicability::MachineApplicable;
             match method_segment.ident.as_str() {
                 "is_ok" if type_suitable_to_unwrap(cx, substs.type_at(1)) => {
@@ -68,8 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
                         "called `assert!` with `Result::is_ok`",
                         "replace with",
                         format!(
-                            "{}.unwrap()",
-                            snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
+                            "{}.unwrap(){}",
+                            snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0,
+                            semicolon
                         ),
                         app,
                     );
@@ -82,8 +84,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
                         "called `assert!` with `Result::is_err`",
                         "replace with",
                         format!(
-                            "{}.unwrap_err()",
-                            snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
+                            "{}.unwrap_err(){}",
+                            snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0,
+                            semicolon
                         ),
                         app,
                     );
@@ -94,13 +97,6 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
     }
 }
 
-/// This checks whether a given type is known to implement Debug.
-fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
-    cx.tcx
-        .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()
 }

clippy_lints/src/methods/err_expect.rs

@@ -1,6 +1,6 @@
 use super::ERR_EXPECT;
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::ty::implements_trait;
+use clippy_utils::ty::has_debug_impl;
 use clippy_utils::{meets_msrv, msrvs, ty::is_type_diagnostic_item};
 use rustc_errors::Applicability;
 use rustc_lint::LateContext;
@@ -28,7 +28,7 @@ pub(super) fn check(
         // Tests if the T type in a `Result<T, E>` is not None
         if let Some(data_type) = get_data_type(cx, result_type);
         // Tests if the T type in a `Result<T, E>` implements debug
-        if has_debug_impl(data_type, cx);
+        if has_debug_impl(cx, data_type);
 
         then {
             span_lint_and_sugg(
@@ -51,10 +51,3 @@ fn get_data_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> {
         _ => None,
     }
 }
-
-/// Given a type, very if the Debug trait has been impl'd
-fn has_debug_impl<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
-    cx.tcx
-        .get_diagnostic_item(sym::Debug)
-        .map_or(false, |debug| implements_trait(cx, ty, debug, &[]))
-}

clippy_lints/src/methods/ok_expect.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
+use clippy_utils::ty::{has_debug_impl, is_type_diagnostic_item};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
@@ -15,7 +15,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
         if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
         let result_type = cx.typeck_results().expr_ty(recv);
         if let Some(error_type) = get_error_type(cx, result_type);
-        if has_debug_impl(error_type, cx);
+        if has_debug_impl(cx, error_type);
 
         then {
             span_lint_and_help(
@@ -37,10 +37,3 @@ fn get_error_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> {
         _ => None,
     }
 }
-
-/// This checks whether a given type is known to implement Debug.
-fn has_debug_impl<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
-    cx.tcx
-        .get_diagnostic_item(sym::Debug)
-        .map_or(false, |debug| implements_trait(cx, ty, debug, &[]))
-}

clippy_utils/src/ty.rs

@@ -31,6 +31,13 @@ pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
     ty.is_copy_modulo_regions(cx.tcx.at(DUMMY_SP), cx.param_env)
 }
 
+/// This checks whether a given type is known to implement Debug.
+pub fn has_debug_impl<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+    cx.tcx
+        .get_diagnostic_item(sym::Debug)
+        .map_or(false, |debug| implements_trait(cx, ty, debug, &[]))
+}
+
 /// Checks whether a type can be partially moved.
 pub fn can_partially_move_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
     if has_drop(cx, ty) || is_copy(cx, ty) {

tests/ui/assertions_on_result_states.fixed

@@ -75,3 +75,9 @@ fn main() {
     let r: Result<Foo, Foo> = Err(Foo);
     assert!(r.is_err());
 }
+
+#[allow(dead_code)]
+fn issue9450() {
+    let res: Result<i32, i32> = Ok(1);
+    res.unwrap_err();
+}

tests/ui/assertions_on_result_states.rs

@@ -75,3 +75,9 @@ fn main() {
     let r: Result<Foo, Foo> = Err(Foo);
     assert!(r.is_err());
 }
+
+#[allow(dead_code)]
+fn issue9450() {
+    let res: Result<i32, i32> = Ok(1);
+    assert!(res.is_err())
+}

tests/ui/assertions_on_result_states.stderr

@@ -36,5 +36,11 @@ error: called `assert!` with `Result::is_err`
 LL |     assert!(r.is_err());
    |     ^^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap_err()`
 
-error: aborting due to 6 previous errors
+error: called `assert!` with `Result::is_err`
+  --> $DIR/assertions_on_result_states.rs:82:5
+   |
+LL |     assert!(res.is_err())
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace with: `res.unwrap_err();`
+
+error: aborting due to 7 previous errors