mirror of
https://github.com/rust-lang/rust.git
synced 2025-10-02 10:18:25 +00:00
Recognize canonical ? pattern with Result
This commit is contained in:
Samuel Tardieu
parent
7a12684741
commit
c4da39cca3
@ -8,9 +8,9 @@ use clippy_utils::source::snippet_with_applicability;
|
||||
use clippy_utils::sugg::Sugg;
|
||||
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
|
||||
use clippy_utils::{
|
||||
eq_expr_value, higher, is_else_clause, is_in_const_context, is_lint_allowed, is_path_lang_item, is_res_lang_ctor,
|
||||
pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt,
|
||||
span_contains_cfg, span_contains_comment, sym,
|
||||
eq_expr_value, fn_def_id_with_node_args, higher, is_else_clause, is_in_const_context, is_lint_allowed,
|
||||
is_path_lang_item, is_res_lang_ctor, pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id,
|
||||
peel_blocks, peel_blocks_with_stmt, span_contains_cfg, span_contains_comment, sym,
|
||||
};
|
||||
use rustc_errors::Applicability;
|
||||
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
|
||||
&& let ExprKind::Ret(Some(wrapped_ret_expr)) = arm_body.kind
|
||||
&& let ExprKind::Call(ok_ctor, [ret_expr]) = wrapped_ret_expr.kind
|
||||
&& is_res_lang_ctor(cx, path_res(cx, ok_ctor), ResultErr)
|
||||
// check `...` is `val` from binding
|
||||
&& path_to_local_id(ret_expr, ok_val)
|
||||
// check if `...` is `val` from binding or `val.into()`
|
||||
&& is_local_or_local_into(cx, ret_expr, ok_val)
|
||||
{
|
||||
true
|
||||
} else {
|
||||
@ -417,6 +417,17 @@ fn check_arm_is_none_or_err<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm: &A
|
||||
}
|
||||
}
|
||||
|
||||
/// Check if `expr` is `val` or `val.into()`
|
||||
fn is_local_or_local_into(cx: &LateContext<'_>, expr: &Expr<'_>, val: HirId) -> bool {
|
||||
let is_into_call = fn_def_id_with_node_args(cx, expr)
|
||||
.and_then(|(fn_def_id, _)| cx.tcx.trait_of_assoc(fn_def_id))
|
||||
.is_some_and(|trait_def_id| cx.tcx.is_diagnostic_item(sym::Into, trait_def_id));
|
||||
match expr.kind {
|
||||
ExprKind::MethodCall(_, recv, [], _) | ExprKind::Call(_, [recv]) => is_into_call && path_to_local_id(recv, val),
|
||||
_ => path_to_local_id(expr, val),
|
||||
}
|
||||
}
|
||||
|
||||
fn check_arms_are_try<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm1: &Arm<'tcx>, arm2: &Arm<'tcx>) -> bool {
|
||||
(check_arm_is_some_or_ok(cx, mode, arm1) && check_arm_is_none_or_err(cx, mode, arm2))
|
||||
|| (check_arm_is_some_or_ok(cx, mode, arm2) && check_arm_is_none_or_err(cx, mode, arm1))
|
||||
|
||||
@ -1,6 +1,4 @@
|
||||
#![feature(try_blocks)]
|
||||
#![allow(unreachable_code)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(clippy::unnecessary_wraps)]
|
||||
|
||||
use std::sync::MutexGuard;
|
||||
@ -465,3 +463,15 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn issue_15679() -> Result<i32, String> {
|
||||
let some_result: Result<i32, &'static str> = todo!();
|
||||
|
||||
some_result?;
|
||||
|
||||
some_result?;
|
||||
|
||||
some_result?;
|
||||
|
||||
Ok(0)
|
||||
}
|
||||
|
||||
@ -1,6 +1,4 @@
|
||||
#![feature(try_blocks)]
|
||||
#![allow(unreachable_code)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(clippy::unnecessary_wraps)]
|
||||
|
||||
use std::sync::MutexGuard;
|
||||
@ -561,3 +559,27 @@ fn issue_13642(x: Option<i32>) -> Option<()> {
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn issue_15679() -> Result<i32, String> {
|
||||
let some_result: Result<i32, &'static str> = todo!();
|
||||
|
||||
match some_result {
|
||||
//~^ question_mark
|
||||
Ok(val) => val,
|
||||
Err(err) => return Err(err.into()),
|
||||
};
|
||||
|
||||
match some_result {
|
||||
//~^ question_mark
|
||||
Ok(val) => val,
|
||||
Err(err) => return Err(Into::into(err)),
|
||||
};
|
||||
|
||||
match some_result {
|
||||
//~^ question_mark
|
||||
Ok(val) => val,
|
||||
Err(err) => return Err(<&str as Into<String>>::into(err)),
|
||||
};
|
||||
|
||||
Ok(0)
|
||||
}
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:9:5
|
||||
--> tests/ui/question_mark.rs:7:5
|
||||
|
|
||||
LL | / if a.is_none() {
|
||||
LL | |
|
||||
@ -11,7 +11,7 @@ LL | | }
|
||||
= help: to override `-D warnings` add `#[allow(clippy::question_mark)]`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:55:9
|
||||
--> tests/ui/question_mark.rs:53:9
|
||||
|
|
||||
LL | / if (self.opt).is_none() {
|
||||
LL | |
|
||||
@ -20,7 +20,7 @@ LL | | }
|
||||
| |_________^ help: replace it with: `(self.opt)?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:60:9
|
||||
--> tests/ui/question_mark.rs:58:9
|
||||
|
|
||||
LL | / if self.opt.is_none() {
|
||||
LL | |
|
||||
@ -29,7 +29,7 @@ LL | | }
|
||||
| |_________^ help: replace it with: `self.opt?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:65:17
|
||||
--> tests/ui/question_mark.rs:63:17
|
||||
|
|
||||
LL | let _ = if self.opt.is_none() {
|
||||
| _________________^
|
||||
@ -41,7 +41,7 @@ LL | | };
|
||||
| |_________^ help: replace it with: `Some(self.opt?)`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:72:17
|
||||
--> tests/ui/question_mark.rs:70:17
|
||||
|
|
||||
LL | let _ = if let Some(x) = self.opt {
|
||||
| _________________^
|
||||
@ -53,7 +53,7 @@ LL | | };
|
||||
| |_________^ help: replace it with: `self.opt?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:90:9
|
||||
--> tests/ui/question_mark.rs:88:9
|
||||
|
|
||||
LL | / if self.opt.is_none() {
|
||||
LL | |
|
||||
@ -62,7 +62,7 @@ LL | | }
|
||||
| |_________^ help: replace it with: `self.opt.as_ref()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:99:9
|
||||
--> tests/ui/question_mark.rs:97:9
|
||||
|
|
||||
LL | / if self.opt.is_none() {
|
||||
LL | |
|
||||
@ -71,7 +71,7 @@ LL | | }
|
||||
| |_________^ help: replace it with: `self.opt.as_ref()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:108:9
|
||||
--> tests/ui/question_mark.rs:106:9
|
||||
|
|
||||
LL | / if self.opt.is_none() {
|
||||
LL | |
|
||||
@ -80,7 +80,7 @@ LL | | }
|
||||
| |_________^ help: replace it with: `self.opt.as_ref()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:116:26
|
||||
--> tests/ui/question_mark.rs:114:26
|
||||
|
|
||||
LL | let v: &Vec<_> = if let Some(ref v) = self.opt {
|
||||
| __________________________^
|
||||
@ -92,7 +92,7 @@ LL | | };
|
||||
| |_________^ help: replace it with: `self.opt.as_ref()?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:127:17
|
||||
--> tests/ui/question_mark.rs:125:17
|
||||
|
|
||||
LL | let v = if let Some(v) = self.opt {
|
||||
| _________________^
|
||||
@ -104,7 +104,7 @@ LL | | };
|
||||
| |_________^ help: replace it with: `self.opt?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:149:5
|
||||
--> tests/ui/question_mark.rs:147:5
|
||||
|
|
||||
LL | / if f().is_none() {
|
||||
LL | |
|
||||
@ -113,7 +113,7 @@ LL | | }
|
||||
| |_____^ help: replace it with: `f()?;`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:154:16
|
||||
--> tests/ui/question_mark.rs:152:16
|
||||
|
|
||||
LL | let _val = match f() {
|
||||
| ________________^
|
||||
@ -124,7 +124,7 @@ LL | | };
|
||||
| |_____^ help: try instead: `f()?`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:165:5
|
||||
--> tests/ui/question_mark.rs:163:5
|
||||
|
|
||||
LL | / match f() {
|
||||
LL | |
|
||||
@ -134,7 +134,7 @@ LL | | };
|
||||
| |_____^ help: try instead: `f()?`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:171:5
|
||||
--> tests/ui/question_mark.rs:169:5
|
||||
|
|
||||
LL | / match opt_none!() {
|
||||
LL | |
|
||||
@ -144,13 +144,13 @@ LL | | };
|
||||
| |_____^ help: try instead: `opt_none!()?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:198:13
|
||||
--> tests/ui/question_mark.rs:196:13
|
||||
|
|
||||
LL | let _ = if let Ok(x) = x { x } else { return x };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:201:5
|
||||
--> tests/ui/question_mark.rs:199:5
|
||||
|
|
||||
LL | / if x.is_err() {
|
||||
LL | |
|
||||
@ -159,7 +159,7 @@ LL | | }
|
||||
| |_____^ help: replace it with: `x?;`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:206:16
|
||||
--> tests/ui/question_mark.rs:204:16
|
||||
|
|
||||
LL | let _val = match func_returning_result() {
|
||||
| ________________^
|
||||
@ -170,7 +170,7 @@ LL | | };
|
||||
| |_____^ help: try instead: `func_returning_result()?`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:212:5
|
||||
--> tests/ui/question_mark.rs:210:5
|
||||
|
|
||||
LL | / match func_returning_result() {
|
||||
LL | |
|
||||
@ -180,7 +180,7 @@ LL | | };
|
||||
| |_____^ help: try instead: `func_returning_result()?`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:304:5
|
||||
--> tests/ui/question_mark.rs:302:5
|
||||
|
|
||||
LL | / if let Err(err) = func_returning_result() {
|
||||
LL | |
|
||||
@ -189,7 +189,7 @@ LL | | }
|
||||
| |_____^ help: replace it with: `func_returning_result()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:312:5
|
||||
--> tests/ui/question_mark.rs:310:5
|
||||
|
|
||||
LL | / if let Err(err) = func_returning_result() {
|
||||
LL | |
|
||||
@ -198,7 +198,7 @@ LL | | }
|
||||
| |_____^ help: replace it with: `func_returning_result()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:395:13
|
||||
--> tests/ui/question_mark.rs:393:13
|
||||
|
|
||||
LL | / if a.is_none() {
|
||||
LL | |
|
||||
@ -208,7 +208,7 @@ LL | | }
|
||||
| |_____________^ help: replace it with: `a?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:456:5
|
||||
--> tests/ui/question_mark.rs:454:5
|
||||
|
|
||||
LL | / let Some(v) = bar.foo.owned.clone() else {
|
||||
LL | | return None;
|
||||
@ -216,7 +216,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let v = bar.foo.owned.clone()?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:471:5
|
||||
--> tests/ui/question_mark.rs:469:5
|
||||
|
|
||||
LL | / let Some(ref x) = foo.opt_x else {
|
||||
LL | | return None;
|
||||
@ -224,7 +224,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let x = foo.opt_x.as_ref()?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:481:5
|
||||
--> tests/ui/question_mark.rs:479:5
|
||||
|
|
||||
LL | / let Some(ref mut x) = foo.opt_x else {
|
||||
LL | | return None;
|
||||
@ -232,7 +232,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let x = foo.opt_x.as_mut()?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:492:5
|
||||
--> tests/ui/question_mark.rs:490:5
|
||||
|
|
||||
LL | / let Some(ref x @ ref y) = foo.opt_x else {
|
||||
LL | | return None;
|
||||
@ -240,7 +240,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let x @ y = foo.opt_x.as_ref()?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:496:5
|
||||
--> tests/ui/question_mark.rs:494:5
|
||||
|
|
||||
LL | / let Some(ref x @ WrapperStructWithString(_)) = bar else {
|
||||
LL | | return None;
|
||||
@ -248,7 +248,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let x @ &WrapperStructWithString(_) = bar.as_ref()?;`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:500:5
|
||||
--> tests/ui/question_mark.rs:498:5
|
||||
|
|
||||
LL | / let Some(ref mut x @ WrapperStructWithString(_)) = bar else {
|
||||
LL | | return None;
|
||||
@ -256,7 +256,7 @@ LL | | };
|
||||
| |______^ help: replace it with: `let x @ &mut WrapperStructWithString(_) = bar.as_mut()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:522:5
|
||||
--> tests/ui/question_mark.rs:520:5
|
||||
|
|
||||
LL | / if arg.is_none() {
|
||||
LL | |
|
||||
@ -265,7 +265,7 @@ LL | | }
|
||||
| |_____^ help: replace it with: `arg?;`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:526:15
|
||||
--> tests/ui/question_mark.rs:524:15
|
||||
|
|
||||
LL | let val = match arg {
|
||||
| _______________^
|
||||
@ -276,12 +276,42 @@ LL | | };
|
||||
| |_____^ help: try instead: `arg?`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> tests/ui/question_mark.rs:536:5
|
||||
--> tests/ui/question_mark.rs:534:5
|
||||
|
|
||||
LL | / let Some(a) = *a else {
|
||||
LL | | return None;
|
||||
LL | | };
|
||||
| |______^ help: replace it with: `let a = (*a)?;`
|
||||
|
||||
error: aborting due to 30 previous errors
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:566:5
|
||||
|
|
||||
LL | / match some_result {
|
||||
LL | |
|
||||
LL | | Ok(val) => val,
|
||||
LL | | Err(err) => return Err(err.into()),
|
||||
LL | | };
|
||||
| |_____^ help: try instead: `some_result?`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:572:5
|
||||
|
|
||||
LL | / match some_result {
|
||||
LL | |
|
||||
LL | | Ok(val) => val,
|
||||
LL | | Err(err) => return Err(Into::into(err)),
|
||||
LL | | };
|
||||
| |_____^ help: try instead: `some_result?`
|
||||
|
||||
error: this `match` expression can be replaced with `?`
|
||||
--> tests/ui/question_mark.rs:578:5
|
||||
|
|
||||
LL | / match some_result {
|
||||
LL | |
|
||||
LL | | Ok(val) => val,
|
||||
LL | | Err(err) => return Err(<&str as Into<String>>::into(err)),
|
||||
LL | | };
|
||||
| |_____^ help: try instead: `some_result?`
|
||||
|
||||
error: aborting due to 33 previous errors
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user