Reword needless_question_mark diagnostics and docs

clippy_lints/src/needless_question_mark.rs

@@ -1,6 +1,5 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::path_res;
-use clippy_utils::source::snippet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{Block, Body, Expr, ExprKind, LangItem, MatchSource, QPath};
@@ -9,52 +8,38 @@ use rustc_session::declare_lint_pass;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Suggests alternatives for useless applications of `?` in terminating expressions
+    /// Suggests replacing `Ok(x?)` or `Some(x?)` with `x` in return positions where the `?` operator
+    /// is not needed to convert the type of `x`.
     ///
     /// ### Why is this bad?
     /// There's no reason to use `?` to short-circuit when execution of the body will end there anyway.
     ///
     /// ### Example
     /// ```no_run
-    /// struct TO {
-    ///     magic: Option<usize>,
+    /// # use std::num::ParseIntError;
+    /// fn f(s: &str) -> Option<usize> {
+    ///     Some(s.find('x')?)
     /// }
     ///
-    /// fn f(to: TO) -> Option<usize> {
-    ///     Some(to.magic?)
+    /// fn g(s: &str) -> Result<usize, ParseIntError> {
+    ///     Ok(s.parse()?)
     /// }
-    ///
-    /// struct TR {
-    ///     magic: Result<usize, bool>,
-    /// }
-    ///
-    /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
-    ///     tr.and_then(|t| Ok(t.magic?))
-    /// }
-    ///
     /// ```
     /// Use instead:
     /// ```no_run
-    /// struct TO {
-    ///     magic: Option<usize>,
+    /// # use std::num::ParseIntError;
+    /// fn f(s: &str) -> Option<usize> {
+    ///     s.find('x')
     /// }
     ///
-    /// fn f(to: TO) -> Option<usize> {
-    ///     to.magic
-    /// }
-    ///
-    /// struct TR {
-    ///     magic: Result<usize, bool>,
-    /// }
-    ///
-    /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
-    ///     tr.and_then(|t| t.magic)
+    /// fn g(s: &str) -> Result<usize, ParseIntError> {
+    ///     s.parse()
     /// }
     /// ```
     #[clippy::version = "1.51.0"]
     pub NEEDLESS_QUESTION_MARK,
     complexity,
-    "Suggest `value.inner_option` instead of `Some(value.inner_option?)`. The same goes for `Result<T, E>`."
+    "using `Ok(x?)` or `Some(x?)` where `x` would be equivalent"
 }
 
 declare_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]);
@@ -111,10 +96,10 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
     if let ExprKind::Call(path, [arg]) = expr.kind
         && let Res::Def(DefKind::Ctor(..), ctor_id) = path_res(cx, path)
         && let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
-        && let sugg_remove = if cx.tcx.lang_items().option_some_variant() == Some(variant_id) {
-            "Some()"
+        && let variant = if cx.tcx.lang_items().option_some_variant() == Some(variant_id) {
+            "Some"
         } else if cx.tcx.lang_items().result_ok_variant() == Some(variant_id) {
-            "Ok()"
+            "Ok"
         } else {
             return;
         }
@@ -126,14 +111,25 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
         && let inner_ty = cx.typeck_results().expr_ty(inner_expr)
         && expr_ty == inner_ty
     {
-        span_lint_and_sugg(
+        span_lint_hir_and_then(
             cx,
             NEEDLESS_QUESTION_MARK,
+            expr.hir_id,
             expr.span,
-            "question mark operator is useless here",
-            format!("try removing question mark and `{sugg_remove}`"),
-            format!("{}", snippet(cx, inner_expr.span, r#""...""#)),
-            Applicability::MachineApplicable,
+            format!("enclosing `{variant}` and `?` operator are unneeded"),
+            |diag| {
+                diag.multipart_suggestion(
+                    format!("remove the enclosing `{variant}` and `?` operator"),
+                    vec![
+                        (expr.span.until(inner_expr.span), String::new()),
+                        (
+                            inner_expr.span.shrink_to_hi().to(expr.span.shrink_to_hi()),
+                            String::new(),
+                        ),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            },
         );
     }
 }

tests/ui/needless_question_mark.stderr

@@ -1,100 +1,188 @@
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:20:12
    |
 LL |     return Some(to.magic?);
-   |            ^^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `to.magic`
+   |            ^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::needless-question-mark` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_question_mark)]`
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     return Some(to.magic?);
+LL +     return to.magic;
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:29:12
    |
 LL |     return Some(to.magic?)
-   |            ^^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `to.magic`
+   |            ^^^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     return Some(to.magic?)
+LL +     return to.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:35:5
    |
 LL |     Some(to.magic?)
-   |     ^^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `to.magic`
+   |     ^^^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     Some(to.magic?)
+LL +     to.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:41:21
    |
 LL |     to.and_then(|t| Some(t.magic?))
-   |                     ^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `t.magic`
+   |                     ^^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     to.and_then(|t| Some(t.magic?))
+LL +     to.and_then(|t| t.magic)
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:51:9
    |
 LL |         Some(t.magic?)
-   |         ^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `t.magic`
+   |         ^^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -         Some(t.magic?)
+LL +         t.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:57:12
    |
 LL |     return Ok(tr.magic?);
-   |            ^^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `tr.magic`
+   |            ^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -     return Ok(tr.magic?);
+LL +     return tr.magic;
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:65:12
    |
 LL |     return Ok(tr.magic?)
-   |            ^^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `tr.magic`
+   |            ^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -     return Ok(tr.magic?)
+LL +     return tr.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:70:5
    |
 LL |     Ok(tr.magic?)
-   |     ^^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `tr.magic`
+   |     ^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -     Ok(tr.magic?)
+LL +     tr.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:75:21
    |
 LL |     tr.and_then(|t| Ok(t.magic?))
-   |                     ^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `t.magic`
+   |                     ^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -     tr.and_then(|t| Ok(t.magic?))
+LL +     tr.and_then(|t| t.magic)
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:84:9
    |
 LL |         Ok(t.magic?)
-   |         ^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `t.magic`
+   |         ^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -         Ok(t.magic?)
+LL +         t.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:92:16
    |
 LL |         return Ok(t.magic?);
-   |                ^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `t.magic`
+   |                ^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -         return Ok(t.magic?);
+LL +         return t.magic;
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:128:27
    |
 LL |         || -> Option<_> { Some(Some($expr)?) }()
-   |                           ^^^^^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `Some($expr)`
+   |                           ^^^^^^^^^^^^^^^^^^
 ...
 LL |     let _x = some_and_qmark_in_macro!(x?);
    |              ---------------------------- in this macro invocation
    |
    = note: this error originates in the macro `some_and_qmark_in_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -         || -> Option<_> { Some(Some($expr)?) }()
+LL +         || -> Option<_> { Some($expr) }()
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:140:5
    |
 LL |     Some(to.magic?)
-   |     ^^^^^^^^^^^^^^^ help: try removing question mark and `Some()`: `to.magic`
+   |     ^^^^^^^^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     Some(to.magic?)
+LL +     to.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Ok` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:149:5
    |
 LL |     Ok(s.magic?)
-   |     ^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `s.magic`
+   |     ^^^^^^^^^^^^
+   |
+help: remove the enclosing `Ok` and `?` operator
+   |
+LL -     Ok(s.magic?)
+LL +     s.magic
+   |
 
-error: question mark operator is useless here
+error: enclosing `Some` and `?` operator are unneeded
   --> tests/ui/needless_question_mark.rs:154:7
    |
 LL |     { Some(a?) }
-   |       ^^^^^^^^ help: try removing question mark and `Some()`: `a`
+   |       ^^^^^^^^
+   |
+help: remove the enclosing `Some` and `?` operator
+   |
+LL -     { Some(a?) }
+LL +     { a }
+   |
 
 error: aborting due to 15 previous errors
 

tests/ui/question_mark.fixed

@@ -301,6 +301,11 @@ fn pattern() -> Result<(), PatternedError> {
     res
 }
 
+fn expect_expr(a: Option<usize>) -> Option<usize> {
+    #[expect(clippy::needless_question_mark)]
+    Some(a?)
+}
+
 fn main() {}
 
 // `?` is not the same as `return None;` if inside of a try block

tests/ui/question_mark.rs

@@ -371,6 +371,11 @@ fn pattern() -> Result<(), PatternedError> {
     res
 }
 
+fn expect_expr(a: Option<usize>) -> Option<usize> {
+    #[expect(clippy::needless_question_mark)]
+    Some(a?)
+}
+
 fn main() {}
 
 // `?` is not the same as `return None;` if inside of a try block