fix: assertions_on_result_states avoid changing return type in more cases (#15591)

Fixes #9916

changelog: [`assertions_on_result_states`]: avoid changing return type
in more cases

Author: samueltardieu

clippy_lints/src/assertions_on_result_states.rs

@@ -3,10 +3,10 @@ use clippy_utils::macros::{PanicExpn, find_assert_args, root_macro_call_first_no
 use clippy_utils::source::snippet_with_context;
 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, sym};
+use clippy_utils::{path_res, sym};
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{Expr, ExprKind, Node};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, Ty};
 use rustc_session::declare_lint_pass;
@@ -77,17 +77,20 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
                 _ => return,
             };
             span_lint_and_then(cx, ASSERTIONS_ON_RESULT_STATES, macro_call.span, message, |diag| {
-                let semicolon = if is_expr_final_block_expr(cx.tcx, e) { ";" } else { "" };
                 let mut app = Applicability::MachineApplicable;
-                diag.span_suggestion(
-                    macro_call.span,
-                    "replace with",
-                    format!(
-                        "{}.{replacement}(){semicolon}",
-                        snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
-                    ),
-                    app,
-                );
+                let recv = snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0;
+
+                // `assert!` doesn't return anything, but `Result::unwrap(_err)` does, so we might need to add a
+                // semicolon to the suggestion to avoid leaking the type
+                let sugg = match cx.tcx.parent_hir_node(e.hir_id) {
+                    // trailing expr of a block
+                    Node::Block(..) => format!("{recv}.{replacement}();"),
+                    // already has a trailing semicolon
+                    Node::Stmt(..) => format!("{recv}.{replacement}()"),
+                    // this is the last-resort option, because it's rather verbose
+                    _ => format!("{{ {recv}.{replacement}(); }}"),
+                };
+                diag.span_suggestion(macro_call.span, "replace with", sugg, app);
             });
         }
     }

tests/ui/assertions_on_result_states.fixed

@@ -82,9 +82,18 @@ fn main() {
     assert!(r.is_err());
 }
 
-#[allow(dead_code)]
 fn issue9450() {
     let res: Result<i32, i32> = Ok(1);
     res.unwrap_err();
     //~^ assertions_on_result_states
 }
+
+fn issue9916(res: Result<u32, u32>) {
+    let a = 0;
+    match a {
+        0 => {},
+        1 => { res.unwrap(); },
+        //~^ assertions_on_result_states
+        _ => todo!(),
+    }
+}

tests/ui/assertions_on_result_states.rs

@@ -82,9 +82,18 @@ fn main() {
     assert!(r.is_err());
 }
 
-#[allow(dead_code)]
 fn issue9450() {
     let res: Result<i32, i32> = Ok(1);
     assert!(res.is_err())
     //~^ assertions_on_result_states
 }
+
+fn issue9916(res: Result<u32, u32>) {
+    let a = 0;
+    match a {
+        0 => {},
+        1 => assert!(res.is_ok()),
+        //~^ assertions_on_result_states
+        _ => todo!(),
+    }
+}

tests/ui/assertions_on_result_states.stderr

@@ -38,10 +38,16 @@ LL |     assert!(r.is_err());
    |     ^^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap_err()`
 
 error: called `assert!` with `Result::is_err`
-  --> tests/ui/assertions_on_result_states.rs:88:5
+  --> tests/ui/assertions_on_result_states.rs:87:5
    |
 LL |     assert!(res.is_err())
    |     ^^^^^^^^^^^^^^^^^^^^^ help: replace with: `res.unwrap_err();`
 
-error: aborting due to 7 previous errors
+error: called `assert!` with `Result::is_ok`
+  --> tests/ui/assertions_on_result_states.rs:95:14
+   |
+LL |         1 => assert!(res.is_ok()),
+   |              ^^^^^^^^^^^^^^^^^^^^ help: replace with: `{ res.unwrap(); }`
+
+error: aborting due to 8 previous errors