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.

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

fix: upmap ranges in convert_tuple_struct_to_named_struct assist #18912

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: upmap ranges in convert_tuple_struct_to_named_struct assist #18912

wants to merge 1 commit into from
+101 −1

Conversation

vishruth-thimmaiah
Copy link
Contributor

Fixes #18766

This PR upmaps ranges for the convert_tuple_struct_to_named_struct ide-assist, fixing broken substitutions within macros.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 10, 2025
Veykril
Veykril requested changes Jan 16, 2025
View reviewed changes
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.

Actual upmapping is done via the Semantics::original_range* functions which returns a file range (opposed to a text range), the important part there is to account for the file id that may be different than the one that is currently assumed by the code

@vishruth-thimmaiah
Copy link
Contributor Author

The existing functionality already handles file ids properly, I don't think additional changes are needed.

@Veykril
Copy link
Member

Veykril commented Jan 20, 2025

The existing functionality already handles file ids properly, I don't think additional changes are needed.

The file id we get from here might be different, additional original_range will fall back to the entire macro call if it fails upmapping which is not what we want. So this should make proper use of the file id and we also likely wanna use original_range_opt

@Veykril Veykril enabled auto-merge January 21, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Veykril Veykril Awaiting requested review from Veykril

Requested changes must be addressed to merge this pull request.

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.

convert_tuple_struct_to_named_struct produces invalid code
3 participants
@vishruth-thimmaiah @Veykril @rustbot