about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-27 13:01:33 +0000
committerbors <bors@rust-lang.org>2022-10-27 13:01:33 +0000
commitf5d225de3733aa2e1f91f41b5da4ec5dd773a06d (patch)
treedc3f2b6981df8b5e4f86e4bb033666c4bfb5f404
parent710999d941cf1ed911331673f4fbde6279922bb4 (diff)
parent98250af4a3835dd3e24595ab40b37ee79d72203a (diff)
downloadrust-f5d225de3733aa2e1f91f41b5da4ec5dd773a06d.tar.gz
rust-f5d225de3733aa2e1f91f41b5da4ec5dd773a06d.zip
Auto merge of #9722 - ebobrow:question-mark, r=Manishearth
`question_mark` don't lint on `if let Err` with `else`

cc #9518

AFAICT the only time this would be a valid suggestion is the rather esoteric

```rust
let _ = if let Err(e) = x {
    return Err(e);
} else {
    // no side effects
    x.unwrap()
}
```

which doesn't seem worth checking to me. Please correct me if I'm missing something.

changelog: [`question_mark`] don't lint on `if let Err` with `else`
-rw-r--r--clippy_lints/src/question_mark.rs1
-rw-r--r--tests/ui/question_mark.fixed3
-rw-r--r--tests/ui/question_mark.rs3
-rw-r--r--tests/ui/question_mark.stderr4
4 files changed, 9 insertions, 2 deletions
diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index 328371fd602..5644c2f72db 100644
--- a/clippy_lints/src/question_mark.rs
+++ b/clippy_lints/src/question_mark.rs
@@ -189,6 +189,7 @@ fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_
                             && expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, Some(let_pat_sym)))
                             || is_res_lang_ctor(cx, res, ResultErr)
                                 && expr_return_none_or_err(smbl, cx, if_then, let_expr, Some(let_pat_sym))
+                                && if_else.is_none()
                     },
                     _ => false,
                 }
diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed
index 993389232cc..5c49d46da72 100644
--- a/tests/ui/question_mark.fixed
+++ b/tests/ui/question_mark.fixed
@@ -134,6 +134,9 @@ fn result_func(x: Result<i32, i32>) -> Result<i32, i32> {
         return func_returning_result();
     }
 
+    // no warning
+    let _ = if let Err(e) = x { Err(e) } else { Ok(0) };
+
     Ok(y)
 }
 
diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
index 9ae0d88829a..d057df6a9b3 100644
--- a/tests/ui/question_mark.rs
+++ b/tests/ui/question_mark.rs
@@ -166,6 +166,9 @@ fn result_func(x: Result<i32, i32>) -> Result<i32, i32> {
         return func_returning_result();
     }
 
+    // no warning
+    let _ = if let Err(e) = x { Err(e) } else { Ok(0) };
+
     Ok(y)
 }
 
diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr
index 1b6cd524b2f..23172d7e535 100644
--- a/tests/ui/question_mark.stderr
+++ b/tests/ui/question_mark.stderr
@@ -115,7 +115,7 @@ LL | |     }
    | |_____^ help: replace it with: `x?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:193:5
+  --> $DIR/question_mark.rs:196:5
    |
 LL | /     if let Err(err) = func_returning_result() {
 LL | |         return Err(err);
@@ -123,7 +123,7 @@ LL | |     }
    | |_____^ help: replace it with: `func_returning_result()?;`
 
 error: this block may be rewritten with the `?` operator
-  --> $DIR/question_mark.rs:200:5
+  --> $DIR/question_mark.rs:203:5
    |
 LL | /     if let Err(err) = func_returning_result() {
 LL | |         return Err(err);