-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Assist: replace unwrap with match #3732
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
Conversation
cd3ad5b to
51c2f15
Compare
| let ty = ctx.sema.type_of_expr(&caller)?; | ||
|
|
||
| let type_name = ty.as_adt()?.name(ctx.sema.db); | ||
| if type_name == name![Result] { |
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.
Rather then using name!, let's do type_name.to_string() == "Result". name! should be mostly internal. In particular, we probably shouldn't add Option there just for the sake of a single assit.
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.
We could actually make this completely agnostic of the particular enum by looking at the code of the unwrap function and using the branches from there, assuming it's a single match (which it is for Option and Result)... I'm not suggesting we do this right now though 😅
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.
Hmm, yeah, you know going further an assist to inline the function call should actually be a superset of this assist!
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.
Well, except that at least the Result implementation calls a private function instead of directly panicing 😅
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.
bruh
51c2f15 to
d9df0f4
Compare
|
bors r+ |
3732: Assist: replace unwrap with match r=matklad a=unrealhoang attempt on #3669 Co-authored-by: Unreal Hoang <[email protected]>
|
CI failure is unrelated. Could you rebase on top of #3735? bors d+ |
|
✌️ unrealhoang can now approve this pull request. To approve and merge a pull request, simply reply with |
|
This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried. |
Build succeeded |
attempt on #3669