Unraw complex struct enum field python names#5050
Merged
Conversation
Stray case related to PyO3#2395 (probably unnoticed because the functionality is newer and a bit of an edge case). The `python_name` of struct-type enum variant fields (and the `__match_args__` of the variant) do not unraw the names of these fields, which effectively makes them unusable as the resulting identifiers contain '#' which the Python lexer interprets as comments. The only reason you'd probably want to do this anyway is to use reserved identifiers (`type` in my case).
hgmich
commented
Apr 6, 2025
| field_names: &mut Vec<Ident>, | ||
| ) -> syn::Result<(MethodAndMethodDef, syn::ImplItemFn)> { | ||
| let ident = format_ident!("__match_args__"); | ||
| let field_names_unraw: Vec<_> = field_names.iter().map(|name| name.unraw()).collect(); |
Contributor
Author
There was a problem hiding this comment.
Not quite sure this is the best way to do this. Feedback appreciated.
Member
There was a problem hiding this comment.
I think you don't have to collect here, an iterator should be enough for the quote below. (But that's just a tiny optimization)
No need to pass mutable vec ref anymore
Icxolu
reviewed
Apr 7, 2025
Member
Icxolu
left a comment
Icxolu
left a comment
There was a problem hiding this comment.
Thank you! This looks good to me. Maybe we can add a small test case for it?
| field_names: &mut Vec<Ident>, | ||
| ) -> syn::Result<(MethodAndMethodDef, syn::ImplItemFn)> { | ||
| let ident = format_ident!("__match_args__"); | ||
| let field_names_unraw: Vec<_> = field_names.iter().map(|name| name.unraw()).collect(); |
Member
There was a problem hiding this comment.
I think you don't have to collect here, an iterator should be enough for the quote below. (But that's just a tiny optimization)
hgmich
force-pushed
the
fix-raw-ident-enum-variant
branch
from
231c0e5 to
f72533d
Compare
Contributor
Author
Have done now. Originally missed that tests were in the workspace root. I've added a test which covers both field lookup and destructuring via |
hgmich
force-pushed
the
fix-raw-ident-enum-variant
branch
2 times, most recently
from
6de4d76 to
c3ca207
Compare
hgmich
force-pushed
the
fix-raw-ident-enum-variant
branch
from
c3ca207 to
0c4c78f
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Contributor
Author
|
Whoops, looks like I need to consider how to avoid running the pattern matching test on Python versions < 3.10. |
Member
|
Ah, I think you can gate it with |
Contributor
Author
|
Thanks, I've done that and refactored into separate tests to allow easier gating. Should be good to go now! |
davidhewitt
pushed a commit
that referenced
this pull request
Apr 11, 2025
* Unraw complex struct enum field python names Stray case related to #2395 (probably unnoticed because the functionality is newer and a bit of an edge case). The `python_name` of struct-type enum variant fields (and the `__match_args__` of the variant) do not unraw the names of these fields, which effectively makes them unusable as the resulting identifiers contain '#' which the Python lexer interprets as comments. The only reason you'd probably want to do this anyway is to use reserved identifiers (`type` in my case). * Add newsfragment for fix * Fix clippy lints in impl_complex_enum_variant_match_args and callers No need to pass mutable vec ref anymore * don't overeagerly collect field_names iterator * add test case for complex enums containing raw identifiers * only test raw ident pattern matching on Python 3.10+
davidhewitt
pushed a commit
that referenced
this pull request
Apr 21, 2025
* Unraw complex struct enum field python names Stray case related to #2395 (probably unnoticed because the functionality is newer and a bit of an edge case). The `python_name` of struct-type enum variant fields (and the `__match_args__` of the variant) do not unraw the names of these fields, which effectively makes them unusable as the resulting identifiers contain '#' which the Python lexer interprets as comments. The only reason you'd probably want to do this anyway is to use reserved identifiers (`type` in my case). * Add newsfragment for fix * Fix clippy lints in impl_complex_enum_variant_match_args and callers No need to pass mutable vec ref anymore * don't overeagerly collect field_names iterator * add test case for complex enums containing raw identifiers * only test raw ident pattern matching on Python 3.10+
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stray case related to #2395 (probably unnoticed because the functionality is newer and a bit of an edge case).
The
python_nameof struct-type enum variant fields (and the__match_args__of the variant) do not unraw the names of these fields, which effectively makes them unusable as the resulting identifiers contain '#' which the Python lexer interprets as comments. The only reason you'd probably want to do this anyway is to use reserved identifiers (typein my case).