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

Add parser support for PEP 696 #11099

Closed
JelleZijlstra opened this issue Apr 23, 2024 · 4 comments · Fixed by #11120
Closed

Add parser support for PEP 696 #11099

JelleZijlstra opened this issue Apr 23, 2024 · 4 comments · Fixed by #11120
Labels
parser Related to the parser python313 Related to Python 3.13

Comments

@JelleZijlstra
Copy link
Contributor

Ruff's parser does not support the new syntax introduced by PEP 696:

def f[T=int](): pass

I noticed because I'm adding this syntax to CPython and Ruff started failing in CI :).

I saw that you recently switched to a handwritten parser. I'm interested in taking a look at that parser, so I'd like to try to implement this myself.

@dhruvmanila
Copy link
Member

dhruvmanila commented Apr 23, 2024

Thank you for notifying us about the new syntax!

I saw that you recently switched to a handwritten parser. I'm interested in taking a look at that parser, so I'd like to try to implement this myself.

Yes, of course! Although the parser crate (ruff_python_parser) lacks a bit of public-facing documentation, specifically a README and an updated CONTRIBUTING guidelines, it shouldn't black you as I can assist with the implementation. That said, I'll be working on the documentation this week.

I'll need to go through the PEP to list the work items needed to complete this, but for reference, there's the PEP 701 issue and PEP 695 issue.

At a high level, this will impact all of the major areas: the AST, parser, linter, and formatter. If you're solely interested in updating the AST and parser, that's perfectly fine; either I or someone else from the team can handle implementing the changes in the linter and formatter.

@dhruvmanila dhruvmanila added parser Related to the parser python313 Related to Python 3.13 labels Apr 23, 2024
@JelleZijlstra
Copy link
Contributor Author

Thanks! I'll take a look myself first and let you know if I run into any issues. I expect the PEP 695 issue will provide a good reference as mechanically the new syntax is very similar to the bound syntax in PEP 695.

@JelleZijlstra
Copy link
Contributor Author

#11120 implements support in all the places I could find in the Ruff codebase. Rust's ADTs were very nice here; the compiler basically told me almost all the places I had to update. There's a test failure that I'll look into soon.

At runtime this sort of thing is an error:

>>> type X[T = int, U] = str
  File "<stdin>-0", line 1
SyntaxError: non-default type parameter 'U' follows default type parameter

Since Ruff doesn't detect a few similar cases in the existing syntax (#11118, #11119), I left these error checks out of the PEP 696 support too. It might make sense to add a new lint rule checking for these and similar issues, but that can wait for a different PR.

@dhruvmanila
Copy link
Member

At runtime this sort of thing is an error:

>>> type X[T = int, U] = str
  File "<stdin>-0", line 1
SyntaxError: non-default type parameter 'U' follows default type parameter

We do detect this specific case for function parameters. For reference:

// TODO(dhruvmanila): Pyright seems to only highlight the first non-default argument
// https://github.com/microsoft/pyright/blob/3b70417dd549f6663b8f86a76f75d8dfd450f4a8/packages/pyright-internal/src/parser/parser.ts#L2038-L2042
if param.default.is_none()
&& seen_default_param
&& !seen_keyword_only_separator
&& parameters.vararg.is_none()
{
// test_ok params_non_default_after_star
// def foo(a=10, *, b, c=11, d): ...
// def foo(a=10, *args, b, c=11, d): ...
// test_err params_non_default_after_default
// def foo(a=10, b, c: int): ...
parser
.add_error(ParseErrorType::NonDefaultParamAfterDefaultParam, &param);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Related to the parser python313 Related to Python 3.13
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants