Skip to content

Commit 2a6197e

Browse files
authored
Recognize canonical ? pattern with Result (#15680)
This recognizes the following expression as being equivalent to the question mark: ```rust match x { Ok(v) => v, Err(e) => return Err(e.into()), // or `Into::into(x)` or `<T as Into<U>>::into(x)` } ``` Fixes #15679 changelog: [`question_mark`]: recognizes a match with an early return with a call to `.into()` as possibly equivalent to a question mark
2 parents 7a12684 + c4da39c commit 2a6197e

File tree

4 files changed

+113
-40
lines changed

4 files changed

+113
-40
lines changed

clippy_lints/src/question_mark.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ use clippy_utils::source::snippet_with_applicability;
88
use clippy_utils::sugg::Sugg;
99
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
1010
use clippy_utils::{
11-
eq_expr_value, higher, is_else_clause, is_in_const_context, is_lint_allowed, is_path_lang_item, is_res_lang_ctor,
12-
pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt,
13-
span_contains_cfg, span_contains_comment, sym,
11+
eq_expr_value, fn_def_id_with_node_args, higher, is_else_clause, is_in_const_context, is_lint_allowed,
12+
is_path_lang_item, is_res_lang_ctor, pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id,
13+
peel_blocks, peel_blocks_with_stmt, span_contains_cfg, span_contains_comment, sym,
1414
};
1515
use rustc_errors::Applicability;
1616
use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk};
@@ -393,8 +393,8 @@ fn check_arm_is_none_or_err<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm: &A
393393
&& let ExprKind::Ret(Some(wrapped_ret_expr)) = arm_body.kind
394394
&& let ExprKind::Call(ok_ctor, [ret_expr]) = wrapped_ret_expr.kind
395395
&& is_res_lang_ctor(cx, path_res(cx, ok_ctor), ResultErr)
396-
// check `...` is `val` from binding
397-
&& path_to_local_id(ret_expr, ok_val)
396+
// check if `...` is `val` from binding or `val.into()`
397+
&& is_local_or_local_into(cx, ret_expr, ok_val)
398398
{
399399
true
400400
} else {
@@ -417,6 +417,17 @@ fn check_arm_is_none_or_err<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm: &A
417417
}
418418
}
419419

420+
/// Check if `expr` is `val` or `val.into()`
421+
fn is_local_or_local_into(cx: &LateContext<'_>, expr: &Expr<'_>, val: HirId) -> bool {
422+
let is_into_call = fn_def_id_with_node_args(cx, expr)
423+
.and_then(|(fn_def_id, _)| cx.tcx.trait_of_assoc(fn_def_id))
424+
.is_some_and(|trait_def_id| cx.tcx.is_diagnostic_item(sym::Into, trait_def_id));
425+
match expr.kind {
426+
ExprKind::MethodCall(_, recv, [], _) | ExprKind::Call(_, [recv]) => is_into_call && path_to_local_id(recv, val),
427+
_ => path_to_local_id(expr, val),
428+
}
429+
}
430+
420431
fn check_arms_are_try<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm1: &Arm<'tcx>, arm2: &Arm<'tcx>) -> bool {
421432
(check_arm_is_some_or_ok(cx, mode, arm1) && check_arm_is_none_or_err(cx, mode, arm2))
422433
|| (check_arm_is_some_or_ok(cx, mode, arm2) && check_arm_is_none_or_err(cx, mode, arm1))

tests/ui/question_mark.fixed

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
#![feature(try_blocks)]
2-
#![allow(unreachable_code)]
3-
#![allow(dead_code)]
42
#![allow(clippy::unnecessary_wraps)]
53

64
use std::sync::MutexGuard;
@@ -465,3 +463,15 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
465463

466464
None
467465
}
466+
467+
fn issue_15679() -> Result<i32, String> {
468+
let some_result: Result<i32, &'static str> = todo!();
469+
470+
some_result?;
471+
472+
some_result?;
473+
474+
some_result?;
475+
476+
Ok(0)
477+
}

tests/ui/question_mark.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
#![feature(try_blocks)]
2-
#![allow(unreachable_code)]
3-
#![allow(dead_code)]
42
#![allow(clippy::unnecessary_wraps)]
53

64
use std::sync::MutexGuard;
@@ -561,3 +559,27 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
561559

562560
None
563561
}
562+
563+
fn issue_15679() -> Result<i32, String> {
564+
let some_result: Result<i32, &'static str> = todo!();
565+
566+
match some_result {
567+
//~^ question_mark
568+
Ok(val) => val,
569+
Err(err) => return Err(err.into()),
570+
};
571+
572+
match some_result {
573+
//~^ question_mark
574+
Ok(val) => val,
575+
Err(err) => return Err(Into::into(err)),
576+
};
577+
578+
match some_result {
579+
//~^ question_mark
580+
Ok(val) => val,
581+
Err(err) => return Err(<&str as Into<String>>::into(err)),
582+
};
583+
584+
Ok(0)
585+
}

0 commit comments

Comments
 (0)