about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-09-29 18:49:57 +0000
committerbors <bors@rust-lang.org>2023-09-29 18:49:57 +0000
commitb00236d7f0537b22794be838064c944a36e85c70 (patch)
treeaa26d6fdbef44ff2716c791accbcaa55a111de3f
parent67a83ff0576488faf22ee73d40d3c3590c832224 (diff)
parent2d2017942afc221da0295962ac0374bd31cc6d1c (diff)
downloadrust-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.rs4
-rw-r--r--tests/ui/manual_let_else_match.fixed4
-rw-r--r--tests/ui/manual_let_else_match.rs8
-rw-r--r--tests/ui/manual_let_else_match.stderr12
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