Auto merge of #12150 - ithinuel:add_misleading_use_of_ok, r=y21

Add lint for `unused_result_ok`

This PR adds a lint to capture the use of `expr.ok();` when the result is not _really_ used.

This could be interpreted as the result being checked (like it is with `unwrap()` or `expect`) but
it actually only ignores the result.

`let _ = expr;` expresses that intent better.

This was also mentionned in #8994 (although not being the main topic of that issue).

changelog: [`misleading_use_of_ok`]: Add new lint to capture `.ok();` when the result is not _really_ used.

3 files changed, 62 insertions, 0 deletions

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 69f7ca57554..3fb083dd833 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -739,6 +739,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::unused_async::UNUSED_ASYNC_INFO,
     crate::unused_io_amount::UNUSED_IO_AMOUNT_INFO,
     crate::unused_peekable::UNUSED_PEEKABLE_INFO,
+    crate::unused_result_ok::UNUSED_RESULT_OK_INFO,
     crate::unused_rounding::UNUSED_ROUNDING_INFO,
     crate::unused_self::UNUSED_SELF_INFO,
     crate::unused_unit::UNUSED_UNIT_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index a0d77a58ad1..374fcd29783 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -370,6 +370,7 @@ mod unsafe_removed_from_name;
 mod unused_async;
 mod unused_io_amount;
 mod unused_peekable;
+mod unused_result_ok;
 mod unused_rounding;
 mod unused_self;
 mod unused_unit;
@@ -671,6 +672,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(conf)));
     store.register_late_pass(|_| Box::new(missing_inline::MissingInline));
     store.register_late_pass(move |_| Box::new(exhaustive_items::ExhaustiveItems));
+    store.register_late_pass(|_| Box::new(unused_result_ok::UnusedResultOk));
     store.register_late_pass(|_| Box::new(match_result_ok::MatchResultOk));
     store.register_late_pass(|_| Box::new(partialeq_ne_impl::PartialEqNeImpl));
     store.register_late_pass(|_| Box::new(unused_io_amount::UnusedIoAmount));
diff --git a/clippy_lints/src/unused_result_ok.rs b/clippy_lints/src/unused_result_ok.rs
new file mode 100644
index 00000000000..297288db0a5
--- /dev/null
+++ b/clippy_lints/src/unused_result_ok.rs
@@ -0,0 +1,59 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_hir::{ExprKind, Stmt, StmtKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for calls to `Result::ok()` without using the returned `Option`.
+    ///
+    /// ### Why is this bad?
+    /// Using `Result::ok()` may look like the result is checked like `unwrap` or `expect` would do
+    /// but it only silences the warning caused by `#[must_use]` on the `Result`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # fn some_function() -> Result<(), ()> { Ok(()) }
+    /// some_function().ok();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # fn some_function() -> Result<(), ()> { Ok(()) }
+    /// let _ = some_function();
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub UNUSED_RESULT_OK,
+    restriction,
+    "Use of `.ok()` to silence `Result`'s `#[must_use]` is misleading. Use `let _ =` instead."
+}
+declare_lint_pass!(UnusedResultOk => [UNUSED_RESULT_OK]);
+
+impl LateLintPass<'_> for UnusedResultOk {
+    fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
+        if let StmtKind::Semi(expr) = stmt.kind
+            && let ExprKind::MethodCall(ok_path, recv, [], ..) = expr.kind //check is expr.ok() has type Result<T,E>.ok(, _)
+            && ok_path.ident.as_str() == "ok"
+            && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result)
+            && !in_external_macro(cx.sess(), stmt.span)
+        {
+            let ctxt = expr.span.ctxt();
+            let mut applicability = Applicability::MaybeIncorrect;
+            let snippet = snippet_with_context(cx, recv.span, ctxt, "", &mut applicability).0;
+            let sugg = format!("let _ = {snippet}");
+            span_lint_and_sugg(
+                cx,
+                UNUSED_RESULT_OK,
+                expr.span,
+                "ignoring a result with `.ok()` is misleading",
+                "consider using `let _ =` and removing the call to `.ok()` instead",
+                sugg,
+                applicability,
+            );
+        }
+    }
+}