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>,
/// 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>,
*/
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);
}