From 24d8733b8ad128710e50256e109bc0471f8a7867 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sun, 11 Sep 2022 19:11:09 -0700 Subject: [PATCH 1/2] Fix joining a function against metaclass-using object constructors This pull request fixes #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 #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. --- mypy/join.py | 15 ++++++++------- test-data/unit/check-classes.test | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/mypy/join.py b/mypy/join.py index 671924a75da1..4cd2c6b2534b 100644 --- a/mypy/join.py +++ b/mypy/join.py @@ -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 @@ -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 diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 3b1eddc8a084..c51a2efbef8b 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -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 -- ------------------------------ From 77bf64947a14536b64f3a24fe9153cff8293f7ce Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sun, 25 Sep 2022 13:39:03 -0700 Subject: [PATCH 2/2] Respond to code review: simplify tests --- test-data/unit/check-classes.test | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index c51a2efbef8b..41a8f31433e9 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -867,28 +867,21 @@ class D2(C): pass [case testConstructorJoinsWithCustomMetaclass] # flags: --strict-optional -from typing import Any +from typing import TypeVar 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]" +T = TypeVar('T') +def join(x: T, y: T) -> T: pass -def f3(cond: bool) -> Any: - join = NormalClass if cond else WithMetaclass - return reveal_type(join()) # N: Revealed type is "builtins.object" +f1 = join(func, WithMetaclass) +reveal_type(f1()) # N: Revealed type is "Union[__main__.WithMetaclass, None]" -def f4(cond: bool) -> Any: - join = WithMetaclass if cond else NormalClass - return reveal_type(join()) # N: Revealed type is "builtins.object" +f2 = join(WithMetaclass, func) +reveal_type(f2()) # N: Revealed type is "Union[__main__.WithMetaclass, None]" -- Attribute access in class body -- ------------------------------