Add Python version checks to native parser#21539
Conversation
This comment has been minimized.
This comment has been minimized.
fc2b267 to
3a8407e
Compare
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
JukkaL
left a comment
There was a problem hiding this comment.
Also these features may need checks:
- Star unpack (
tuple[*Ts]) introduced in 3.11 - Parentheses optional for multiple exception types in except
The latter may require changes to the parser. If that's the case, we can leave it until later.
| func_def.is_coroutine = True | ||
| read_loc(data, func_def) | ||
| if type_params: | ||
| state.check_min_version("Type parameter lists", (3, 12), func_def.line, func_def.column) |
There was a problem hiding this comment.
Maybe call this feature "Improved type parameter syntax"? You'd need to use singular "is" instead of "are" in the error message in this case, or leave out the verb altogether.
| class_def.decorators = decorators | ||
| read_loc(data, class_def) | ||
| if type_params: | ||
| state.check_min_version("Type parameter lists", (3, 12), class_def.line, class_def.column) |
There was a problem hiding this comment.
Similar to above (name of feature).
| titems.append(s) | ||
| expr = TemplateStrExpr(titems) | ||
| read_loc(data, expr) | ||
| state.check_min_version("t-strings", (3, 14), expr.line, expr.column) |
There was a problem hiding this comment.
I think T-strings are a bit special since they need to new stdlib type, and it's not defined on older versions. This means that we probably shouldn't allow them in stubs either unless targeting 3.14 or later. Also this is important to test using real stubs (pythoneval.test) to ensure the missing type doesn't cause issues.
This comment has been minimized.
This comment has been minimized.
|
So looking at this further, currently the stance is that version check errors are non blocking, so they have If we keep it as non blocking but don't reset the is_star, it would cause a mypy crash with INTERNAL_ERROR because typeshed gates that class for the specific version and would complain that the symbol isn't in the builtins symbol table. The other alernative is to keep make it a blocking error just for this case, however im not sure if that would be inconsistent since it may cause confusion for the user on whether version checks do block or not? Or we follow fastparse and always block. |
|
regarding this feature:
I believe i would need to update the rust side to record whether the source used parenthesis and then have the python read it and do the version check, so it might be better to do it in a later pr |
This comment has been minimized.
This comment has been minimized.
JukkaL
left a comment
There was a problem hiding this comment.
I think it's fine to just assign False to the is_star flag for now, since that feature isn't available on older Python versions, and it's more important to not generate a blocking error than to provide the same AST we'd generate on a newer version.
| except* ValueError: | ||
| y | ||
| [out] | ||
| 1:0: error: Exception groups: requires Python 3.11 or newer (current target: Python 3.10) |
There was a problem hiding this comment.
You can move these tests to a new native-parser-python311.test file and register it in test_nativeparse.py so that you'll get the correct Python version set (and similarly for test cases the require 3.12, etc.).
| from typing_extensions import TypeVarTuple, Unpack | ||
| Ts = TypeVarTuple("Ts") | ||
| def f(x: tuple[Unpack[Ts]]) -> None: ... | ||
| def g(x: tuple[*Ts]) -> None: ... |
There was a problem hiding this comment.
Add reveal_type(g) and check what happens if we call g. It's okay if this behaves inconsistently, as long as it doesn't crash and doesn't do anything very strange.
04e318a to
2871a66
Compare
2871a66 to
faa7f42
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
sounds good then, i've addressed the comments! |
JukkaL
left a comment
There was a problem hiding this comment.
Thanks, looks good! One more thing -- can you add a pythoneval test that uses the new generics syntax in a .pyi file while targeting 3.10? The test case can be added in a follow-up PR if you prefer.
The native parser (rust based) previously accepted newer Python syntax without checking the configured target Python version, whereas the legacy parser (fastparse) would report when a feature isn't available on the target (although those would be blocking errors). This PR adds those compatibility checks to the native parser so that it produces a non blocking error.
The feature gates added are:
I've also moved the tests that would now introduce a python version check error to it's own test file so that it gets the correct Python versoin
In a later PR, I will be adding support for checking this feature since it requires modifying the rust parser a bit: