mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Fix joining a function against metaclass-using object constructors

Open Michael0x2a opened this issue 3 years ago • 3 comments
trafficstars

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 discussed in #13576: 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.

Michael0x2a avatar Sep 12 '22 02:09 Michael0x2a

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Sep 12 '22 03:09 github-actions[bot]

I'm guessing all of these tests would break if something like #12712 was ever merged (there seems to be a consensus that it would be good to do something like that if the regressions could be kept down to a minimum — https://github.com/python/mypy/pull/12395#issuecomment-107408093).

Is there a way of making the tests more future-proof for that? Or not worth bothering about?

AlexWaygood avatar Sep 12 '22 12:09 AlexWaygood

Actually, yes, I agree that a better way to trigger join is to use f(x: T, y: T) -> T: ....

ilevkivskyi avatar Sep 13 '22 09:09 ilevkivskyi

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Sep 25 '22 21:09 github-actions[bot]

Yay Michael!

gvanrossum avatar Sep 25 '22 21:09 gvanrossum