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

Partial unpacking in return, yield, and for before Python 3.9 #16560

Open
ntBre opened this issue Mar 7, 2025 · 2 comments
Open

Partial unpacking in return, yield, and for before Python 3.9 #16560

ntBre opened this issue Mar 7, 2025 · 2 comments
Labels
parser Related to the parser rule Implementing or modifying a lint rule

Comments

@ntBre
Copy link
Contributor

ntBre commented Mar 7, 2025

I made this a sub-issue of #6591 for easier discussion, but I ran into the fact that syntax like this:

def f(): return 1, (*rest)
def g(): yield 1, (*rest)
for _ in  1, (*rest): ...

is allowed on Python 3.7 and 3.8 but not on 3.9 while working on #16485 and then again on #16558.

We actually already emit a normal parse error for this, so the change I guess we'd need is more like skipping the existing ParseError based on the version rather than adding an UnsupportedSyntaxError.

On the other hand, this only affects Python versions before 3.9, which are no longer supported. I'm leaning toward just removing this from #6591 and considering it finished without this check, but I wanted to get some other thoughts on this too.

That approach also raises the question of whether we should have taken the same approach with the parenthesized keyword argument names in #16482. That's the only other example of removed syntax in #6591, unless I'm missing another case in my unmerged PRs. We could just emit a ParseError for that and not worry about removed syntax for versions before 3.9.

For the partial unpacking case, specifically, I also didn't find any documentation about that change, so I think it just fell out of the PEG parser change in 3.9.

@ntBre ntBre changed the title def i(): return 1, (*rest) is no longer allowed (something with *rest) Partial unpacking in return, yield, and for before Python 3.9 Mar 7, 2025
@ntBre ntBre added rule Implementing or modifying a lint rule parser Related to the parser labels Mar 7, 2025
@dhruvmanila
Copy link
Member

Starred expressions are a bit tricky because they depend on the context in which it's being used. I see that it seems to also need to consider the target version.

On the other hand, this only affects Python versions before 3.9, which are no longer supported. I'm leaning toward just removing this from #6591 and considering it finished without this check, but I wanted to get some other thoughts on this too.

I'd be fine to proceed with this.

That approach also raises the question of whether we should have taken the same approach with the parenthesized keyword argument names in #16482. That's the only other example of removed syntax in #6591, unless I'm missing another case in my unmerged PRs. We could just emit a ParseError for that and not worry about removed syntax for versions before 3.9.

Hmm, that's a good point. I wouldn't consider this a high priority but it might make sense to use ParseError for this case and just avoid raising ParseError if the target version allows it.

So, that prompted me to look at why the current implementation doesn't already raise a ParseError for this and I realize that I didn't special case this. In certain cases we do try to parse it using a more relaxed grammar but then later raise ParseError if we encounter invalid expression in that position (example).

This also make me think as to whether there's any need for Change::Removed especially as the current list only includes the removed syntax in end-of-life Python versions.

@ntBre
Copy link
Contributor Author

ntBre commented Mar 10, 2025

Thanks @dhruvmanila! It sounds like we're leaning in the same direction. I think you're right that we could get rid of Change::Removed and the Change enum if we just made these ParseErrors.

On the other hand, I mentioned this to Alex in our 1:1 today, and he said that a significant number of people are still using older, unsupported Python versions, so it may still be worth making these UnsupportedSyntaxErrors.

I agree that it's probably not too high a priority either way, especially this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Related to the parser rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

2 participants