Skip to content

fix: don't make MissingMatchArms diagnostic for empty match body #15971

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 30, 2023
Merged

fix: don't make MissingMatchArms diagnostic for empty match body #15971

merged 2 commits into from
Nov 30, 2023

Conversation

@Young-Flash
Copy link
Member

@Young-Flash Young-Flash commented Nov 26, 2023

before
before

after
after

close #15954

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2023
lnicola
lnicola approved these changes Nov 26, 2023
View reviewed changes
Copy link
Member

@lnicola lnicola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it still runs, but we hide it. Guess it's fine though.

@lnicola
Copy link
Member

lnicola commented Nov 26, 2023

Looks like the test is failing because of the diagnostic range.

@Young-Flash
Copy link
Member Author

Young-Flash commented Nov 27, 2023 •
edited
Loading

Looks like the test is failing because of the diagnostic range.

yeah, are there any mark that indicate diagnostic range with the same begin and end like 23..23? If not, could we just remove that test? 😄

@Veykril
Copy link
Member

Veykril commented Nov 27, 2023

You can just make a variant of check_diagnostics that also discards syntax errors for this one otherwise

@Young-Flash Young-Flash requested a review from lnicola November 29, 2023 09:18
@lnicola
Copy link
Member

lnicola commented Nov 29, 2023

It would be nice to make this more like check_diagnostics, that's quite a bit of duplicated code.

And making filtering syntax errors in check_diagnostics_with_config doesn't cause any test to fail.

lnicola
lnicola reviewed Nov 29, 2023
View reviewed changes
@lnicola
Copy link
Member

lnicola commented Nov 30, 2023

Wow, that actually works?

@bors r+

@bors
Copy link
Contributor

bors commented Nov 30, 2023

📌 Commit b46f378 has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 30, 2023

⌛ Testing commit b46f378 with merge 56abc0a...

@bors
Copy link
Contributor

bors commented Nov 30, 2023

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing 56abc0a to master...

@bors bors merged commit 56abc0a into rust-lang:master Nov 30, 2023
@Young-Flash
Copy link
Member Author

Young-Flash commented Nov 30, 2023

Wow, that actually works?

exactly 😄

@Young-Flash Young-Flash deleted the fix_match_arm branch November 30, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lnicola lnicola lnicola approved these changes

Assignees

No one assigned

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing match arms should not be reported on matches missing a body

5 participants

@Young-Flash @lnicola @Veykril @bors @rustbot