Consider property access from class objects in checkmember.py #18504

sterliakov · 2025-01-21T23:18:31Z

Improves behavior in #16426 and #6185 (recognizes .fset, but still uses optional definition from typeshed).

Improves behavior in #14684, #5936 and #1465 by adding an explicit "not supported" message.

This version ignores any generic parameters (e.g. functools.cached_property is generic in the return type) - IMO that should be sufficient to avoid running all related checks here.

sterliakov · 2025-01-22T02:05:07Z

Primer diff is great:

pandas: doc expects Callables or strings, not properties, and works only by coincidence - that's a true positive. unused-ignore was a false positive ("Callable[[IndexOpsMixin], bool]" has no attribute "fget")
ppb-vector: it was an ignored false positive, accessing fget
ibis: now both errors are correct, they use @property @abstractmethod in an ABC and later override it with a class attribute.

A5rocks

Note that this also fixes #8085.

... But this plays weirdly with #1465 and #5936. I'm not exactly sure whether this is a step in the right direction or not for those. (it removes errors but IDK if it's helping functionality wise because I haven't tested their test cases)

A5rocks · 2025-01-24T05:09:37Z

mypy/checkmember.py

@@ -1110,6 +1112,19 @@ def analyze_class_attribute_access(
            #     C[int].x -> int
            t = erase_typevars(expand_type_by_instance(t, isuper), {tv.id for tv in def_vars})

+        if is_decorated:


Maybe filter before hand?

Suggested change

if is_decorated:

if is_decorated and node.node.func.is_property:

Not much of a difference though, just means there's no need for handling the no property decorator case.

A5rocks · 2025-01-24T05:14:01Z

test-data/unit/check-classes.test

@@ -1665,6 +1665,36 @@ a.f = a.f # E: Property "f" defined in "A" is read-only
 a.f.x # E: "int" has no attribute "x"
 [builtins fixtures/property.pyi]

+[case testPropertyAccessOnClass]


Could you add a reveal_type within the class?

sterliakov · 2025-01-24T17:40:58Z

Thanks for the links!

The first one is the canonical issue for that problem, IDK how could I have missed it. Added.

#1465 won't be fixed here, but the error message in #5936 becomes extremely misleading indeed:

main:10: error: "property" has no attribute "setter"

I'll try to have a look into that.

…y-on-class-checkmember

sterliakov · 2025-01-28T23:01:44Z

All diffs are good. The last one (homeassistant/core) can be trivially fixed with assert and comes from typeshed definition of property (fget is Callable[[Any], Any] | None), and it is certainly no worse than it used to be.

However, this is still a usability issue: if we have a property constructed with @property decorator, it certainly does have a non-None getter. It might be reasonable to account for such behaviour, but I'd rather keep this for a separate PR.

sterliakov · 2025-01-29T05:52:01Z

The extra note is not produced in presence of Any/untyped defs, I'll check it out tomorrow.

…emand

…y-on-class-checkmember

sterliakov · 2025-01-30T15:51:08Z

mypy/checker.py

@@ -5285,7 +5339,7 @@ def visit_decorator_inner(self, e: Decorator, allow_empty: bool = False) -> None
        # For overloaded functions we already checked override for overload as a whole.
        if allow_empty:
            return
-        if e.func.info and not e.func.is_dynamic() and not e.is_overload:


This is a major change, though. IMO the updated behavior is correct: even if the function is untyped, it still shouldn't be allowed to have invalid @override attached. We won't check its superclass compatibility unless --check-untyped-defs is set (check_override_compatibility flag down the road).

github-actions · 2025-01-30T15:53:20Z

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

psycopg (https://github.com/psycopg/psycopg)
+ tests/scripts/spiketest.py:110: error: Signature of "connect" incompatible with supertype "Connection"  [override]
+ tests/scripts/spiketest.py:110: note:      Superclass:
+ tests/scripts/spiketest.py:110: note:          @classmethod
+ tests/scripts/spiketest.py:110: note:          def connect(cls, conninfo: str = ..., *, autocommit: bool = ..., prepare_threshold: int | None = ..., context: AdaptContext | None = ..., row_factory: RowFactory[Row] | None = ..., cursor_factory: type[Cursor[Row]] | None = ..., **kwargs: str | int | None) -> DelayedConnection[Row]
+ tests/scripts/spiketest.py:110: note:      Subclass:
+ tests/scripts/spiketest.py:110: note:          @classmethod
+ tests/scripts/spiketest.py:110: note:          def connect(cls, conninfo: Any, conn_delay: Any = ..., **kwargs: Any) -> Any

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/groupby/indexing.py:248: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/series.py:358: error: Unused "type: ignore" comment  [unused-ignore]
+ pandas/core/indexing.py:1220: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexing.py:1571: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexing.py:2525: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexing.py:2569: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexes/datetimelike.py:138: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexes/datetimelike.py:155: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexes/datetimelike.py:526: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/core/indexes/base.py:4956: error: Argument 1 to "doc" has incompatible type "property"; expected "str | Callable[..., Any] | None"  [arg-type]
+ pandas/_testing/__init__.py:352: error: Cannot override writeable attribute with read-only property  [override]

streamlit (https://github.com/streamlit/streamlit)
+ lib/tests/streamlit/watcher/test_data/misbehaved_module.py: note: In class "_MisbehavedModule":
+ lib/tests/streamlit/watcher/test_data/misbehaved_module.py:20:5: error: Cannot override writeable attribute with read-only property  [override]

spark (https://github.com/apache/spark)
+ python/pyspark/instrumentation_utils.py:83: error: Unused "type: ignore" comment  [unused-ignore]

xarray (https://github.com/pydata/xarray)
+ xarray/core/variable.py:2658: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/core/variable.py: note: In class "IndexVariable":
+ xarray/core/variable.py:2659: error: Property setter overrides are not supported.  [misc]
+ xarray/core/variable.py:2659: note: Consider overriding getter explicitly with super() call.
+ xarray/core/variable.py: note: At top level:
+ xarray/core/variable.py:2665: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/core/variable.py: note: In class "IndexVariable":
+ xarray/core/variable.py:2666: error: Property setter overrides are not supported.  [misc]
+ xarray/core/variable.py:2666: note: Consider overriding getter explicitly with super() call.

pytest (https://github.com/pytest-dev/pytest)
+ testing/test_collection.py:1618: error: Signature of "from_parent" incompatible with supertype "Class"  [override]
+ testing/test_collection.py:1618: note:      Superclass:
+ testing/test_collection.py:1618: note:          @classmethod
+ testing/test_collection.py:1618: note:          def from_parent(cls, parent: Any, *, name: Any, obj: Any = ..., **kw: Any) -> MyCollector
+ testing/test_collection.py:1618: note:      Subclass:
+ testing/test_collection.py:1618: note:          @classmethod
+ testing/test_collection.py:1618: note:          def from_parent(cls, parent: Any, *, name: Any, x: Any) -> Any
+ testing/test_collection.py:1618: error: Signature of "from_parent" incompatible with supertype "Node"  [override]
+ testing/test_collection.py:1618: note:      Superclass:
+ testing/test_collection.py:1618: note:          @classmethod
+ testing/test_collection.py:1618: note:          def from_parent(cls, parent: Node, **kw: Any) -> MyCollector
+ testing/test_collection.py:1618: note:      Subclass:
+ testing/test_collection.py:1618: note:          @classmethod
+ testing/test_collection.py:1618: note:          def from_parent(cls, parent: Any, *, name: Any, x: Any) -> Any

ppb-vector (https://github.com/ppb/ppb-vector)
+ tests/utils.py:86: error: Unused "type: ignore" comment  [unused-ignore]

pwndbg (https://github.com/pwndbg/pwndbg)
- pwndbg/aglib/heap/structs.py:175: error: Invalid index type "Any | type[Any] | _CField[Any, Any, Any] | overloaded function" for "dict[type[object], Type]"; expected type "type[object]"  [index]
+ pwndbg/aglib/heap/structs.py:175: error: Invalid index type "Any | type[Any] | _CField[Any, Any, Any] | property" for "dict[type[object], Type]"; expected type "type[object]"  [index]
- pwndbg/aglib/heap/structs.py:177: error: Argument 1 to "array" of "Type" has incompatible type "Any | _CField[Any, Any, Any] | overloaded function"; expected "int"  [arg-type]
+ pwndbg/aglib/heap/structs.py:177: error: Argument 1 to "array" of "Type" has incompatible type "Any | _CField[Any, Any, Any] | property"; expected "int"  [arg-type]
- pwndbg/aglib/heap/structs.py:206: error: Invalid index type "Any | type[Any] | _CField[Any, Any, Any] | overloaded function" for "dict[type[object], Type]"; expected type "type[object]"  [index]
+ pwndbg/aglib/heap/structs.py:206: error: Invalid index type "Any | type[Any] | _CField[Any, Any, Any] | property" for "dict[type[object], Type]"; expected type "type[object]"  [index]
- pwndbg/aglib/heap/structs.py:207: error: Argument 1 to "array" of "Type" has incompatible type "Any | _CField[Any, Any, Any] | overloaded function"; expected "int"  [arg-type]
+ pwndbg/aglib/heap/structs.py:207: error: Argument 1 to "array" of "Type" has incompatible type "Any | _CField[Any, Any, Any] | property"; expected "int"  [arg-type]

discord.py (https://github.com/Rapptz/discord.py)
- discord/member.py:492: error: "Callable[[Member], Status]" has no attribute "setter"  [attr-defined]
+ discord/member.py:492: note: Property setter and deleter must be adjacent to the getter.

ibis (https://github.com/ibis-project/ibis)
- ibis/expr/operations/udf.py:155: error: Argument 2 to "type" has incompatible type "tuple[Callable[[_UDF], type[B]]]"; expected "tuple[type, ...]"  [arg-type]
+ ibis/expr/operations/udf.py:155: error: Argument 2 to "type" has incompatible type "tuple[property]"; expected "tuple[type, ...]"  [arg-type]
- ibis/backends/tests/base.py:180: error: "Callable[[BackendTest], Iterable[str]]" has no attribute "__iter__" (not iterable)  [attr-defined]
+ ibis/backends/tests/base.py:180: error: "property" has no attribute "__iter__" (not iterable)  [attr-defined]

tornado (https://github.com/tornadoweb/tornado)
+ tornado/test/auth_test.py:78: error: Signature of "_oauth_get_user_future" incompatible with supertype "OAuthMixin"  [override]
+ tornado/test/auth_test.py:78: note:      Superclass:
+ tornado/test/auth_test.py:78: note:          def _oauth_get_user_future(self, access_token: dict[str, Any]) -> Coroutine[Any, Any, dict[str, Any]]
+ tornado/test/auth_test.py:78: note:      Subclass:
+ tornado/test/auth_test.py:78: note:          def _oauth_get_user_future(*Any, **Any) -> Future[Any]
+ tornado/test/web_test.py:1514: error: Signature of "make_static_url" incompatible with supertype "StaticFileHandler"  [override]
+ tornado/test/web_test.py:1514: note:      Superclass:
+ tornado/test/web_test.py:1514: note:          @classmethod
+ tornado/test/web_test.py:1514: note:          def make_static_url(cls, settings: dict[str, Any], path: str, include_version: bool = ...) -> str
+ tornado/test/web_test.py:1514: note:      Subclass:
+ tornado/test/web_test.py:1514: note:          @classmethod
+ tornado/test/web_test.py:1514: note:          def make_static_url(cls, settings: Any, path: Any) -> Any
+ tornado/test/web_test.py:1514: note:      Superclass:
+ tornado/test/web_test.py:1514: note:          @classmethod
+ tornado/test/web_test.py:1514: note:          def make_static_url(cls, settings: dict[str, Any], path: str, include_version: bool = ...) -> str
+ tornado/test/web_test.py:1514: note:      Subclass:
+ tornado/test/web_test.py:1514: note:          @classmethod
+ tornado/test/web_test.py:1514: note:          def make_static_url(cls, settings: Any, path: Any) -> Any

pandera (https://github.com/pandera-dev/pandera)
- pandera/api/pandas/components.py:434: error: Unused "type: ignore" comment  [unused-ignore]

pytest-robotframework (https://github.com/detachhead/pytest-robotframework)
+ pytest_robotframework/_internal/pytest/robot_file_support.py:133: error: Method "setup" is marked as an override, but no base method was found with this name  [misc]
+ pytest_robotframework/_internal/pytest/robot_file_support.py:137: error: Method "runtest" is marked as an override, but no base method was found with this name  [misc]
+ pytest_robotframework/_internal/pytest/robot_file_support.py:164: error: Method "teardown" is marked as an override, but no base method was found with this name  [misc]

core (https://github.com/home-assistant/core)
+ homeassistant/components/mqtt/sensor.py:352: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/mqtt/sensor.py:352: error: "None" not callable  [misc]
+ homeassistant/components/mqtt/sensor.py:352: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/mqtt/binary_sensor.py:257: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/mqtt/binary_sensor.py:257: error: "None" not callable  [misc]
+ homeassistant/components/mqtt/binary_sensor.py:257: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/rest/sensor.py:149: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/rest/sensor.py:149: error: "None" not callable  [misc]
+ homeassistant/components/rest/sensor.py:149: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/rest/sensor.py:150: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/rest/sensor.py:150: error: "None" not callable  [misc]
+ homeassistant/components/rest/sensor.py:150: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/rest/binary_sensor.py:140: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/rest/binary_sensor.py:140: error: "None" not callable  [misc]
+ homeassistant/components/rest/binary_sensor.py:140: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/rest/binary_sensor.py:141: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/rest/binary_sensor.py:141: error: "None" not callable  [misc]
+ homeassistant/components/rest/binary_sensor.py:141: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/template/image.py:137: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/template/image.py:137: error: "None" not callable  [misc]
+ homeassistant/components/template/image.py:137: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/template/image.py:178: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/template/image.py:178: error: "None" not callable  [misc]
+ homeassistant/components/template/image.py:178: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/template/image.py:179: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/template/image.py:179: error: "None" not callable  [misc]
+ homeassistant/components/template/image.py:179: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/scrape/sensor.py:231: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/scrape/sensor.py:231: error: "None" not callable  [misc]
+ homeassistant/components/scrape/sensor.py:231: note: Error code "misc" not covered by "type: ignore" comment
+ homeassistant/components/scrape/sensor.py:232: error: Unused "type: ignore" comment  [unused-ignore]
+ homeassistant/components/scrape/sensor.py:232: error: "None" not callable  [misc]
+ homeassistant/components/scrape/sensor.py:232: note: Error code "misc" not covered by "type: ignore" comment

mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/tools/console/overlay.py:20: error: Cannot override writeable attribute with read-only property  [override]

pyodide (https://github.com/pyodide/pyodide)
+ src/py/pyodide/console.py:50: error: Cannot override writeable attribute with read-only property  [override]
+ src/py/pyodide/console.py:54: error: Cannot override writeable attribute with read-only property  [override]

sterliakov · 2025-01-30T16:00:21Z

psycopg: they have check_untyped_defs enabled for tests.*, that was a false negative
pandas: OK
xarray: property override, no weird "has no attribute" error anymore, an explicit "not supported" error instead
streamlit, pytest, mitmproxy, pyodide, tornado: they have check_untyped_defs enabled, this should've been reported earlier
spark: correct, uses property.fset as intended, was fp
ppb-vector: correct, uses property as intended
pytest_robotframework: Not a bug. We report @override on methods of subclasses of unresolved classes (that's probably a bug, but unrelated one), and pytest is missing there during primer run.
pwndbg, ibis: correct types in the error messages now
pandera: They set follow_imports = skip, so base class has type Any, and override is perfectly valid (and isn't even an override). But something shady is going on there: while trying to untangle this, I managed to produce a caching crash (will report today) and a wide variety of diagnostics combinations for those overrides. Decorator itself isn't the problem: adding it to the next function (example) which is also an override does not affect typechecking. Can't reproduce locally: running mypy with same flags and venv as primer does emit unused-ignore there.
discord.py: correct, non-adjacent setter
homeassistant: suffers from property typing in typeshed. mypy now recognizes that property has .fset and .fget, but they are | None so can't be used without assert. Maybe special-case this later?

sterliakov · 2025-01-30T16:33:11Z

Unfortunately, this removes errors in #15507 (property setter defined in a context manager), but that would be more difficult to fix here - nested blocks in class def are rather complicated.

sterliakov added 2 commits January 22, 2025 00:10

Check property access from class object

de85cfa

Adjust the failing test

3dcf759

This comment has been minimized.

Sign in to view

sterliakov marked this pull request as ready for review January 22, 2025 02:05

A5rocks approved these changes Jan 24, 2025

View reviewed changes

sterliakov marked this pull request as draft January 24, 2025 17:41

sterliakov added 3 commits January 28, 2025 20:22

Reveal property at class level too

9c739b3

Merge remote-tracking branch 'upstream/master' into bugfix/st-propert…

6c45c3f

…y-on-class-checkmember

Only note if the replacement is also a Decorator

5f9e15a

This comment has been minimized.

Sign in to view

sterliakov marked this pull request as ready for review January 28, 2025 23:01

sterliakov marked this pull request as draft January 29, 2025 05:56

Handle overloaded case consistently

8db72ed

This comment has been minimized.

Sign in to view

Exit earlier for incremental mode, but still check properties

d46ba12

This comment has been minimized.

Sign in to view

sterliakov added 3 commits January 30, 2025 00:48

Revert change to recurse_into_functions, only analyze decorators on d…

a8f44b9

…emand

Merge remote-tracking branch 'upstream/master' into bugfix/st-propert…

9833179

…y-on-class-checkmember

This should work for all redefinitions

6ab9453

This comment has been minimized.

Sign in to view

Only add note if we see .setter or .deleter

baf5528

This comment has been minimized.

Sign in to view

Fix typing

ef68217

sterliakov commented Jan 30, 2025

View reviewed changes

sterliakov marked this pull request as ready for review January 30, 2025 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider property access from class objects in checkmember.py #18504

Consider property access from class objects in checkmember.py #18504

sterliakov commented Jan 21, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

sterliakov commented Jan 22, 2025

A5rocks left a comment

A5rocks Jan 24, 2025

A5rocks Jan 24, 2025

sterliakov commented Jan 24, 2025

This comment has been minimized.

sterliakov commented Jan 28, 2025

sterliakov commented Jan 29, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sterliakov Jan 30, 2025

github-actions bot commented Jan 30, 2025

sterliakov commented Jan 30, 2025

sterliakov commented Jan 30, 2025

	if is_decorated:
	if is_decorated and node.node.func.is_property:

Consider property access from class objects in checkmember.py #18504

Are you sure you want to change the base?

Consider property access from class objects in checkmember.py #18504

Conversation

sterliakov commented Jan 21, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

sterliakov commented Jan 22, 2025

A5rocks left a comment

Choose a reason for hiding this comment

A5rocks Jan 24, 2025

Choose a reason for hiding this comment

A5rocks Jan 24, 2025

Choose a reason for hiding this comment

sterliakov commented Jan 24, 2025

This comment has been minimized.

sterliakov commented Jan 28, 2025

sterliakov commented Jan 29, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sterliakov Jan 30, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 30, 2025

sterliakov commented Jan 30, 2025

sterliakov commented Jan 30, 2025

sterliakov commented Jan 21, 2025 •

edited

Loading