Don't lint manual_let_else in cases where the question mark operator would work
Also, lint question_mark for `let...else` clauses that can be simplified to use `?`. This lint isn't perfect as it doesn't support the unstable try blocks.
This commit is contained in:
parent
73f14176e3
commit
86391abc70
5 changed files with 245 additions and 2 deletions
56
tests/ui/manual_let_else_question_mark.fixed
Normal file
56
tests/ui/manual_let_else_question_mark.fixed
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
//@run-rustfix
|
||||
#![allow(unused_braces, unused_variables, dead_code)]
|
||||
#![allow(
|
||||
clippy::collapsible_else_if,
|
||||
clippy::unused_unit,
|
||||
clippy::let_unit_value,
|
||||
clippy::match_single_binding,
|
||||
clippy::never_loop
|
||||
)]
|
||||
#![warn(clippy::manual_let_else, clippy::question_mark)]
|
||||
|
||||
enum Variant {
|
||||
A(usize, usize),
|
||||
B(usize),
|
||||
C,
|
||||
}
|
||||
|
||||
fn g() -> Option<(u8, u8)> {
|
||||
None
|
||||
}
|
||||
|
||||
fn e() -> Variant {
|
||||
Variant::A(0, 0)
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
||||
fn foo() -> Option<()> {
|
||||
// Fire here, normal case
|
||||
let v = g()?;
|
||||
|
||||
// Don't fire here, the pattern is refutable
|
||||
let Variant::A(v, w) = e() else { return None };
|
||||
|
||||
// Fire here, the pattern is irrefutable
|
||||
let (v, w) = g()?;
|
||||
|
||||
// Don't fire manual_let_else in this instance: question mark can be used instead.
|
||||
let v = g()?;
|
||||
|
||||
// Do fire manual_let_else in this instance: question mark cannot be used here due to the return
|
||||
// body.
|
||||
let Some(v) = g() else {
|
||||
return Some(());
|
||||
};
|
||||
|
||||
// Here we could also fire the question_mark lint, but we don't (as it's a match and not an if let).
|
||||
// So we still emit manual_let_else here. For the *resulting* code, we *do* emit the question_mark
|
||||
// lint, so for rustfix reasons, we allow the question_mark lint here.
|
||||
#[allow(clippy::question_mark)]
|
||||
{
|
||||
let Some(v) = g() else { return None };
|
||||
}
|
||||
|
||||
Some(())
|
||||
}
|
||||
61
tests/ui/manual_let_else_question_mark.rs
Normal file
61
tests/ui/manual_let_else_question_mark.rs
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
//@run-rustfix
|
||||
#![allow(unused_braces, unused_variables, dead_code)]
|
||||
#![allow(
|
||||
clippy::collapsible_else_if,
|
||||
clippy::unused_unit,
|
||||
clippy::let_unit_value,
|
||||
clippy::match_single_binding,
|
||||
clippy::never_loop
|
||||
)]
|
||||
#![warn(clippy::manual_let_else, clippy::question_mark)]
|
||||
|
||||
enum Variant {
|
||||
A(usize, usize),
|
||||
B(usize),
|
||||
C,
|
||||
}
|
||||
|
||||
fn g() -> Option<(u8, u8)> {
|
||||
None
|
||||
}
|
||||
|
||||
fn e() -> Variant {
|
||||
Variant::A(0, 0)
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
||||
fn foo() -> Option<()> {
|
||||
// Fire here, normal case
|
||||
let Some(v) = g() else { return None };
|
||||
|
||||
// Don't fire here, the pattern is refutable
|
||||
let Variant::A(v, w) = e() else { return None };
|
||||
|
||||
// Fire here, the pattern is irrefutable
|
||||
let Some((v, w)) = g() else { return None };
|
||||
|
||||
// Don't fire manual_let_else in this instance: question mark can be used instead.
|
||||
let v = if let Some(v_some) = g() { v_some } else { return None };
|
||||
|
||||
// Do fire manual_let_else in this instance: question mark cannot be used here due to the return
|
||||
// body.
|
||||
let v = if let Some(v_some) = g() {
|
||||
v_some
|
||||
} else {
|
||||
return Some(());
|
||||
};
|
||||
|
||||
// Here we could also fire the question_mark lint, but we don't (as it's a match and not an if let).
|
||||
// So we still emit manual_let_else here. For the *resulting* code, we *do* emit the question_mark
|
||||
// lint, so for rustfix reasons, we allow the question_mark lint here.
|
||||
#[allow(clippy::question_mark)]
|
||||
{
|
||||
let v = match g() {
|
||||
Some(v_some) => v_some,
|
||||
_ => return None,
|
||||
};
|
||||
}
|
||||
|
||||
Some(())
|
||||
}
|
||||
49
tests/ui/manual_let_else_question_mark.stderr
Normal file
49
tests/ui/manual_let_else_question_mark.stderr
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> $DIR/manual_let_else_question_mark.rs:30:5
|
||||
|
|
||||
LL | let Some(v) = g() else { return None };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `let v = g()?;`
|
||||
|
|
||||
= note: `-D clippy::question-mark` implied by `-D warnings`
|
||||
|
||||
error: this `let...else` may be rewritten with the `?` operator
|
||||
--> $DIR/manual_let_else_question_mark.rs:36:5
|
||||
|
|
||||
LL | let Some((v, w)) = g() else { return None };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `let (v, w) = g()?;`
|
||||
|
||||
error: this block may be rewritten with the `?` operator
|
||||
--> $DIR/manual_let_else_question_mark.rs:39:13
|
||||
|
|
||||
LL | let v = if let Some(v_some) = g() { v_some } else { return None };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `g()?`
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else_question_mark.rs:43:5
|
||||
|
|
||||
LL | / let v = if let Some(v_some) = g() {
|
||||
LL | | v_some
|
||||
LL | | } else {
|
||||
LL | | return Some(());
|
||||
LL | | };
|
||||
| |______^
|
||||
|
|
||||
= note: `-D clippy::manual-let-else` implied by `-D warnings`
|
||||
help: consider writing
|
||||
|
|
||||
LL ~ let Some(v) = g() else {
|
||||
LL + return Some(());
|
||||
LL + };
|
||||
|
|
||||
|
||||
error: this could be rewritten as `let...else`
|
||||
--> $DIR/manual_let_else_question_mark.rs:54:9
|
||||
|
|
||||
LL | / let v = match g() {
|
||||
LL | | Some(v_some) => v_some,
|
||||
LL | | _ => return None,
|
||||
LL | | };
|
||||
| |__________^ help: consider writing: `let Some(v) = g() else { return None };`
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue