-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Deunwrap add_missing_match_arms #15594
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
Conversation
.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"), |
There was a problem hiding this comment.
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 {}
}
There was a problem hiding this comment.
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: [email protected]
[email protected]
[email protected] "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.
There was a problem hiding this comment.
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.
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.
@bors r+ |
☀️ Test successful - checks-actions |
Last subtask of #15398