Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[manual_let_else]: only omit block if span is from same ctxt #11580

Merged
merged 1 commit into from Sep 29, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Sep 28, 2023

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.

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

@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2023

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 28, 2023
@Jarcho
Copy link
Contributor

Jarcho commented Sep 29, 2023

snippet_with_context already returns whether the snippet is a macro call or not. That should be used instead.

@y21
Copy link
Member Author

y21 commented Sep 29, 2023

Nice, that's even better.

@Jarcho
Copy link
Contributor

Jarcho commented Sep 29, 2023

Thank you. @bors r+

@bors
Copy link
Collaborator

bors commented Sep 29, 2023

📌 Commit 2d20179 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 29, 2023

⌛ Testing commit 2d20179 with merge b00236d...

@bors
Copy link
Collaborator

bors commented Sep 29, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing b00236d to master...

@bors bors merged commit b00236d into rust-lang:master Sep 29, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manual let else suggests fix that doesn't compile
4 participants