Fix recursion issue with nested instances and unions

[Peilonrayz]

Description

Fixes #9657

Test Plan

I have added three new tests to test-data/unit/check-unions.test

• testNestedProtocolUnions
  This is an exact copy of the MVCE I included in the issue.

• testNestedProtocolGenericUnions
  At first I 'fixed' the issue by not running the for if left is a protocol in mypy.subtypes:is_protocol_implementation. I however noticed that this would still cause the same issue if we changed to Iter = Union[Iterator[T], List[T]]. And all I'd have done is add a subtle bug. This is just the same as testNestedProtocolUnions but to account for non-protocols.

• testNestedProtocolGenericUnionsDeep
  This tests that we can go 5 Iters deep. This is important as we can fix the bug at 3 levels by adding the following to mypy.sametypes:is_same_type.

  if isinstance(left, UnionType) is not isinstance(right, UnionType):
      return False

My changes pass all the tests ran via ./runtests.py.

Origin of the Bug

The issue comes from assuming in mypy.subtypes:is_protocol_implementation.

1. The assumed type is an instance that has a union as its argument.

2. mypy.sametypes:is_same_type works fine for the Instance.

3. We check the Unions are the same type, again with mypy.sametypes:is_same_type.

4. This falls apart as we then call simplify_union which goes on to call mypy.subtypes:is_protocol_implementation.

   In this second call to is_protocol_implementation what we assume the type to be stays the same. This causes problems because we're constantly simplifying what we assume the type to be against itself.

To circumvent this semi-infinite loop I've forced the left value to always be assumed to be a simplified union. This is risky as this is a new assumption. It seems to work, but I'm unsure if this is because of a lack of tests that'd put a non-simplified union on the left side.

Alternates

I can think of two different solutions that don't add an assumption to mypy.sametypes:is_same_type.

1. Adjusting the assumed types in mypy.subtypes:is_protocol_implementation to correctly handle the recursion. I don't know how to implement this.
2. Changing UnionType to have a 'simplified' attribute. I could implement this. But it'd be better wherever the assumptions are being made, and I do not know where that is.

[JukkaL]

Thanks for the PR and the detailed explanation!

This seems to regress some supported use cases. I left an alternative idea below.

[Peilonrayz]

As suggested by JukkaL, I have changed from mypy.sametypes.is_same_type(l, left) to l == left. And it seems to work fine.

I have added three additional tests to check if get_proper_type or mypy.sametypes.simplify_union is needed. These tests pass so I can't see any issues here.

[JukkaL]

Thanks for the updates! Looks good now. It would be good to if you can add a unit test for is_same_type (but not necessary), as explained in my previous comment.

[Peilonrayz]

Sorry for the delay. I have now added the test in what I think is the best place in the file. I ensured that the test fails on the older solution locally. I hope this is the correct place to add the test, and thank you for helping me find the file in the first place.

[hauntsaninja]

Thanks for a well-explained issue report and subsequent fix to a tricky bug!