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

Deunwrap add_missing_match_arms #15594

Merged
merged 2 commits into from Sep 22, 2023

Conversation

alibektas
Copy link
Member

Last subtask of #15398

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2023
.map(|f| make::ext::simple_ident_pat(f.name().unwrap()).into());
let pats = field_list.fields().map(|f| {
make::ext::simple_ident_pat(
f.name().expect("Record field must have a name"),
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like it could panic in the following situation, but I'm not exactly sure if it will:

enum A {
    A,
    Missing { a: u32, : u32, c: u32 }
}

fn a() {
    let b = A::A;
    match b$0 {}
}

Copy link
Member Author

@alibektas alibektas Sep 15, 2023

Choose a reason for hiding this comment

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

Thank you for mentioning this, I checked it and it doesn't. Syntax tree for the missing field name shows :

[crates/ide-assists/src/handlers/add_missing_match_arms.rs:458] &f = RecordField {
    syntax: RECORD_FIELD@40..43
      NAME@40..43
        IDENT@40..43 "u32"
    ,

how little the piece may be it is still being recognized as NAME and something like { a:32 , , c: u32} completely skips seeing a record field between a and c. So we are good.

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to add this as a test case though, we might change recovery in the future which could change this behavior.

@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 22, 2023
Although it doesn't panic now, further changes to how we recover from incomplete syntax
may cause this assist to panic. To mitigate this a test case has been added.
@Veykril
Copy link
Member

Veykril commented Sep 22, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

📌 Commit 622e1a8 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

⌛ Testing commit 622e1a8 with merge 59bcbaf...

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 59bcbaf to master...

@bors bors merged commit 59bcbaf into rust-lang:master Sep 22, 2023
10 checks passed
@lnicola lnicola mentioned this pull request Sep 25, 2023
15 tasks
@alibektas alibektas deleted the deunwrap/add_missing_match_arms branch September 29, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants