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

Add support for PEP 696 syntax #11120

Merged
merged 15 commits into from
Apr 26, 2024
Merged

Add support for PEP 696 syntax #11120

merged 15 commits into from
Apr 26, 2024

Conversation

JelleZijlstra
Copy link
Contributor

Fixes #11099.

My strategy here was:

  • Add new fields to the AST nodes for TypeVar, ParamSpec, and TypeVarTuple
  • Fix all the missing pattern matching cases that the compiler found
  • Fix tests
  • Add parser support for the new syntax and update tests
  • Grep for existing code dealing with TypeVar bounds to find other places to edit

@JelleZijlstra JelleZijlstra mentioned this pull request Apr 24, 2024
Closed
@JelleZijlstra
Copy link
Contributor Author

The CI failure looks like a random timeout. (Speculation: Maybe this change triggered too many recompilations and therefore it timed out?)

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 24, 2024 •
edited

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

dhruvmanila
dhruvmanila reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Wow, that was blazingly fast! Thank you for working on this. It is an impressive PR as it touches all of the major areas of the codebase (AST, parser, linter, and formatter).

I've made some suggestions for consistency and have a few doubts regarding the grammar and the evaluation-order. But, otherwise I think this is pretty much good to go.

crates/ruff_python_ast/src/nodes.rs Outdated Show resolved Hide resolved
crates/ruff_python_parser/src/parser/statement.rs Show resolved Hide resolved
crates/ruff_python_parser/src/parser/statement.rs Show resolved Hide resolved
crates/ruff_python_formatter/src/type_param/type_param_type_var_tuple.rs Outdated Show resolved Hide resolved
crates/ruff_python_formatter/src/type_param/type_param_type_var.rs Outdated Show resolved Hide resolved
crates/ruff_python_formatter/src/type_param/type_param_param_spec.rs Outdated Show resolved Hide resolved
crates/ruff_python_ast/src/visitor.rs Outdated Show resolved Hide resolved
crates/ruff_python_ast/src/visitor/transformer.rs Outdated Show resolved Hide resolved
crates/ruff_python_ast/src/comparable.rs Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila added parser Related to the parser python313 Related to Python 3.13 labels Apr 24, 2024
MichaReiser
MichaReiser reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Wow nice! This is excellent.

Would you mind adding a few formatter snapshot tests? For example, you can add them to https://github.com/astral-sh/ruff/blob/0bf0aa28ac05eb4157c8e1932abc91301aaedce7/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/type_alias.py

Ideally, with a few cases that involve comments and extra long lines.

@JelleZijlstra
Copy link
Contributor Author

@MichaReiser thanks, I added a number of tests in 7e053f6. A few comments get moved to the other side of a punctuation mark but that seems fine.

It looks like you don't have a similar test case file for type parameter definitions on functions, other than the one in fixtures/black.

dhruvmanila
dhruvmanila approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thank you!

MichaReiser
MichaReiser approved these changes Apr 26, 2024
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you!

@MichaReiser MichaReiser merged commit cd3e319 into astral-sh:main Apr 26, 2024
18 checks passed
@charliermarsh
Copy link
Member

Thank you @JelleZijlstra, this is awesome.

@JelleZijlstra JelleZijlstra deleted the pep696 branch April 26, 2024 13:54
@BrewTestBot BrewTestBot mentioned this pull request May 4, 2024
Merged
@charliermarsh charliermarsh mentioned this pull request May 13, 2024
Merged
charliermarsh added a commit that referenced this pull request May 13, 2024
@charliermarsh
Add Python 3.13 to list of allowed Python versions (#11411)
6ed2482 
## Summary

I believe we're already "Python 3.13-ready"? The main Ruff-impacting
change I see in https://docs.python.org/3.13/whatsnew/3.13.html is [PEP
696](https://peps.python.org/pep-0696/) which Jelle added in
#11120.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@dhruvmanila dhruvmanila dhruvmanila approved these changes

Assignees
No one assigned
Labels
parser Related to the parser python313 Related to Python 3.13
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add parser support for PEP 696
4 participants
@JelleZijlstra @charliermarsh @MichaReiser @dhruvmanila