Repository navigation
fix cached_property overriding unannotated class variable #3062 - #3564
asukaminato0721 wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D107101300. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5901c27 to
f8058dd
Compare
f8058dd to
42aad50
Compare
This comment has been minimized.
This comment has been minimized.
stroxler
left a comment
There was a problem hiding this comment.
Thanks! This looks like it's probably headed in a good direction, I like the mypy_primer delta.
Based on AI review, I think we need to clear up some more behaviors before we can merge:
This PR is bigger than it needs to be
Maybe split the unrelated signature-help change into a separate PR. Its tests should include whitespace before an existing argument: currently every expected index also equals arguments.len(), so the tests pass even if token counting returns None.
Literal widening only happens in the compatibility check
The helper widens an inferred Literal[True] return to bool, but actual cached-property reads and assignment checking retain Literal[True]. This accepts substitutions that violate the attribute’s reported type:
- A writable protocol declares
x: bool. - A class implements
xas a cached property returning unannotatedTrue. - A function taking the protocol assigns
Falsetox. - Pyrefly still accepts passing the resulting value to a function requiring
Literal[True].
Verified: the patched binary reports zero errors under default; CPython fails the corresponding assertion. The unpatched binary rejects the protocol conversion. The inheritance case has the same inconsistency: writing False through the base type is accepted, while writing it directly through the subclass is rejected.
Fix: establish one cached-property value type used consistently for reads, writes, and compatibility checks. Add both protocol and inherited-write regressions.
Read-only failures get a mutable-attribute diagnostic
Overriding a frozen dataclass’s x: int with a cached property returning str now reports bad-override-mutable-attribute, despite the parent attribute being read-only. This matters for configurations that control those error kinds separately. Add a negative read-only-parent test and preserve the appropriate diagnostic classification.
|
Diff from mypy_primer, showing the effect of this PR on open source code: ============================================================
SUMMARY
============================================================
Total: +12 new errors, -74 fixed errors
By preset: +5/-74 (default), +7/-74 (strict)
Projects with changes (4):
openlibrary: +1 -0
jax: +2 -0
django-stubs: +5 -74
core: +4 -0
============================================================
FULL DIFF DETAILS
------------------------------------------------------------
openlibrary (https://github.com/internetarchive/openlibrary)
+ ERROR openlibrary/core/lists/model.py:662:17-41: Unused `# type: ignore` comment [unused-type-ignore]
jax (https://github.com/google/jax)
+ ERROR jax/_src/mesh.py:363:27-58: Unused `# pyrefly: ignore` comment for code(s): bad-override [unused-ignore]
+ ERROR jax/_src/mesh.py:551:27-58: Unused `# pyrefly: ignore` comment for code(s): bad-override [unused-ignore]
django-stubs (https://github.com/typeddjango/django-stubs)
- ERROR django-stubs/contrib/gis/db/backends/mysql/features.pyi:17:9-45: Class member `DatabaseFeatures.supports_geometry_field_unique_index` overrides parent class `BaseSpatialFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:17:9-16: Class member `MySQLOperations.mariadb` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:20:9-14: Class member `MySQLOperations.mysql` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:23:9-15: Class member `MySQLOperations.select` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:26:9-18: Class member `MySQLOperations.from_text` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
+ ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:26:9-18: Class member `MySQLOperations.from_text` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override-mutable-attribute]
- ERROR django-stubs/contrib/gis/db/backends/mysql/operations.pyi:34:9-30: Class member `MySQLOperations.unsupported_functions` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/oracle/introspection.pyi:10:9-27: Class member `OracleIntrospection.data_types_reverse` overrides parent class `DatabaseIntrospection` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/postgis/operations.pyi:52:9-23: Class member `PostGISOperations.function_names` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/postgis/operations.pyi:55:9-24: Class member `PostGISOperations.spatial_version` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/spatialite/features.pyi:10:9-31: Class member `DatabaseFeatures.supports_area_geodetic` overrides parent class `BaseSpatialFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/spatialite/operations.pyi:26:9-30: Class member `SpatiaLiteOperations.unsupported_functions` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/gis/db/backends/spatialite/operations.pyi:29:9-24: Class member `SpatiaLiteOperations.spatial_version` overrides parent class `BaseSpatialOperations` in an inconsistent manner [bad-override]
- ERROR django-stubs/contrib/postgres/indexes.pyi:16:9-24: Class member `PostgresIndex.max_name_length` overrides parent class `Index` in an inconsistent manner [bad-override]
- ERROR django-stubs/core/handlers/asgi.pyi:35:9-12: Class member `ASGIRequest.GET` overrides parent class `HttpRequest` in an inconsistent manner [bad-override]
- ERROR django-stubs/core/handlers/asgi.pyi:42:9-16: Class member `ASGIRequest.COOKIES` overrides parent class `HttpRequest` in an inconsistent manner [bad-override]
- ERROR django-stubs/core/handlers/wsgi.pyi:27:9-12: Class member `WSGIRequest.GET` overrides parent class `HttpRequest` in an inconsistent manner [bad-override]
- ERROR django-stubs/core/handlers/wsgi.pyi:30:9-16: Class member `WSGIRequest.COOKIES` overrides parent class `HttpRequest` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:38:9-33: Class member `DatabaseFeatures.minimum_database_version` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
+ ERROR django-stubs/db/backends/mysql/features.pyi:38:9-33: Class member `DatabaseFeatures.minimum_database_version` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override-mutable-attribute]
- ERROR django-stubs/db/backends/mysql/features.pyi:41:9-24: Class member `DatabaseFeatures.test_collations` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
+ ERROR django-stubs/db/backends/mysql/features.pyi:41:9-24: Class member `DatabaseFeatures.test_collations` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override-mutable-attribute]
- ERROR django-stubs/db/backends/mysql/features.pyi:44:9-26: Class member `DatabaseFeatures.django_test_skips` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:47:9-25: Class member `DatabaseFeatures.allows_auto_pk_0` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:50:9-31: Class member `DatabaseFeatures.update_can_self_select` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:53:9-36: Class member `DatabaseFeatures.can_introspect_foreign_keys` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:56:9-33: Class member `DatabaseFeatures.introspected_field_types` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:59:9-39: Class member `DatabaseFeatures.can_return_columns_from_insert` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:65:9-30: Class member `DatabaseFeatures.has_zoneinfo_database` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:70:9-42: Class member `DatabaseFeatures.supports_column_check_constraints` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:76:9-41: Class member `DatabaseFeatures.can_introspect_check_constraints` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:79:9-33: Class member `DatabaseFeatures.has_select_for_update_of` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:82:9-34: Class member `DatabaseFeatures.supported_explain_formats` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:88:9-32: Class member `DatabaseFeatures.ignores_table_name_case` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:91:9-37: Class member `DatabaseFeatures.supports_default_in_lead_lag` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:97:9-34: Class member `DatabaseFeatures.can_introspect_json_field` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:100:9-39: Class member `DatabaseFeatures.supports_index_column_ordering` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:103:9-36: Class member `DatabaseFeatures.supports_expression_indexes` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:106:9-37: Class member `DatabaseFeatures.supports_select_intersection` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:112:9-37: Class member `DatabaseFeatures.supports_expression_defaults` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:115:9-30: Class member `DatabaseFeatures.has_native_uuid_field` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:118:9-37: Class member `DatabaseFeatures.allows_group_by_selected_pks` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:121:9-27: Class member `DatabaseFeatures.supports_any_value` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:124:9-32: Class member `DatabaseFeatures.supports_uuid4_function` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/mysql/features.pyi:130:9-32: Class member `DatabaseFeatures.supports_uuid7_function` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:54:9-40: Class member `DatabaseFeatures.supports_json_negative_indexing` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:57:9-26: Class member `DatabaseFeatures.django_test_skips` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:60:9-33: Class member `DatabaseFeatures.introspected_field_types` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:63:9-24: Class member `DatabaseFeatures.test_collations` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:66:9-40: Class member `DatabaseFeatures.supports_collation_on_charfield` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:69:9-42: Class member `DatabaseFeatures.supports_primitives_in_json_field` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:72:9-33: Class member `DatabaseFeatures.supports_frame_exclusion` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:75:9-40: Class member `DatabaseFeatures.supports_comparing_boolean_expr` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:80:9-27: Class member `DatabaseFeatures.bare_select_suffix` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:83:9-31: Class member `DatabaseFeatures.supports_tuple_lookups` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:86:9-32: Class member `DatabaseFeatures.supports_uuid4_function` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:89:9-43: Class member `DatabaseFeatures.supports_uuid4_function_in_default` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:92:9-42: Class member `DatabaseFeatures.supports_stored_generated_columns` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/oracle/features.pyi:95:9-34: Class member `DatabaseFeatures.supports_bit_aggregations` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:51:9-32: Class member `DatabaseFeatures.supports_uuid7_function` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:54:9-38: Class member `DatabaseFeatures.supports_uuid7_function_shift` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:57:9-26: Class member `DatabaseFeatures.django_test_skips` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:60:9-38: Class member `DatabaseFeatures.django_test_expected_failures` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:65:9-25: Class member `DatabaseFeatures.max_query_params` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/postgresql/features.pyi:68:9-52: Class member `DatabaseFeatures.prohibits_null_characters_in_text_exception` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
+ ERROR django-stubs/db/backends/postgresql/features.pyi:68:9-52: Class member `DatabaseFeatures.prohibits_null_characters_in_text_exception` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override-mutable-attribute]
- ERROR django-stubs/db/backends/postgresql/features.pyi:71:9-33: Class member `DatabaseFeatures.introspected_field_types` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/sqlite3/features.pyi:14:9-32: Class member `DatabaseFeatures.supports_uuid7_function` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/sqlite3/features.pyi:17:9-26: Class member `DatabaseFeatures.django_test_skips` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/sqlite3/features.pyi:20:9-33: Class member `DatabaseFeatures.introspected_field_types` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/sqlite3/features.pyi:26:9-28: Class member `DatabaseFeatures.supports_json_field` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/backends/sqlite3/features.pyi:35:9-39: Class member `DatabaseFeatures.can_return_columns_from_insert` overrides parent class `BaseDatabaseFeatures` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/models/expressions.pyi:136:9-24: Class member `CombinedExpression.allowed_default` overrides parent class `Expression` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/models/expressions.pyi:195:9-24: Class member `Func.allowed_default` overrides parent class `Expression` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/models/expressions.pyi:294:9-24: Class member `When.allowed_default` overrides parent class `Expression` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/models/expressions.pyi:307:9-24: Class member `Case.allowed_default` overrides parent class `Expression` in an inconsistent manner [bad-override]
- ERROR django-stubs/db/models/fields/related.pyi:73:9-22: Class member `RelatedField.related_model` overrides parent class `Field` in an inconsistent manner [bad-override]
+ ERROR django-stubs/db/models/fields/related.pyi:73:9-22: Class member `RelatedField.related_model` overrides parent class `Field` in an inconsistent manner [bad-override-mutable-attribute]
- ERROR django-stubs/db/models/lookups.pyi:48:9-24: Class member `Lookup.allowed_default` overrides parent class `Expression` in an inconsistent manner [bad-override]
core (https://github.com/home-assistant/core)
+ ERROR homeassistant/components/recorder/models/state.py:70:42-66: Unused `# type: ignore` comment [unused-type-ignore]
+ ERROR homeassistant/components/recorder/models/state.py:78:43-67: Unused `# type: ignore` comment [unused-type-ignore]
+ ERROR homeassistant/components/recorder/models/state.py:86:42-66: Unused `# type: ignore` comment [unused-type-ignore]
+ ERROR homeassistant/components/recorder/models/state.py:94:49-73: Unused `# type: ignore` comment [unused-type-ignore] |
stroxler
left a comment
There was a problem hiding this comment.
Great! I think this is almost ready to merge.
But I just realized it's another too-old PR, the import diff is stale so I can't easily finish my review.
Could you close this and reopen? Assign me to the fresh PR when you do While you're at it, there are two more concerns caught in a fresh review that should be pretty easy to fix:
Overload comparison accepts unsafe protocol writes
Compatibility unions all overload returns, while reads and direct writes select overloads using the receiver. A cached property on C[int] can therefore satisfy a writable protocol with x: int | str, despite its actual attribute type being int.
Verified: the unpatched build rejects the conversion; the patched build reports zero errors; writing a string through the protocol then fails an integer assertion in CPython. Direct string assignment remains rejected.
Fix: use receiver-aware getter resolution for compatibility, with both negative broad-protocol and positive narrow-protocol tests.
Literal widening rejects valid inner decorators
Widening occurs before inner decorators are checked. With @cached_property outside a decorator requiring a function returning Literal[True], an unannotated return True is prematurely presented as bool.
Verified: unpatched Pyrefly, mypy, pyright, and CPython accept the example; patched Pyrefly rejects it.
Fix: preserve the original return while checking inner decorators, then apply cached-value normalization at the resulting getter/value boundary.
|
I'm going to leave this open since it's mostly and I don't want to lose track of it - again, to get this merged the quickest path is for you to close, reopen (with adjustments for the latest round of review), and assign me. I think it's about ready, the imported diff is just too old for me to work with |
Summary
Fixes #3062
cached properties are now treated as writable for override consistency, using the getter return type and promoting inferred literal returns like return True to match class-variable inference.
Test Plan
add test