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

feat: implement tuple return type to tuple struct assist #15696

Merged
merged 3 commits into from Oct 9, 2023

Conversation

rmehri01
Copy link
Contributor

@rmehri01 rmehri01 commented Oct 1, 2023

This PR implements the convert_tuple_return_type_to_struct assist, for converting the return type of a function or method from a tuple to a tuple struct. Additionally, it moves the to_camel_case and char_has_case functions from case_conv to stdx so that they can be used similar to to_lower_snake_case.

tuple_return_type_to_tuple_struct.webm

Currently, the assist puts the struct definition above the function, or above the nearest impl or trait if applicable and only rewrites literal tuples that are returned in the body of the function. Additionally, it only attempts to rewrite simple tuple pattern usages with the corresponding tuple struct pattern but does so across files and modules.

I think that this is sufficient for the majority of use cases but I could be wrong. One thing I'm still not sure how to approach is handling Self and generics/lifetimes in the tuple type to be extracted. I was thinking of either manually figuring out what lifetimes and generics are in scope and using them (sort of similar to the generate_function assist) or maybe using ctx.sema.resolve_type and generic_params on hir::Type but this seems to not deal with lifetimes.

Closes #14293

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 1, 2023
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

.

Comment on lines 131 to 132
.ancestors()
.nth(5)
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment about the magic 5 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah sorry, this is mostly a relic, I was basically trying to handle this case where we don't want to replace the outer tuple with the struct pattern. So, I was trying to get the direct parent of the call or method call expression but I've pushed a cleaner version now.

let ((bar1, bar2), foo) = (bar(), 3);

Also, in this case it would be nice to replace the corresponding pattern but I'm not sure if/how we should do that? Like:

let (BarResult(bar1, bar2), foo) = (bar(), 3);

Copy link
Member

Choose a reason for hiding this comment

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

Ye that's tricky, we can only do heuristics here I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe we can leave it for now then, I think figuring out how to handle generics is more important.

@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 Oct 4, 2023
@Veykril
Copy link
Member

Veykril commented Oct 9, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 9, 2023

📌 Commit 9ba8dbc has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 9, 2023

⌛ Testing commit 9ba8dbc with merge ab62c01...

@bors
Copy link
Collaborator

bors commented Oct 9, 2023

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

@bors bors merged commit ab62c01 into rust-lang:master Oct 9, 2023
10 checks passed
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.

feature request: convert tuple return type to tuple struct.
4 participants