diff options
| author | bors <bors@rust-lang.org> | 2023-09-29 18:49:57 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-09-29 18:49:57 +0000 |
| commit | b00236d7f0537b22794be838064c944a36e85c70 (patch) | |
| tree | aa26d6fdbef44ff2716c791accbcaa55a111de3f | |
| parent | 67a83ff0576488faf22ee73d40d3c3590c832224 (diff) | |
| parent | 2d2017942afc221da0295962ac0374bd31cc6d1c (diff) | |
| download | rust-b00236d7f0537b22794be838064c944a36e85c70.tar.gz rust-b00236d7f0537b22794be838064c944a36e85c70.zip | |
Auto merge of #11580 - y21:issue11579, r=Jarcho
[`manual_let_else`]: only omit block if span is from same ctxt
Fixes #11579.
The lint already had logic for omitting a block in `else` if a block is already present, however this didn't handle the case where the block is from a different expansion/syntax context. E.g.
```rs
macro_rules! panic_in_block {
() => { { panic!() } }
}
let _ = match Some(1) {
Some(v) => v,
_ => panic_in_block!()
};
```
It would see this in its expanded form as `_ => { panic!() }` and think it doesn't have to include a block in its suggestion because it is already there, however that's not true if it's from a different expansion like in this case.
changelog: [`manual_let_else`]: only omit block in suggestion if the block is from the same expansion
| -rw-r--r-- | clippy_lints/src/manual_let_else.rs | 4 | ||||
| -rw-r--r-- | tests/ui/manual_let_else_match.fixed | 4 | ||||
| -rw-r--r-- | tests/ui/manual_let_else_match.rs | 8 | ||||
| -rw-r--r-- | tests/ui/manual_let_else_match.stderr | 12 |
4 files changed, 25 insertions, 3 deletions
diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index c531137b785..2117308cd40 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -136,9 +136,9 @@ fn emit_manual_let_else( // for this to be machine applicable. let mut app = Applicability::HasPlaceholders; let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app); - let (sn_else, _) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app); + let (sn_else, else_is_mac_call) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app); - let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) { + let else_bl = if matches!(else_body.kind, ExprKind::Block(..)) && !else_is_mac_call { sn_else.into_owned() } else { format!("{{ {sn_else} }}") diff --git a/tests/ui/manual_let_else_match.fixed b/tests/ui/manual_let_else_match.fixed index 09b713f0410..588ba5edd8f 100644 --- a/tests/ui/manual_let_else_match.fixed +++ b/tests/ui/manual_let_else_match.fixed @@ -133,3 +133,7 @@ fn not_fire() { [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ ..] => data, }; } + +fn issue11579() { + let Some(msg) = Some("hi") else { unreachable!("can't happen") }; +} diff --git a/tests/ui/manual_let_else_match.rs b/tests/ui/manual_let_else_match.rs index e6af4738420..c37b5613ff7 100644 --- a/tests/ui/manual_let_else_match.rs +++ b/tests/ui/manual_let_else_match.rs @@ -170,3 +170,11 @@ fn not_fire() { [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ ..] => data, }; } + +fn issue11579() { + let msg = match Some("hi") { + //~^ ERROR: this could be rewritten as `let...else` + Some(m) => m, + _ => unreachable!("can't happen"), + }; +} diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index 8ca2c84072d..18bfe324ba7 100644 --- a/tests/ui/manual_let_else_match.stderr +++ b/tests/ui/manual_let_else_match.stderr @@ -92,5 +92,15 @@ LL | | _ => return, LL | | }; | |______^ help: consider writing: `let ([data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0]) = data.as_slice() else { return };` -error: aborting due to 9 previous errors +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else_match.rs:175:5 + | +LL | / let msg = match Some("hi") { +LL | | +LL | | Some(m) => m, +LL | | _ => unreachable!("can't happen"), +LL | | }; + | |______^ help: consider writing: `let Some(msg) = Some("hi") else { unreachable!("can't happen") };` + +error: aborting due to 10 previous errors |
