Skip to content

Commit

Permalink
Fix joining a function against metaclass-using object constructors
Browse files Browse the repository at this point in the history
This pull request fixes python#9838.

It turns out that when an object is using a metaclass, it uses
that metaclass as the fallback instead of `builtins.type`.

This caused the `if t.fallback.type.fullname != "builtins.type"`
check we were performing in `join_similar_callables` and
`combine_similar_callables` to pick the wrong fallback in the case
where we were attempting to join a function against a constructor
for an object that used a metaclass.

This ended up causing a crash later for basically the exact same
reason python#13576 caused a crash: using `abc.ABCMeta` causes
`Callable.is_type_obj()` to return true, which causes us to enter
a codepath where we call `Callable.type_object()`. But this function
is not prepared to handle the case where the return type of the callable
is a Union, causing an assert to fail.

I opted to fix this by adjusting the join algorithm so it does
`if t.fallback.type.fullname == "builtins.function"`.

One question I did punt on -- what should happen in the case where
one of the fallbacks is `builtins.type` and the other is a metaclass?

I suspect it's impossible for this case to actually occur: I think
mypy would opt to use the algorithm for joining two `Type[...]` entities
instead of these callable joining algorithms. While I'm not 100% sure of
this, the current approach of just arbitrarily picking one of the two
fallbacks seemed good enough for now.
  • Loading branch information
Michael0x2a committed Sep 12, 2022
1 parent 0f17aff commit 24d8733
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
15 changes: 8 additions & 7 deletions mypy/join.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,10 @@ def join_similar_callables(t: CallableType, s: CallableType) -> CallableType:
arg_types: list[Type] = []
for i in range(len(t.arg_types)):
arg_types.append(meet_types(t.arg_types[i], s.arg_types[i]))
# TODO in combine_similar_callables also applies here (names and kinds)
# The fallback type can be either 'function' or 'type'. The result should have 'type' as
# fallback only if both operands have it as 'type'.
if t.fallback.type.fullname != "builtins.type":
# TODO in combine_similar_callables also applies here (names and kinds; user metaclasses)
# The fallback type can be either 'function', 'type', or some user-provided metaclass.
# The result should always use 'function' as a fallback if either operands are using it.
if t.fallback.type.fullname == "builtins.function":
fallback = t.fallback
else:
fallback = s.fallback
Expand All @@ -580,9 +580,10 @@ def combine_similar_callables(t: CallableType, s: CallableType) -> CallableType:
for i in range(len(t.arg_types)):
arg_types.append(join_types(t.arg_types[i], s.arg_types[i]))
# TODO kinds and argument names
# The fallback type can be either 'function' or 'type'. The result should have 'type' as
# fallback only if both operands have it as 'type'.
if t.fallback.type.fullname != "builtins.type":
# TODO what should happen if one fallback is 'type' and the other is a user-provided metaclass?
# The fallback type can be either 'function', 'type', or some user-provided metaclass.
# The result should always use 'function' as a fallback if either operands are using it.
if t.fallback.type.fullname == "builtins.function":
fallback = t.fallback
else:
fallback = s.fallback
Expand Down
24 changes: 24 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,30 @@ class C(B): pass
class D(C): pass
class D2(C): pass

[case testConstructorJoinsWithCustomMetaclass]
# flags: --strict-optional
from typing import Any
import abc

def func() -> None: pass
class NormalClass: pass
class WithMetaclass(metaclass=abc.ABCMeta): pass

def f1(cond: bool) -> Any:
join = func if cond else WithMetaclass
return reveal_type(join()) # N: Revealed type is "Union[__main__.WithMetaclass, None]"

def f2(cond: bool) -> Any:
join = WithMetaclass if cond else func
return reveal_type(join()) # N: Revealed type is "Union[__main__.WithMetaclass, None]"

def f3(cond: bool) -> Any:
join = NormalClass if cond else WithMetaclass
return reveal_type(join()) # N: Revealed type is "builtins.object"

def f4(cond: bool) -> Any:
join = WithMetaclass if cond else NormalClass
return reveal_type(join()) # N: Revealed type is "builtins.object"

-- Attribute access in class body
-- ------------------------------
Expand Down

0 comments on commit 24d8733

Please sign in to comment.