]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/needless_question_mark.rs
Auto merge of #9546 - kraktus:default_not_default_trait, r=xFrednet
[rust.git] / clippy_lints / src / needless_question_mark.rs
index 7b156a8c49dd9d93aec449e7898ce3acb7c09dba..59b6492e112c05453c4dc24cdbf50d2c070d2ef7 100644 (file)
@@ -1,25 +1,21 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_lang_ctor;
 use clippy_utils::source::snippet;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{differing_macro_contexts, is_lang_ctor};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionSome, ResultOk};
-use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
+use rustc_hir::{AsyncGeneratorKind, Block, Body, Expr, ExprKind, GeneratorKind, LangItem, MatchSource, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::sym;
 
 declare_clippy_lint! {
-    /// **What it does:**
+    /// ### What it does
     /// Suggests alternatives for useless applications of `?` in terminating expressions
     ///
-    /// **Why is this bad?** There's no reason to use `?` to short-circuit when execution of the body will end there anyway.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
+    /// ### Why is this bad?
+    /// There's no reason to use `?` to short-circuit when execution of the body will end there anyway.
     ///
+    /// ### Example
     /// ```rust
     /// struct TO {
     ///     magic: Option<usize>,
@@ -56,6 +52,7 @@
     ///     tr.and_then(|t| t.magic)
     /// }
     /// ```
+    #[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>`."
 
 declare_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]);
 
-#[derive(Debug)]
-enum SomeOkCall<'a> {
-    SomeCall(&'a Expr<'a>, &'a Expr<'a>),
-    OkCall(&'a Expr<'a>, &'a Expr<'a>),
-}
-
 impl LateLintPass<'_> for NeedlessQuestionMark {
     /*
      * The question mark operator is compatible with both Result<T, E> and Option<T>,
@@ -90,104 +81,63 @@ impl LateLintPass<'_> for NeedlessQuestionMark {
      */
 
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
-        let e = match &expr.kind {
-            ExprKind::Ret(Some(e)) => e,
-            _ => return,
-        };
-
-        if let Some(ok_some_call) = is_some_or_ok_call(cx, e) {
-            emit_lint(cx, &ok_some_call);
+        if let ExprKind::Ret(Some(e)) = expr.kind {
+            check(cx, e);
         }
     }
 
     fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
-        // Function / Closure block
-        let expr_opt = if let ExprKind::Block(block, _) = &body.value.kind {
-            block.expr
-        } else {
-            // Single line closure
-            Some(&body.value)
-        };
-
-        if_chain! {
-            if let Some(expr) = expr_opt;
-            if let Some(ok_some_call) = is_some_or_ok_call(cx, expr);
-            then {
-                emit_lint(cx, &ok_some_call);
+        if let Some(GeneratorKind::Async(AsyncGeneratorKind::Fn)) = body.generator_kind {
+            if let ExprKind::Block(
+                Block {
+                    expr:
+                        Some(Expr {
+                            kind: ExprKind::DropTemps(async_body),
+                            ..
+                        }),
+                    ..
+                },
+                _,
+            ) = body.value.kind
+            {
+                if let ExprKind::Block(Block { expr: Some(expr), .. }, ..) = async_body.kind {
+                    check(cx, expr);
+                }
             }
-        };
+        } else {
+            check(cx, body.value.peel_blocks());
+        }
     }
 }
 
-fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) {
-    let (entire_expr, inner_expr) = match expr {
-        SomeOkCall::OkCall(outer, inner) | SomeOkCall::SomeCall(outer, inner) => (outer, inner),
-    };
-
-    span_lint_and_sugg(
-        cx,
-        NEEDLESS_QUESTION_MARK,
-        entire_expr.span,
-        "question mark operator is useless here",
-        "try",
-        format!("{}", snippet(cx, inner_expr.span, r#""...""#)),
-        Applicability::MachineApplicable,
-    );
-}
-
-fn is_some_or_ok_call<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<SomeOkCall<'a>> {
+fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
     if_chain! {
-        // Check outer expression matches CALL_IDENT(ARGUMENT) format
-        if let ExprKind::Call(path, args) = &expr.kind;
+        if let ExprKind::Call(path, [arg]) = &expr.kind;
         if let ExprKind::Path(ref qpath) = &path.kind;
-        if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk);
-
-        // Extract inner expression from ARGUMENT
-        if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &args[0].kind;
-        if let ExprKind::Call(called, args) = &inner_expr_with_q.kind;
-        if args.len() == 1;
-
-        if let ExprKind::Path(QPath::LangItem(LangItem::TryIntoResult, _)) = &called.kind;
+        let sugg_remove = if is_lang_ctor(cx, qpath, OptionSome) {
+            "Some()"
+        } else if is_lang_ctor(cx, qpath, ResultOk) {
+            "Ok()"
+        } else {
+            return;
+        };
+        if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &arg.kind;
+        if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind;
+        if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind;
+        if expr.span.ctxt() == inner_expr.span.ctxt();
+        let expr_ty = cx.typeck_results().expr_ty(expr);
+        let inner_ty = cx.typeck_results().expr_ty(inner_expr);
+        if expr_ty == inner_ty;
         then {
-            // Extract inner expr type from match argument generated by
-            // question mark operator
-            let inner_expr = &args[0];
-
-            // if the inner expr is inside macro but the outer one is not, do not lint (#6921)
-            if  differing_macro_contexts(expr.span, inner_expr.span) {
-                return None;
-            }
-
-            let inner_ty = cx.typeck_results().expr_ty(inner_expr);
-            let outer_ty = cx.typeck_results().expr_ty(expr);
-
-            // Check if outer and inner type are Option
-            let outer_is_some = is_type_diagnostic_item(cx, outer_ty, sym::option_type);
-            let inner_is_some = is_type_diagnostic_item(cx, inner_ty, sym::option_type);
-
-            // Check for Option MSRV
-            if outer_is_some && inner_is_some {
-                return Some(SomeOkCall::SomeCall(expr, inner_expr));
-            }
-
-            // Check if outer and inner type are Result
-            let outer_is_result = is_type_diagnostic_item(cx, outer_ty, sym::result_type);
-            let inner_is_result = is_type_diagnostic_item(cx, inner_ty, sym::result_type);
-
-            // Additional check: if the error type of the Result can be converted
-            // via the From trait, then don't match
-            let does_not_call_from = !has_implicit_error_from(cx, expr, inner_expr);
-
-            // Must meet Result MSRV
-            if outer_is_result && inner_is_result && does_not_call_from {
-                return Some(SomeOkCall::OkCall(expr, inner_expr));
-            }
+            span_lint_and_sugg(
+                cx,
+                NEEDLESS_QUESTION_MARK,
+                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,
+            );
         }
     }
-
-    None
-}
-
-fn has_implicit_error_from(cx: &LateContext<'_>, entire_expr: &Expr<'_>, inner_result_expr: &Expr<'_>) -> bool {
-    return cx.typeck_results().expr_ty(entire_expr) != cx.typeck_results().expr_ty(inner_result_expr);
 }