Skip to content

Commit

Permalink
Improve handling of attribute access on class objects (#14988)
Browse files Browse the repository at this point in the history
Fixes #14056

#14056 was originally reported as a mypyc issue (because that's how it
presented itself to the user), but the underlying bug is really a bug to
do with how mypy understands metaclasses. On mypy `master`:

```py
class Meta(type):
    bar: str

class Foo(metaclass=Meta):
    bar: int

reveal_type(Foo().bar)  # Revealed type is int (correct!)
reveal_type(Foo.bar)  # Revealed type is int, but should be str
```

This PR fixes that incorrect behaviour.

Since this is really a mypy bug rather than a mypyc bug, I haven't added
a mypyc test, but I'm happy to if that would be useful. (I'll need some
guidance as to exactly where it should go -- I don't know much about
mypyc internals!)
  • Loading branch information
AlexWaygood committed Jun 16, 2023
1 parent db5b5af commit 21cc1c7
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 5 deletions.
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
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]

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 @@ -4540,6 +4540,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'

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

0 comments on commit 21cc1c7

Please sign in to comment.