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

Emit warning for invalid use of quoted types in union syntax #18183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brianschubert
Copy link
Collaborator

Using quoted types in PEP 604 union syntax can be problematic, since at runtime, using | between a type and string produces an error:

>>> x: int | "str"
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    x: int | "str"
       ~~~~^~~~~~~
TypeError: unsupported operand type(s) for |: 'type' and 'str'


>>> U = int | float
>>> x: U | "str"
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    x: U | "str"
       ~~^~~~~~~
TypeError: unsupported operand type(s) for |: 'types.UnionType' and 'str'

The exception is when the type is a TypeVar or a _UnionGenericAlias, which can both be safely |'d with strings:

>>> T = TypeVar("T")
>>> U = T | int

>>> T | "str"
typing.Union[~T, ForwardRef('str')]
>>> U | "str"
typing.Union[~T, int, ForwardRef('str')]

This PR makes mypy emit a warning for the former cases (where using | with a string will produce a runtime error) while permitting the latter cases (where using | with a string is safe).

A few notes about the implementation:

  • No errors are emitted inside stub files (where quoted types are redundant but harmless).
  • No errors are emitted for annotations inside files that use from __future__ import annotations. Like in stubs, quoted types in these cases are redundant but mostly harmless (aside from issues with runtime inspection). It might be a good idea to emit errors for these cases too (pyright does), but I wasn't sure if the benefit is enough to justify the churn. Without this exception, this PR has ~20 mypy_primer hits across 4 projects.
  • I've verified that all test cases that emit errors do indeed raise exceptions at runtime, and all cases that don't, don't. (checked with 3.10.12 and 3.13.0).

@brianschubert
Copy link
Collaborator Author

Conformance results diff for this PR (improves conformance in annotations_forward_refs)

:...skipping...
diff --git a/conformance/results/mypy/annotations_forward_refs.toml b/conformance/results/mypy/annotations_forward_refs.toml
index bee7ebb..6763a18 100644
--- a/conformance/results/mypy/annotations_forward_refs.toml
+++ b/conformance/results/mypy/annotations_forward_refs.toml
@@ -5,6 +5,8 @@ Does not report error for use of quoted type with "|" operator (runtime error).
 Incorrectly generates error for quoted type defined in class scope.
 """
 output = """
+annotations_forward_refs.py:24: error: X | Y syntax for unions cannot use quoted operands; use quotes around the entire expression instead  [misc]
+annotations_forward_refs.py:25: error: X | Y syntax for unions cannot use quoted operands; use quotes around the entire expression instead  [misc]
 annotations_forward_refs.py:41: error: Invalid type comment or annotation  [valid-type]
 annotations_forward_refs.py:42: error: Invalid type comment or annotation  [valid-type]
 annotations_forward_refs.py:43: error: Invalid type comment or annotation  [valid-type]
@@ -33,8 +35,6 @@ conformance_automated = "Fail"
 errors_diff = """
 Line 22: Expected 1 errors
 Line 23: Expected 1 errors
-Line 24: Expected 1 errors
-Line 25: Expected 1 errors
 Line 66: Expected 1 errors
 Line 87: Unexpected errors ['annotations_forward_refs.py:87: error: Function "annotations_forward_refs.ClassD.int" is not valid as a type  [valid-type]']
 Line 96: Unexpected errors ['annotations_forward_refs.py:96: error: Expression is of type int?, not "int"  [assert-type]']
diff --git a/conformance/results/mypy/version.toml b/conformance/results/mypy/version.toml
index 51d97b4..b1035e1 100644
--- a/conformance/results/mypy/version.toml
+++ b/conformance/results/mypy/version.toml
@@ -1,2 +1,2 @@
-version = "mypy 1.14.0+dev.499adaed8adbded1a180e30d071438fef81779ec"
-test_duration = 2.4
+version = "mypy 1.14.0+dev.ce34db689d8a6cab6f540a2c38ecc32952534b0a"
+test_duration = 4.6

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant