Skip to content

Commit f202d28

Browse files
committed
Recognize canonical ? pattern with Result
1 parent 7a12684 commit f202d28

File tree

4 files changed

+115
-36
lines changed

4 files changed

+115
-36
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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(unreachable_code)]
33
#![allow(dead_code)]
44
#![allow(clippy::unnecessary_wraps)]
5+
#![allow(clippy::useless_conversion)]
56

67
use std::sync::MutexGuard;
78

@@ -465,3 +466,15 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
465466

466467
None
467468
}
469+
470+
fn issue_15679() -> Result<i32, String> {
471+
let some_result: Result<i32, &'static str> = todo!();
472+
473+
some_result?;
474+
475+
some_result?;
476+
477+
some_result?;
478+
479+
Ok(0)
480+
}

tests/ui/question_mark.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(unreachable_code)]
33
#![allow(dead_code)]
44
#![allow(clippy::unnecessary_wraps)]
5+
#![allow(clippy::useless_conversion)]
56

67
use std::sync::MutexGuard;
78

@@ -561,3 +562,27 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
561562

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

0 commit comments

Comments
 (0)