diff options
| author | bors <bors@rust-lang.org> | 2021-10-26 23:46:21 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-10-26 23:46:21 +0000 |
| commit | ba2ac3e26360a827fcc0e741d3e831ca7e9aa4f4 (patch) | |
| tree | 2f47b5c227c7e6ff55778186162b39bb32fe29d5 | |
| parent | a48367e21cd2bd489eb01f576556a11eeb0efce9 (diff) | |
| parent | fb0fbad5caa97da949cb2c6dbc56a1185bb53af9 (diff) | |
| download | rust-ba2ac3e26360a827fcc0e741d3e831ca7e9aa4f4.tar.gz rust-ba2ac3e26360a827fcc0e741d3e831ca7e9aa4f4.zip | |
Auto merge of #7860 - dswij:question-mark-fp, r=giraffate
Fix `question_mark` FP on custom error type Closes #7859 #7840 aims to ignore `question_mark` when the return type is custom, which is [covered here](https://github.com/rust-lang/rust-clippy/blob/df65291edd6b89a241fed483ab165c32df468746/tests/ui/question_mark.rs#L144-L149). But this fails when there is a call in conditional predicate changelog: [`question_mark`] Fix false positive when there is call in conditional predicate
| -rw-r--r-- | clippy_lints/src/question_mark.rs | 16 | ||||
| -rw-r--r-- | tests/ui/question_mark.fixed | 21 | ||||
| -rw-r--r-- | tests/ui/question_mark.rs | 21 | ||||
| -rw-r--r-- | tests/ui/question_mark.stderr | 4 |
4 files changed, 45 insertions, 17 deletions
diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 4d616e26bfc..f63ef163bcb 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -172,23 +172,17 @@ impl QuestionMark { } } - fn expression_returns_unmodified_err( - cx: &LateContext<'_>, - expression: &Expr<'_>, - origin_hir_id: &Expr<'_>, - ) -> bool { - match expression.kind { + fn expression_returns_unmodified_err(cx: &LateContext<'_>, expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool { + match expr.kind { ExprKind::Block(block, _) => { if let Some(return_expression) = Self::return_expression(block) { - return Self::expression_returns_unmodified_err(cx, return_expression, origin_hir_id); + return Self::expression_returns_unmodified_err(cx, return_expression, cond_expr); } false }, - ExprKind::Ret(Some(expr)) | ExprKind::Call(expr, _) => { - Self::expression_returns_unmodified_err(cx, expr, origin_hir_id) - }, - ExprKind::Path(_) => path_to_local(expression) == path_to_local(origin_hir_id), + ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr), + ExprKind::Path(_) => path_to_local(expr) == path_to_local(cond_expr), _ => false, } } diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index ccb2e5a302e..e93469e5f55 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -104,7 +104,11 @@ fn func() -> Option<i32> { Some(0) } -fn result_func(x: Result<i32, &str>) -> Result<i32, &str> { +fn func_returning_result() -> Result<i32, i32> { + Ok(1) +} + +fn result_func(x: Result<i32, i32>) -> Result<i32, i32> { let _ = x?; x?; @@ -113,9 +117,22 @@ fn result_func(x: Result<i32, &str>) -> Result<i32, &str> { let y = if let Ok(x) = x { x } else { - return Err("some error"); + return Err(0); + }; + + // issue #7859 + // no warning + let _ = if let Ok(x) = func_returning_result() { + x + } else { + return Err(0); }; + // no warning + if func_returning_result().is_err() { + return func_returning_result(); + } + Ok(y) } diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index ca3722371f5..dd179e9bee8 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -134,7 +134,11 @@ fn func() -> Option<i32> { Some(0) } -fn result_func(x: Result<i32, &str>) -> Result<i32, &str> { +fn func_returning_result() -> Result<i32, i32> { + Ok(1) +} + +fn result_func(x: Result<i32, i32>) -> Result<i32, i32> { let _ = if let Ok(x) = x { x } else { return x }; if x.is_err() { @@ -145,9 +149,22 @@ fn result_func(x: Result<i32, &str>) -> Result<i32, &str> { let y = if let Ok(x) = x { x } else { - return Err("some error"); + return Err(0); + }; + + // issue #7859 + // no warning + let _ = if let Ok(x) = func_returning_result() { + x + } else { + return Err(0); }; + // no warning + if func_returning_result().is_err() { + return func_returning_result(); + } + Ok(y) } diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 161588cb73c..8d782b71dd6 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -101,13 +101,13 @@ LL | | } | |_____^ help: replace it with: `f()?;` error: this if-let-else may be rewritten with the `?` operator - --> $DIR/question_mark.rs:138:13 + --> $DIR/question_mark.rs:142: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 - --> $DIR/question_mark.rs:140:5 + --> $DIR/question_mark.rs:144:5 | LL | / if x.is_err() { LL | | return x; |
