Skip to content

fix Recognize explicitly constructed properties as such #3141#3144

Draft
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:3141
Draft

fix Recognize explicitly constructed properties as such #3141#3144
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:3141

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

Summary

Fixes #3141

The change normalizes property(...) assignments in class bodies into Pyrefly’s internal property representation instead of leaving them as generic descriptors.

That makes override checking treat p = property(lambda self: None) the same way as @property.

Test Plan

add test

@github-actions
Copy link
Copy Markdown

Diff from mypy_primer, showing the effect of this PR on open source code:

dulwich (https://github.com/dulwich/dulwich)
+ ERROR dulwich/bitmap.py:852:36-49: Cannot index into `BaseObjectStore` [bad-index]
+ ERROR dulwich/dumb.py:541:30-43: Argument `bytes` is not assignable to parameter `element` with type `ObjectID` in function `set.add` [bad-argument-type]
+ ERROR dulwich/gc.py:176:32-45: Argument `bytes` is not assignable to parameter `x` with type `ObjectID` in function `collections.deque.append` [bad-argument-type]
+ ERROR dulwich/gc.py:177:31-44: Argument `bytes` is not assignable to parameter `element` with type `ObjectID` in function `set.add` [bad-argument-type]
+ ERROR dulwich/object_filters.py:523:31-50: Argument `tuple[bytes, Literal[''], Literal[0]]` is not assignable to parameter `object` with type `tuple[ObjectID, str, int]` in function `list.append` [bad-argument-type]
+ ERROR dulwich/object_store.py:252:25-28: Cannot index into `ObjectContainer` [bad-index]
+ ERROR dulwich/object_store.py:307:29-32: Cannot index into `ObjectContainer` [bad-index]
+ ERROR dulwich/object_store.py:2826:63-71: Argument `list[bytes]` is not assignable to parameter `lst` with type `Iterable[ObjectID]` in function `_split_commits_and_tags` [bad-argument-type]
+ ERROR dulwich/object_store.py:2997:31-81: Argument `list[tuple[bytes, None, int, bool]]` is not assignable to parameter `entries` with type `Iterable[tuple[ObjectID, bytes | None, int | None, bool]]` in function `MissingObjectFinder.add_todo` [bad-argument-type]
+ ERROR dulwich/object_store.py:3636:9-23: `bytes` is not assignable to `ObjectID | RawObjectID` [bad-assignment]
+ ERROR dulwich/objectspec.py:512:41-48: Cannot index into `PackCapableObjectStore` [bad-index]
+ ERROR dulwich/porcelain/__init__.py:4712:44-47: Argument `bytes` is not assignable to parameter `sha` with type `ObjectID | RawObjectID` in function `dulwich.repo.BaseRepo.get_object` [bad-argument-type]
+ ERROR dulwich/porcelain/__init__.py:4900:40-69: Cannot set item in `dict[bytes, tuple[int, list[bytes], str]]` [unsupported-operation]
+ ERROR dulwich/porcelain/__init__.py:6232:21-25: Argument `bytes | None` is not assignable to parameter `data` with type `bytes` in function `dulwich.objects.Blob._set_data` [bad-argument-type]
+ ERROR dulwich/porcelain/__init__.py:6537:39-52: Argument `bytes` is not assignable to parameter `sha` with type `ObjectID | RawObjectID` in function `dulwich.repo.BaseRepo.get_object` [bad-argument-type]
+ ERROR dulwich/refs.py:1644:20-34: Object of class `bytes` has no attribute `get_object` [missing-attribute]
+ ERROR dulwich/repo.py:2415:44-47: Argument `bytes` is not assignable to parameter `sha` with type `ObjectID | RawObjectID` in function `BaseRepo.get_object` [bad-argument-type]
+ ERROR dulwich/walk.py:192:24-37: Argument `bytes` is not assignable to parameter `object_id` with type `ObjectID` in function `_CommitTimeQueue._push` [bad-argument-type]
+ ERROR dulwich/worktree.py:560:25-36: Argument `Sequence[ObjectID] | list[ObjectID]` is not assignable to parameter `value` with type `list[ObjectID]` in function `dulwich.objects.Commit._set_parents` [bad-argument-type]
+ ERROR dulwich/worktree.py:566:29-40: Argument `Sequence[ObjectID] | list[ObjectID]` is not assignable to parameter `value` with type `list[ObjectID]` in function `dulwich.objects.Commit._set_parents` [bad-argument-type]
+ ERROR dulwich/worktree.py:657:29-40: Argument `Sequence[ObjectID] | list[ObjectID]` is not assignable to parameter `value` with type `list[ObjectID]` in function `dulwich.objects.Commit._set_parents` [bad-argument-type]
+ ERROR dulwich/worktree.py:716:46-49: Argument `bytes` is not assignable to parameter `sha` with type `ObjectID | RawObjectID` in function `dulwich.repo.BaseRepo.get_object` [bad-argument-type]

jax (https://github.com/google/jax)
+ ERROR jax/_src/stages.py:424:3-12: Class member `Traced.args_info` overrides parent class `Stage` in an inconsistent manner [bad-override]

comtypes (https://github.com/enthought/comtypes)
+ ERROR comtypes/_post_coinit/unknwn.py:305:5-10: Class member `_compointer_base.value` overrides parent class `c_void_p` in an inconsistent manner [bad-override]

strawberry (https://github.com/strawberry-graphql/strawberry)
+ ERROR strawberry/experimental/pydantic/conversion.py:93:24-59: No matching overload found for function `getattr` called with arguments: (Unknown, str | None, None) [no-matching-overload]
+ ERROR strawberry/field_extensions/input_mutation.py:39:23-44: Cannot set item in `dict[str, Any]` [unsupported-operation]
+ ERROR strawberry/printer/printer.py:160:59-60: Argument `StrawberryField` is not assignable to parameter `obj` with type `HasGraphQLName` in function `strawberry.schema.name_converter.NameConverter.get_graphql_name` [bad-argument-type]
+ ERROR strawberry/schema/name_converter.py:104:38-43: Argument `StrawberryField` is not assignable to parameter `obj` with type `HasGraphQLName` in function `NameConverter.get_graphql_name` [bad-argument-type]
+ ERROR strawberry/schema/schema.py:482:64-69: Argument `StrawberryField` is not assignable to parameter `obj` with type `HasGraphQLName` in function `strawberry.schema.name_converter.NameConverter.get_graphql_name` [bad-argument-type]
+ ERROR strawberry/schema/schema_converter.py:381:64-69: Argument `StrawberryField` is not assignable to parameter `obj` with type `HasGraphQLName` in function `strawberry.schema.name_converter.NameConverter.get_graphql_name` [bad-argument-type]
+ ERROR strawberry/types/field.py:234:46-62: Argument `str | None` is not assignable to parameter with type `str` [bad-argument-type]
+ ERROR strawberry/types/info.py:135:16-39: Returned type `str | None` is not assignable to declared return type `str` [bad-return]
+ ERROR strawberry/types/type_resolver.py:145:51-68: Argument `str | None` is not assignable to parameter `field_name` with type `str` in function `strawberry.exceptions.private_strawberry_field.PrivateStrawberryFieldError.__init__` [bad-argument-type]
+ ERROR strawberry/types/type_resolver.py:154:21-38: Argument `str | None` is not assignable to parameter `field_name` with type `str` in function `strawberry.exceptions.FieldWithResolverAndDefaultValueError.__init__` [bad-argument-type]
+ ERROR strawberry/types/type_resolver.py:167:21-38: Argument `str | None` is not assignable to parameter `field_name` with type `str` in function `strawberry.exceptions.FieldWithResolverAndDefaultFactoryError.__init__` [bad-argument-type]

zope.interface (https://github.com/zopefoundation/zope.interface)
- ERROR src/zope/interface/declarations.py:217:9-18: Class member `_ImmutableDeclaration.__bases__` overrides parent class `Declaration` in an inconsistent manner [bad-override]
+ ERROR src/zope/interface/declarations.py:217:9-18: Class member `_ImmutableDeclaration.__bases__` overrides parent class `Declaration` in an inconsistent manner [bad-param-name-override]

pandas (https://github.com/pandas-dev/pandas)
+ ERROR pandas/core/generic.py:9741:43-58: Argument `ndarray[tuple[Any, ...], dtype[Unknown]]` is not assignable to parameter `values` with type `Sequence[Hashable]` in function `pandas.core.indexes.base.Index._set_names` [bad-argument-type]
- ERROR pandas/core/indexes/base.py:1853:16-25: Argument `Hashable | Sequence[Hashable] | list[Hashable | None] | Any | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
- ERROR pandas/core/indexes/base.py:1855:75-84: Argument `Hashable | Sequence[Hashable] | list[Hashable | None] | Any | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR pandas/core/indexes/base.py:1853:16-25: Argument `FrozenList | Hashable | Sequence[Hashable] | list[Hashable | None] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR pandas/core/indexes/base.py:1855:75-84: Argument `FrozenList | Hashable | Sequence[Hashable] | list[Hashable | None] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
- ERROR pandas/core/indexes/base.py:1861:16-25: Returned type `Hashable | Sequence[Hashable] | list[Hashable | None] | Any | None` is not assignable to declared return type `list[Hashable]` [bad-return]
+ ERROR pandas/core/indexes/base.py:1861:16-25: Returned type `FrozenList | Hashable | Sequence[Hashable] | list[Hashable | None] | None` is not assignable to declared return type `list[Hashable]` [bad-return]
+ ERROR pandas/core/indexes/multi.py:1750:5-10: Class member `MultiIndex.names` overrides parent class `Index` in an inconsistent manner [bad-param-name-override]
- ERROR pandas/core/indexes/multi.py:1812:16-21: Returned type `int | integer | Unknown` is not assignable to declared return type `int` [bad-return]
+ ERROR pandas/core/indexes/multi.py:1812:16-21: Returned type `int | integer | Any` is not assignable to declared return type `int` [bad-return]
- ERROR pandas/io/pytables.py:3411:49-54: Argument `ExtensionArray | Index | Series | ndarray | Any` is not assignable to parameter `value` with type `ExtensionArray | ndarray` in function `GenericFixed.write_array_empty` [bad-argument-type]
+ ERROR pandas/io/pytables.py:3411:49-54: Argument `ExtensionArray | Index | Series | ndarray` is not assignable to parameter `value` with type `ExtensionArray | ndarray` in function `GenericFixed.write_array_empty` [bad-argument-type]
- ERROR pandas/io/pytables.py:3452:45-50: Argument `ExtensionArray | Index | Series | ndarray | Any` is not assignable to parameter `value` with type `ExtensionArray | ndarray` in function `GenericFixed.write_array_empty` [bad-argument-type]
+ ERROR pandas/io/pytables.py:3452:45-50: Argument `ExtensionArray | Index | Series | ndarray` is not assignable to parameter `value` with type `ExtensionArray | ndarray` in function `GenericFixed.write_array_empty` [bad-argument-type]
+ ERROR pandas/tests/series/methods/test_matmul.py:32:46-56: Argument `ExtensionArray | ndarray` is not assignable to parameter `b` with type `_Buffer | _NestedSequence[bytes | complex | str] | _NestedSequence[_SupportsArray[dtype]] | _SupportsArray[dtype] | bytes | complex | str` in function `numpy._core.multiarray.dot` [bad-argument-type]
+ ERROR pandas/tests/series/methods/test_matmul.py:67:46-56: Argument `ExtensionArray | ndarray` is not assignable to parameter `b` with type `_Buffer | _NestedSequence[bytes | complex | str] | _NestedSequence[_SupportsArray[dtype]] | _SupportsArray[dtype] | bytes | complex | str` in function `numpy._core.multiarray.dot` [bad-argument-type]
+ ERROR pandas/tests/series/methods/test_matmul.py:73:46-56: Argument `ExtensionArray | ndarray` is not assignable to parameter `b` with type `_Buffer | _NestedSequence[bytes | complex | str] | _NestedSequence[_SupportsArray[dtype]] | _SupportsArray[dtype] | bytes | complex | str` in function `numpy._core.multiarray.dot` [bad-argument-type]

optuna (https://github.com/optuna/optuna)
+ ERROR optuna/importance/_ped_anova/evaluator.py:248:47-55: Argument `list[float] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR optuna/samplers/nsgaii/_elite_population_selection_strategy.py:84:24-44: Argument `list[float] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR optuna/samplers/nsgaii/_elite_population_selection_strategy.py:89:12-35: `None` is not subscriptable [unsupported-operation]
+ ERROR optuna/samplers/nsgaii/_elite_population_selection_strategy.py:89:39-63: `None` is not subscriptable [unsupported-operation]
+ ERROR optuna/samplers/nsgaii/_elite_population_selection_strategy.py:92:33-48: `None` is not subscriptable [unsupported-operation]
+ ERROR optuna/study/_multi_objective.py:31:16-24: Argument `list[float] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR optuna/study/_multi_objective.py:37:50-58: Argument `list[float] | None` is not assignable to parameter `iter1` with type `Iterable[float]` in function `zip.__new__` [bad-argument-type]
+ ERROR optuna/visualization/_param_importances.py:109:34-56: `None` is not subscriptable [unsupported-operation]
+ ERROR optuna/visualization/_pareto_front.py:336:12-24: Returned type `list[float] | None` is not assignable to declared return type `Sequence[float]` [bad-return]
+ ERROR optuna/visualization/_rank.py:273:68-80: Argument `list[float] | None` is not assignable to parameter `iterable` with type `Iterable[float]` in function `enumerate.__new__` [bad-argument-type]
+ ERROR tests/storages_tests/rdb_tests/test_storage.py:297:30-45: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/storages_tests/rdb_tests/test_storage.py:298:29-44: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/study_tests/test_study.py:1158:36-55: Argument `list[float] | None` is not assignable to parameter `values` with type `list[float]` in function `optuna.study.study.Study._log_completed_trial` [bad-argument-type]
+ ERROR tests/study_tests/test_study.py:1163:36-55: Argument `list[float] | None` is not assignable to parameter `values` with type `list[float]` in function `optuna.study.study.Study._log_completed_trial` [bad-argument-type]
+ ERROR tests/study_tests/test_study.py:1168:36-55: Argument `list[float] | None` is not assignable to parameter `values` with type `list[float]` in function `optuna.study.study.Study._log_completed_trial` [bad-argument-type]
+ ERROR tests/study_tests/test_study.py:1254:19-27: Argument `list[float] | None` is not assignable to parameter `iterable` with type `Iterable[float]` in function `tuple.__new__` [bad-argument-type]
+ ERROR tests/visualization_tests/test_contour.py:523:26-45: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_edf.py:172:33-52: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_parallel_coordinate.py:780:33-52: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_param_importances.py:273:26-46: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_param_importances.py:295:26-46: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_pareto_front.py:333:21-42: Argument `(t: FrozenTrial) -> float | Unknown` is not assignable to parameter `targets` with type `((FrozenTrial) -> Sequence[float]) | None` in function `optuna.visualization._pareto_front._get_pareto_front_info` [bad-argument-type]
+ ERROR tests/visualization_tests/test_pareto_front.py:333:31-42: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_pareto_front.py:351:32-43: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_pareto_front.py:351:45-56: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_pareto_front.py:351:58-69: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_rank.py:615:26-45: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_slice.py:403:26-45: `None` is not subscriptable [unsupported-operation]
+ ERROR tests/visualization_tests/test_utils.py:131:16-44: `None` is not subscriptable [unsupported-operation]

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

❌ 5 regression(s) | ✅ 1 improvement(s) | ➖ 1 neutral | 7 project(s) total | +76, -7 errors

5 regression(s) across dulwich, jax, strawberry, pandas, optuna. error kinds: bad-argument-type: bytes vs ObjectID, bad-index: Cannot index BaseObjectStore, bad-assignment and unsupported-operation. caused by get_property_class_field_type(). 1 improvement(s) across comtypes.

Project Verdict Changes Error Kinds Root Cause
dulwich ❌ Regression +22 bad-argument-type: bytes vs ObjectID get_property_class_field_type()
jax ❌ Regression +1 bad-override get_property_class_field_type()
comtypes ✅ Improvement +1 bad-override get_property_class_field_type()
strawberry ❌ Regression +11 bad-argument-type (StrawberryField vs HasGraphQLName protocol) get_property_class_field_type()
zope.interface ➖ Neutral +1, -1 bad-override, bad-param-name-override get_property_class_field_type()
pandas ❌ Regression +11, -6 NEW bad-argument-type (8 errors) get_property_class_field_type()
optuna ❌ Regression +29 unsupported-operation: None is not subscriptable get_property_class_field_type()
Detailed analysis

❌ Regression (5)

dulwich (+22)

bad-argument-type: bytes vs ObjectID: 15 new errors claiming bytes is not assignable to ObjectID. These appear to be cascade effects from changed property type resolution. None are flagged by mypy, 0/15 by pyright. The code has worked correctly before this PR.
bad-index: Cannot index BaseObjectStore: 4 errors claiming BaseObjectStore cannot be indexed. BaseObjectStore.__getitem__ is a fundamental method that definitely exists. This is a clear false positive, likely caused by the property normalization changing how __getitem__ is resolved.
bad-assignment and unsupported-operation: 2 errors that are cascade effects from the same root cause - changed property type resolution.
missing-attribute: bytes.get_object: 1 error where pyright also flags it. This could be a real issue where a variable is typed as bytes but should be a ref object. However, given the context of all other errors being false positives from the same PR change, this is likely also a cascade from incorrect type inference.

Overall: The PR intended to fix property constructor override checking (#3141), but as a side effect it appears to have changed type inference for existing property-based attributes in dulwich. The Cannot index into BaseObjectStore errors (4 instances) are clearly false positives since BaseObjectStore definitely supports __getitem__. The 15 bad-argument-type errors about bytes vs ObjectID are new cascade errors that weren't present before. Since 21/22 errors are pyrefly-only and affect fundamental operations of a well-tested project, these are regressions.

Per-category reasoning:

  • bad-argument-type: bytes vs ObjectID: 15 new errors claiming bytes is not assignable to ObjectID. These appear to be cascade effects from changed property type resolution. None are flagged by mypy, 0/15 by pyright. The code has worked correctly before this PR.
  • bad-index: Cannot index BaseObjectStore: 4 errors claiming BaseObjectStore cannot be indexed. BaseObjectStore.__getitem__ is a fundamental method that definitely exists. This is a clear false positive, likely caused by the property normalization changing how __getitem__ is resolved.
  • bad-assignment and unsupported-operation: 2 errors that are cascade effects from the same root cause - changed property type resolution.
  • missing-attribute: bytes.get_object: 1 error where pyright also flags it. This could be a real issue where a variable is typed as bytes but should be a ref object. However, given the context of all other errors being false positives from the same PR change, this is likely also a cascade from incorrect type inference.

Attribution: The change to get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs normalizes property(...) constructor calls into pyrefly's internal property representation. This likely changed how properties like Tag.object or BaseObjectStore.__getitem__ are resolved, altering their inferred return types and causing cascade type errors throughout dulwich. The .or_else(|| self.get_property_class_field_type(...)) insertion before other field type resolution methods may have changed the resolution order.

jax (+1)

The analysis is factually correct. Looking at the source code:

  1. Parent class Stage (line 332-333): Defines args_info: Any as a plain class-level annotation, making it a writable attribute typed as Any.

  2. Child class Traced (line 401, 424): Defines args_info = property(_traced_args_info) using the property(...) constructor form rather than the @property decorator.

  3. The override issue: A property is read-only by default (no setter), while the parent's args_info: Any is a writable attribute. Replacing a writable attribute with a read-only property is indeed a writability violation that constitutes a bad-override.

  4. PR context: The analysis correctly identifies that the PR normalizes property(...) to be treated like @property, which means pyrefly now recognizes args_info = property(_traced_args_info) as a property descriptor and flags the writability mismatch.

  5. False positive assessment: The analysis correctly notes that with Any as the parent type, neither mypy nor pyright would flag this, and that replacing attributes with properties in subclasses is a common Python pattern. The parent type is indeed Any (line 333).

  6. Other subclasses confirm the pattern: Lowered (line 534-538) declares args_info in __slots__ and assigns it in __init__, and Compiled (line 653-654) similarly has args_info in __slots__ - these are direct attribute assignments, not properties, so they wouldn't trigger the same issue. Only Traced uses property(...) form.

The analysis accurately describes the code, the type system behavior, and the nature of the false positive.

Attribution: The new get_property_class_field_type() method in pyrefly/lib/alt/class/class_field.rs now recognizes property(...) constructor calls as property descriptors. This causes args_info = property(_traced_args_info) in Traced to be treated as a property override of Stage.args_info: Any, triggering the bad-override check that didn't fire before when it was treated as a generic value.

strawberry (+11)

bad-argument-type (StrawberryField vs HasGraphQLName protocol): The property normalization likely changed the inferred type of python_name or graphql_name on StrawberryField, breaking protocol satisfaction for HasGraphQLName. All 8 are pyrefly-only false positives.
no-matching-overload (getattr): If python_name is now str | None instead of str, the getattr call with (Unknown, str | None, None) won't match overloads expecting str. Pyrefly-only false positive.
unsupported-operation (dict setitem): If python_name is str | None, using it as a key in dict[str, Any] would fail. But the value is always str at runtime. Pyrefly-only false positive caused by changed inference.
bad-return (str | None vs str): A property return type changed from str to str | None due to the new property normalization. Pyrefly-only false positive.

Overall: The PR's property normalization changes how property types are inferred for property() constructor calls. This appears to change the inferred return type of properties like python_name on StrawberryField from str to str | None, causing cascading type errors (protocol mismatches, overload failures, dict key issues, return type mismatches). All 11 errors are pyrefly-only, and the strawberry project is well-tested and works correctly. These are false positives introduced by the inference change.

Per-category reasoning:

  • bad-argument-type (StrawberryField vs HasGraphQLName protocol): The property normalization likely changed the inferred type of python_name or graphql_name on StrawberryField, breaking protocol satisfaction for HasGraphQLName. All 8 are pyrefly-only false positives.
  • no-matching-overload (getattr): If python_name is now str | None instead of str, the getattr call with (Unknown, str | None, None) won't match overloads expecting str. Pyrefly-only false positive.
  • unsupported-operation (dict setitem): If python_name is str | None, using it as a key in dict[str, Any] would fail. But the value is always str at runtime. Pyrefly-only false positive caused by changed inference.
  • bad-return (str | None vs str): A property return type changed from str to str | None due to the new property normalization. Pyrefly-only false positive.

Attribution: The change in pyrefly/lib/alt/class/class_field.rs adding get_property_class_field_type() and the .[or_else()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_field.rs) call at line 2145 changes how property(...) constructor calls are resolved. This likely changes the inferred types of properties on StrawberryField (e.g., python_name going from str to str | None), cascading into all 11 errors across the strawberry project.

pandas (+11, -6)

NEW bad-argument-type (8 errors): Property getter inference produces ndarray[..., dtype[Unknown]] for Index.names, which fails assignability checks. The Unknown indicates inference failure. All pyrefly-only — regression.
NEW bad-return (2 errors): Downstream of changed names type resolution. The return type union now includes FrozenList which pyrefly can't reconcile with list[Hashable]. Pyrefly-only — regression.
NEW bad-param-name-override (1 error): Spurious override check on MultiIndex.names vs Index.names property. Property getters don't have meaningful parameter names. Pyrefly-only — regression.
REMOVED bad-argument-type (4 errors): Old false positives from broader type inference removed — improvement, but replaced by new false positives.
REMOVED bad-return (2 errors): Old false positives removed — improvement, but replaced by new false positives.

Overall: The PR correctly fixes the property constructor recognition (issue #3141), but the property getter type inference produces Unknown types for pandas's Index.names property, leading to 11 new false positives (none co-reported by mypy/pyright) while removing only 6 old false positives. Net: 5 more false positives than before.

Per-category reasoning:

  • NEW bad-argument-type (8 errors): Property getter inference produces ndarray[..., dtype[Unknown]] for Index.names, which fails assignability checks. The Unknown indicates inference failure. All pyrefly-only — regression.
  • NEW bad-return (2 errors): Downstream of changed names type resolution. The return type union now includes FrozenList which pyrefly can't reconcile with list[Hashable]. Pyrefly-only — regression.
  • NEW bad-param-name-override (1 error): Spurious override check on MultiIndex.names vs Index.names property. Property getters don't have meaningful parameter names. Pyrefly-only — regression.
  • REMOVED bad-argument-type (4 errors): Old false positives from broader type inference removed — improvement, but replaced by new false positives.
  • REMOVED bad-return (2 errors): Old false positives removed — improvement, but replaced by new false positives.

Attribution: The change to get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs now recognizes property(...) constructor calls and converts them to internal property types. This changes how Index.names (defined as names = property(...)) is typed, producing ndarray[..., dtype[Unknown]] instead of the previous broader union that included Any. The Unknown in the inferred type indicates incomplete type resolution of the property getter, causing downstream bad-argument-type and bad-return errors.

optuna (+29)

unsupported-operation: None is not subscriptable: 18 errors from subscripting attributes typed as T | None (like FrozenTrial.values: Sequence[float] | None). These are cascade effects from the property resolution change. Neither mypy nor pyright flag these, suggesting the None case is handled differently or the property was previously unresolved.
bad-argument-type: ... | None not assignable to Sized/etc: 10 errors from passing T | None values to functions expecting non-None types (e.g., len(population[0].values) where values is Sequence[float] | None). Same cascade from property resolution change. 0/10 co-reported.
bad-return: list[float] | None not assignable to Sequence[float]: 1 error returning a value whose type now includes None due to property resolution. Same root cause. 0/1 co-reported.

Overall: The PR correctly improves property resolution for property(...) constructors, but this has a cascade effect: properties that were previously unresolved now expose their | None return types, triggering many downstream errors in code that doesn't null-check. Since 0/29 are co-reported by mypy/pyright, and these are all cascade effects from a single type resolution change in a well-tested project, these are effectively new false positives. The code is correct at runtime because these paths are only reached when values are populated.

Per-category reasoning:

  • unsupported-operation: None is not subscriptable: 18 errors from subscripting attributes typed as T | None (like FrozenTrial.values: Sequence[float] | None). These are cascade effects from the property resolution change. Neither mypy nor pyright flag these, suggesting the None case is handled differently or the property was previously unresolved.
  • bad-argument-type: ... | None not assignable to Sized/etc: 10 errors from passing T | None values to functions expecting non-None types (e.g., len(population[0].values) where values is Sequence[float] | None). Same cascade from property resolution change. 0/10 co-reported.
  • bad-return: list[float] | None not assignable to Sequence[float]: 1 error returning a value whose type now includes None due to property resolution. Same root cause. 0/1 co-reported.

Attribution: The addition of get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs now recognizes property(...) constructor calls and resolves them to proper property types. This likely changed how FrozenTrial.values (defined via property(...)) is resolved — from an unresolved/Unknown type to Sequence[float] | None — causing all 29 downstream errors when code accesses .values without None-checking.

✅ Improvement (1)

comtypes (+1)

This is a genuine type-level override inconsistency. _compointer_base extends c_void_p and overrides the value property. The parent c_void_p.value has type int | None, but the child redefines it as property(__get_value) where __get_value returns hints.Self (the instance itself). This violates the Liskov Substitution Principle — a consumer expecting c_void_p.value to return int | None would get a _compointer_base instance instead. Pyright also flags this (cross-check confirms). The PR's change to recognize property(...) constructor calls as proper properties is correct behavior — it now treats value = property(fget) the same as @property decorated methods, enabling proper override checking. This is a true positive that was previously missed because pyrefly didn't recognize the property() constructor form.
Attribution: The PR adds get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs which recognizes property(...) constructor calls (like value = property(__get_value, doc="""Return self.""") on line 305) and converts them into pyrefly's internal property representation. Before this change, value = property(...) was treated as a generic descriptor, so override checking didn't compare it against the parent's value property. Now that pyrefly correctly recognizes it as a property, it can perform proper override checking, which reveals the type inconsistency between _compointer_base.value (returns Self) and c_void_p.value (returns int | None).

➖ Neutral (1)

zope.interface (+1, -1)

This is a neutral change — the error at the same location (line 217:9-18) changed from bad-override to bad-param-name-override. The same __bases__ member is being flagged, but the nature of the detected issue has shifted. The human-readable error message text is identical ('Class member _ImmutableDeclaration.__bases__ overrides parent class Declaration in an inconsistent manner'), but the error codes represent different kinds of override violations: bad-override is a general override inconsistency, while bad-param-name-override specifically flags parameter name mismatches in method overrides. The PR's property normalization logic causes pyrefly to now recognize @property-decorated methods more precisely, which leads to a different, more specific override check — likely now comparing the setter method's parameter names against the parent class's __bases__ setter rather than flagging a general type-level override mismatch. The net effect is one error removed and one error added at the exact same location, so the total error count is unchanged.
Attribution: The PR adds get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs which normalizes property(...) assignments into pyrefly's internal property representation. This change means pyrefly now recognizes __bases__ defined via @property decorator more precisely, which causes the override check to produce a more specific error code (bad-param-name-override instead of the generic bad-override). Previously, the property might have been treated as a generic descriptor, producing a generic override error. Now that it's recognized as a proper property, the override checker can give a more specific diagnosis about parameter name inconsistency.

Suggested fixes

Summary: The new get_property_class_field_type() function in class_field.rs incorrectly infers property types when the getter returns a nullable type or when the property constructor form changes type resolution order, causing 74 pyrefly-only false positives across 5 projects.

1. In get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs, when a setter is present, the function returns the setter type (with property metadata) instead of the getter type. This is incorrect — the type of a property as seen by attribute access should be based on the getter's return type, not the setter. When there IS a setter, the function should still return the getter (but with the setter recorded in the PropertyMetadata). Change the if-let branch: instead of returning Some(setter), return Some(getter) with the getter's metadata updated to include both getter_without_property, setter_without_property, and has_deleter. Specifically, in the if let Some(mut setter) = setter branch, transform the getter's metadata (not the setter's) to have role=Getter with the setter field populated, and return Some(getter) instead of Some(setter).

Files: pyrefly/lib/alt/class/class_field.rs
Confidence: high
Affected projects: pandas, optuna, strawberry, dulwich, jax
Fixes: bad-argument-type, bad-index, bad-return, unsupported-operation, missing-attribute, no-matching-overload, bad-assignment, bad-param-name-override
When a property has both getter and setter (e.g., names = property(fget=_get_names, fset=_set_names)), the current code returns the setter's type. But property attribute access resolves to the getter's return type. Returning the setter means the property's type becomes the setter's signature rather than the getter's return type. This explains why pandas's Index.names gets Unknown types (the setter's parameter type rather than getter's return), why optuna's FrozenTrial.values becomes Sequence[float] | None (from the setter accepting None), and why strawberry's python_name becomes str | None. This single fix should eliminate the majority of the 74 regression errors across all 5 projects.

2. In get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs, the property_constructor_callable() method should handle the case where the getter argument is None (i.e., property(None, setter) or when fget is not provided). Currently if the getter is None, get_property_class_field_type returns None early (line let getter = self.property_constructor_arg(call, 0, "fget")?;), which means the entire property is not recognized. But more importantly, when the getter IS a lambda or simple function, the return type inference may not properly resolve through the lambda body. Add a fallback: if the inferred getter type produces Unknown return types, fall back to not recognizing this as a property (return None from get_property_class_field_type), letting the existing downstream resolution handle it.

Files: pyrefly/lib/alt/class/class_field.rs
Confidence: medium
Affected projects: dulwich, pandas
Fixes: bad-argument-type, bad-index, bad-assignment, unsupported-operation, missing-attribute
For cases like dulwich where property(...) constructors use complex getter functions, the type inference through property_constructor_callable may produce Unknown types. Adding a quality gate that checks whether the resolved getter type contains Unknown and falling back to the previous behavior would prevent regressions while still handling the well-typed cases. This would specifically help dulwich (22 errors) and pandas (11 errors) where the getter inference produces degraded types.

3. In get_property_class_field_type() in pyrefly/lib/alt/class/class_field.rs, the bad-param-name-override error for pandas (MultiIndex.names vs Index.names) and the bad-override for jax (Traced.args_info vs Stage.args_info) are caused by the property now being recognized as a function with parameter names. When comparing property overrides, parameter names of the getter's self parameter should not be checked. In the override checking logic, add a guard that skips bad-param-name-override checks when both parent and child are properties (i.e., have PropertyMetadata).

Files: pyrefly/lib/alt/class/class_field.rs
Confidence: medium
Affected projects: pandas, jax
Fixes: bad-param-name-override, bad-override
Property getters always take self as their only parameter. Checking parameter name consistency between a parent property getter and child property getter is meaningless since they both just have self. The bad-param-name-override for pandas's MultiIndex.names is a false positive from this. This would fix 1 error in pandas and potentially the jax bad-override as well.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (7 LLM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recognize explicitly constructed properties as such

1 participant