-
Notifications
You must be signed in to change notification settings - Fork 255
Disallow invalid string literals around | in type annotation
#2048
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
base: main
Are you sure you want to change the base?
Disallow invalid string literals around | in type annotation
#2048
Conversation
This diff prevents invalid union types from being constructed using string literals (as a forward reference) and a type (e.g. `"str" | int`). Pyrefly currently reports no error for these runtime errors. Note that union types can still be constructed if either side of the union operator is a type variable. Implementation-wise, when we parse a string literal in a type annotation into an expression, we use the `node_index` field to convey that the parsed name originated from a string literal. This is somewhat hacky but is probably the easiest way to achieve this goal without making major changes to the existing code. With this change, we are passing 2 more conformance test cases.
|
Diff from mypy_primer, showing the effect of this PR on open source code: dulwich (https://github.com/dulwich/dulwich)
+ ERROR dulwich/objects.py:478:26-65: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR dulwich/objects.py:591:26-65: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR dulwich/objects.py:622:12-51: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR dulwich/objects.py:663:12-51: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=dulwich/objects.py,line=478,col=26,endLine=478,endColumn=65,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=dulwich/objects.py,line=591,col=26,endLine=591,endColumn=65,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=dulwich/objects.py,line=622,col=12,endLine=622,endColumn=51,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=dulwich/objects.py,line=663,col=12,endLine=663,endColumn=51,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
prefect (https://github.com/PrefectHQ/prefect)
+ ERROR src/integrations/prefect-snowflake/prefect_snowflake/experimental/workers/spcs.py:204:21-48: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR src/integrations/prefect-snowflake/prefect_snowflake/experimental/workers/spcs.py:205:15-28: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=src/integrations/prefect-snowflake/prefect_snowflake/experimental/workers/spcs.py,line=204,col=21,endLine=204,endColumn=48,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=src/integrations/prefect-snowflake/prefect_snowflake/experimental/workers/spcs.py,line=205,col=15,endLine=205,endColumn=28,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
optuna (https://github.com/optuna/optuna)
+ ERROR optuna/samplers/_cmaes.py:459:10-27: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:77:69-88: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:123:10-42: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:155:10-44: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:309:10-42: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:341:10-44: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:374:10-34: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:445:10-34: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:517:10-46: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:550:10-38: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/storages/_rdb/models.py:568:44-69: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR optuna/visualization/matplotlib/_contour.py:241:6-25: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR tests/artifacts_tests/test_gcs.py:24:43-60: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=optuna/samplers/_cmaes.py,line=459,col=10,endLine=459,endColumn=27,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=77,col=69,endLine=77,endColumn=88,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=123,col=10,endLine=123,endColumn=42,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=155,col=10,endLine=155,endColumn=44,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=309,col=10,endLine=309,endColumn=42,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=341,col=10,endLine=341,endColumn=44,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=374,col=10,endLine=374,endColumn=34,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=445,col=10,endLine=445,endColumn=34,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=517,col=10,endLine=517,endColumn=46,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=550,col=10,endLine=550,endColumn=38,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/storages/_rdb/models.py,line=568,col=44,endLine=568,endColumn=69,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=optuna/visualization/matplotlib/_contour.py,line=241,col=6,endLine=241,endColumn=25,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=tests/artifacts_tests/test_gcs.py,line=24,col=43,endLine=24,endColumn=60,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
sphinx (https://github.com/sphinx-doc/sphinx)
+ ERROR sphinx/domains/c/_ast.py:31:5-32:17: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR sphinx/domains/c/_ast.py:31:5-33:16: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR sphinx/domains/c/_ast.py:31:5-34:22: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR sphinx/domains/c/_ast.py:31:5-35:16: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR sphinx/domains/c/_ast.py:31:5-36:24: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR sphinx/domains/c/_ast.py:31:5-37:17: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=32,endColumn=17,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=33,endColumn=16,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=34,endColumn=22,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=35,endColumn=16,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=36,endColumn=24,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=sphinx/domains/c/_ast.py,line=31,col=5,endLine=37,endColumn=17,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
schema_salad (https://github.com/common-workflow-language/schema_salad)
+ ERROR mypy-stubs/rdflib/graph.pyi:149:10-31: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR mypy-stubs/rdflib/paths.pyi:11:29-46: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ERROR mypy-stubs/rdflib/paths.pyi:14:34-51: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=mypy-stubs/rdflib/graph.pyi,line=149,col=10,endLine=149,endColumn=31,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=mypy-stubs/rdflib/paths.pyi,line=11,col=29,endLine=11,endColumn=46,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
+ ::error file=mypy-stubs/rdflib/paths.pyi,line=14,col=34,endLine=14,endColumn=51,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
psycopg (https://github.com/psycopg/psycopg)
+ ERROR psycopg_c/psycopg_c/_psycopg.pyi:36:17-34: Cannot construct union type on forward reference string literals [invalid-annotation]
+ ::error file=psycopg_c/psycopg_c/_psycopg.pyi,line=36,col=17,endLine=36,endColumn=34,title=Pyrefly invalid-annotation::Cannot construct union type on forward reference string literals
|
|
Converting to draft because I didn't realise there are new features in Python3.14 (https://docs.python.org/3/whatsnew/3.14.html#pep-649-pep-749-deferred-evaluation-of-annotations) that affects annotation behaviour. By sampling some of the errors from open source code, I realised that deferring annotation evaluation means that the runtime error might not arise unless the annotation is evaluated, e.g. However, I've also neglected handling of generic alias (?) which should work in Python3.13, which was revealed in the open source codebases. |
|
Thanks for taking this on! To minimize false positives, it looks like we need to avoid erroring in these cases:
|
Summary
This diff prevents invalid union types from being constructed using string literals (as a forward reference) and a type (e.g.
"str" | int). Pyrefly currently reports no error for these runtime errors.Note that union types can still be constructed if either side of the union operator is a type variable.
Implementation-wise, when we parse a string literal in a type annotation into an expression, we use the
node_indexfield to convey that the parsed name originated from a string literal. This is somewhat hacky but is probably the easiest way to achieve this goal without making major changes to the existing code.With this change, we are passing 2 more conformance test cases.
Test Plan
python test.py