Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of attribute access on class objects #14988

Merged
merged 3 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: Member
# See https://github.com/python/mypy/pull/1787 for more info.
# TODO: do not rely on same type variables being present in all constructor overloads.
result = analyze_class_attribute_access(
ret_type, name, mx, original_vars=typ.items[0].variables
ret_type, name, mx, original_vars=typ.items[0].variables, mcs_fallback=typ.fallback
)
if result:
return result
Expand Down Expand Up @@ -434,17 +434,21 @@ def analyze_type_type_member_access(
if isinstance(typ.item.item, Instance):
item = typ.item.item.type.metaclass_type
ignore_messages = False

if item is not None:
fallback = item.type.metaclass_type or fallback

if item and not mx.is_operator:
# See comment above for why operators are skipped
result = analyze_class_attribute_access(item, name, mx, override_info)
result = analyze_class_attribute_access(
item, name, mx, mcs_fallback=fallback, override_info=override_info
)
if result:
if not (isinstance(get_proper_type(result), AnyType) and item.type.fallback_to_any):
return result
else:
# We don't want errors on metaclass lookup for classes with Any fallback
ignore_messages = True
if item is not None:
fallback = item.type.metaclass_type or fallback

with mx.msg.filter_errors(filter_errors=ignore_messages):
return _analyze_member_access(name, fallback, mx, override_info)
Expand Down Expand Up @@ -893,6 +897,8 @@ def analyze_class_attribute_access(
itype: Instance,
name: str,
mx: MemberContext,
*,
mcs_fallback: Instance,
override_info: TypeInfo | None = None,
original_vars: Sequence[TypeVarLikeType] | None = None,
) -> Type | None:
Expand All @@ -919,6 +925,22 @@ def analyze_class_attribute_access(
return apply_class_attr_hook(mx, hook, AnyType(TypeOfAny.special_form))
return None

if (
isinstance(node.node, Var)
and not node.node.is_classvar
and not hook
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and not hook was required here, or this test started to fail:

[case testClassAttrPluginMetaclassRegularBase]
# flags: --config-file tmp/mypy.ini
from typing import Any, Type
class M(type):
attr = 'test'
class B:
attr = None
class Cls(B, metaclass=M):
pass
reveal_type(Cls.attr) # N: Revealed type is "builtins.int"
[file mypy.ini]
\[mypy]
plugins=<ROOT>/test-data/unit/plugins/class_attr_hook.py

and mcs_fallback.type.get(name)
):
# If the same attribute is declared on the metaclass and the class but with different types,
# and the attribute on the class is not a ClassVar,
# the type of the attribute on the metaclass should take priority
# over the type of the attribute on the class,
# when the attribute is being accessed from the class object itself.
#
# Return `None` here to signify that the name should be looked up
# on the class object itself rather than the instance.
return None

is_decorated = isinstance(node.node, Decorator)
is_method = is_decorated or isinstance(node.node, FuncBase)
if mx.is_lvalue:
Expand Down
5 changes: 4 additions & 1 deletion test-data/unit/check-class-namedtuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,10 @@ class X(NamedTuple):
reveal_type(X._fields) # N: Revealed type is "Tuple[builtins.str, builtins.str]"
reveal_type(X._field_types) # N: Revealed type is "builtins.dict[builtins.str, Any]"
reveal_type(X._field_defaults) # N: Revealed type is "builtins.dict[builtins.str, Any]"
reveal_type(X.__annotations__) # N: Revealed type is "builtins.dict[builtins.str, Any]"

# In typeshed's stub for builtins.pyi, __annotations__ is `dict[str, Any]`,
# but it's inferred as `Mapping[str, object]` here due to the fixture we're using
reveal_type(X.__annotations__) # N: Revealed type is "typing.Mapping[builtins.str, builtins.object]"

[builtins fixtures/dict.pyi]
Comment on lines +329 to 333
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test needed to be altered slightly, as with this change, mypy starts inferring the correct type of X.__annotations__ according to the fixture that this test uses:

class type:
__annotations__: Mapping[str, object]


Expand Down
33 changes: 33 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -4521,6 +4521,39 @@ def f(TA: Type[A]):
reveal_type(TA) # N: Revealed type is "Type[__main__.A]"
reveal_type(TA.x) # N: Revealed type is "builtins.int"

[case testMetaclassConflictingInstanceVars]
from typing import ClassVar

class Meta(type):
foo: int
bar: int
eggs: ClassVar[int] = 42
spam: ClassVar[int] = 42

class Foo(metaclass=Meta):
foo: str
bar: ClassVar[str] = 'bar'
eggs: str
spam: ClassVar[str] = 'spam'
Comment on lines +4535 to +4537
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could argue that bar and spam here violate LSP (since setting a class variable on a class is ~basically the same as setting an instance variable on a metaclass). This PR doesn't change the behaviour there, however: mypy emits no errors here on master, and doesn't with this PR applied either.


reveal_type(Foo.foo) # N: Revealed type is "builtins.int"
reveal_type(Foo.bar) # N: Revealed type is "builtins.str"
reveal_type(Foo.eggs) # N: Revealed type is "builtins.int"
reveal_type(Foo.spam) # N: Revealed type is "builtins.str"

class MetaSub(Meta): ...

class Bar(metaclass=MetaSub):
foo: str
bar: ClassVar[str] = 'bar'
eggs: str
spam: ClassVar[str] = 'spam'

reveal_type(Bar.foo) # N: Revealed type is "builtins.int"
reveal_type(Bar.bar) # N: Revealed type is "builtins.str"
reveal_type(Bar.eggs) # N: Revealed type is "builtins.int"
reveal_type(Bar.spam) # N: Revealed type is "builtins.str"

[case testSubclassMetaclass]
class M1(type):
x = 0
Expand Down
13 changes: 13 additions & 0 deletions test-data/unit/pythoneval.test
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,19 @@ def good9(foo1: Foo[Concatenate[int, P]], foo2: Foo[[int, str, bytes]], *args: P
[out]
_testStrictEqualitywithParamSpec.py:11: error: Non-overlapping equality check (left operand type: "Foo[[int]]", right operand type: "Bar[[int]]")

[case testInferenceOfDunderDictOnClassObjects]
class Foo: ...
reveal_type(Foo.__dict__)
reveal_type(Foo().__dict__)
Foo.__dict__ = {}
Foo().__dict__ = {}

[out]
_testInferenceOfDunderDictOnClassObjects.py:2: note: Revealed type is "types.MappingProxyType[builtins.str, Any]"
_testInferenceOfDunderDictOnClassObjects.py:3: note: Revealed type is "builtins.dict[builtins.str, Any]"
_testInferenceOfDunderDictOnClassObjects.py:4: error: Property "__dict__" defined in "type" is read-only
_testInferenceOfDunderDictOnClassObjects.py:4: error: Incompatible types in assignment (expression has type "Dict[<nothing>, <nothing>]", variable has type "MappingProxyType[str, Any]")

[case testTypeVarTuple]
# flags: --enable-incomplete-feature=TypeVarTuple --enable-incomplete-feature=Unpack --python-version=3.11
from typing import Any, Callable, Unpack, TypeVarTuple
Expand Down