[ty] Add hint that PEP 604 union syntax is only available in 3.10+ #18192
Conversation
sharkdp
added
ty
|
| if op == &ast::Operator::BitOr && left_ty.into_class_literal().is_some() | ||
| && right_ty.into_class_literal().is_some() && Program::get(self.db()).python_version(self.db()) | ||
| < PythonVersion::PY310 | ||
| { | ||
| diag.info("Note that `X | Y` PEP 604 union syntax is only available in Python 3.10 and later"); | ||
| } |
There was a problem hiding this comment.
One could certainly construct cases where we emit this info even if the user is trying to do something that does not involve type hints, but it seems unlikely? And not too distracting even if?
On the contrary, there are probably also cases where we fail to emit this info due to the class-literal filter above. We could think about relaxing this (e.g. checking if type is in the MRO of both left_ty and right_ty, or further trying to detect other types that might appear in those positions), but I'm not sure if it's worth it?
sharkdp
force-pushed
the
david/pep604-union-diagnostic-hint
branch
from
8f3fb7d to
06f15ae
Compare
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
Sweet. I let someone else answer your comment around "filtering".
It would be nice if we could also show the actual python version, similar to #18068
| IntOrStr = int | str | ||
| ``` | ||
|
|
||
| ### Earlier versions |
There was a problem hiding this comment.
Should there be a test case for stringified annotations (from __future__ import annotations)?
There was a problem hiding this comment.
I don't understand how that's related to the functionality being added here? int | str in non-type-expressions is an error in Python 3.9 and earlier, no matter if annotations are deferred or not, right?
There was a problem hiding this comment.
if from __future__ import is present at the top of the file then all annotations are stringified, and so they should not error even in 3.9.
There was a problem hiding this comment.
In annotations, yes. But not in non-type expressions:
▶ uv run -p 3.9 python
Python 3.9.20 (main, Oct 16 2024, 04:36:33)
[Clang 18.1.8 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from __future__ import annotations
>>> def f(x: int | str): ...
...
>>> A = int | str
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'type' and 'type'
There was a problem hiding this comment.
I think it would be clearer to say "non-annotations" than "non-type-expressions", since (with implicit type alias support, which we don't fully have yet), int | str in A = int | str should be evaluated as a type expression -- but it will still fail at runtime in older Python versions, regardless of from __future__ import annotations, because it is not syntactically an annotation, and from __future__ import annotations affects only annotations.
There was a problem hiding this comment.
(This will probably also add complexity to the implementation to keep these tests passing when we improve our support for implicit type aliases, because we might need to evaluate these both as value expressions -- in order to get these diagnostics -- and as type expressions -- in order to know how to interpret them when used as a type alias. )
There was a problem hiding this comment.
I am sorry, I should have read the PR description more closely. The linked issue talks about the union syntax in function definitions so I got confused. I am assuming that error messages coming from type expressions are coming in a separate PR? (ie. to address astral-sh/ty#449).
…rals * origin/main: [ty] Add hint that PEP 604 union syntax is only available in 3.10+ (#18192) Unify `Message` variants (#18051) [`airflow`] Update `AIR301` and `AIR311` with the latest Airflow implementations (#17985) [`airflow`] Move rules from `AIR312` to `AIR302` (#17940) [ty] Small LSP cleanups (#18201) [ty] Show related information in diagnostic (#17359) Default `src.root` to `['.', '<project_name>']` if the directory exists (#18141)
* main: [ty] Use first matching constructor overload when inferring specializations (#18204) [ty] Add hint that PEP 604 union syntax is only available in 3.10+ (#18192) Unify `Message` variants (#18051) [`airflow`] Update `AIR301` and `AIR311` with the latest Airflow implementations (#17985) [`airflow`] Move rules from `AIR312` to `AIR302` (#17940) [ty] Small LSP cleanups (#18201) [ty] Show related information in diagnostic (#17359) Default `src.root` to `['.', '<project_name>']` if the directory exists (#18141)
Summary
Add a new diagnostic hint if you try to use PEP 604
X | Yunion syntax in a non-type-expression before 3.10:closes astral-sh/ty#437
Test Plan
New snapshot test