Better names of and more compatibility between ad hoc intersections of instances (#18506)

While working on #18433, we encountered [this
bug.](#18433 (comment)).
@ilevkivskyi identified [the underlying
problem](#18433 (comment)),
and we decided to try to reuse previously created ad hoc intersections
of instances instead of always creating new ones.

While working on this PR, I realised that reusing intersections requires
more complete names. Currently, module and type variable specifications
are not included, which could result in mistakes when using these names
as identifiers.

So, I switched to more complete names. Now, for example, `<subclass of
"A" and "A">` becomes `<subclass of "mod1.A[builtins.int]" and
"mod2.A">`. Hence, I had to adjust many existing test cases where
`reveal_type` is used.

`testReuseIntersectionForRepeatedIsinstanceCalls` confirms that the
mentioned bug is fixed.

`testIsInstanceAdHocIntersectionIncrementalNestedClass` and
`testIsInstanceAdHocIntersectionIncrementalUnions` are in a separate
commit. I think they are not really necessary, so we might prefer to
remove them. I added them originally because I had to adjust
`lookup_fully_qualified` a little. The change is very simple, but I
could not create a test case where it is not sufficient.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: tyralla

mypy/checker.py

@@ -5501,13 +5501,9 @@ def intersect_instances(
         theoretical subclass of the instances the user may be trying to use
         the generated intersection can serve as a placeholder.
 
-        This function will create a fresh subclass every time you call it,
-        even if you pass in the exact same arguments. So this means calling
-        `self.intersect_intersection([inst_1, inst_2], ctx)` twice will result
-        in instances of two distinct subclasses of inst_1 and inst_2.
-
-        This is by design: we want each ad-hoc intersection to be unique since
-        they're supposed represent some other unknown subclass.
+        This function will create a fresh subclass the first time you call it.
+        So this means calling `self.intersect_intersection([inst_1, inst_2], ctx)`
+        twice will return the same subclass of inst_1 and inst_2.
 
         Returns None if creating the subclass is impossible (e.g. due to
         MRO errors or incompatible signatures). If we do successfully create
@@ -5540,20 +5536,19 @@ def _get_base_classes(instances_: tuple[Instance, Instance]) -> list[Instance]:
             return base_classes_
 
         def _make_fake_typeinfo_and_full_name(
-            base_classes_: list[Instance], curr_module_: MypyFile
+            base_classes_: list[Instance], curr_module_: MypyFile, options: Options
         ) -> tuple[TypeInfo, str]:
-            names_list = pretty_seq([x.type.name for x in base_classes_], "and")
-            short_name = f"<subclass of {names_list}>"
-            full_name_ = gen_unique_name(short_name, curr_module_.names)
-            cdef, info_ = self.make_fake_typeinfo(
-                curr_module_.fullname, full_name_, short_name, base_classes_
-            )
-            return info_, full_name_
+            names = [format_type_bare(x, options=options, verbosity=2) for x in base_classes_]
+            name = f"<subclass of {pretty_seq(names, 'and')}>"
+            if (symbol := curr_module_.names.get(name)) is not None:
+                assert isinstance(symbol.node, TypeInfo)
+                return symbol.node, name
+            cdef, info_ = self.make_fake_typeinfo(curr_module_.fullname, name, name, base_classes_)
+            return info_, name
 
         base_classes = _get_base_classes(instances)
-        # We use the pretty_names_list for error messages but can't
-        # use it for the real name that goes into the symbol table
-        # because it can have dots in it.
+        # We use the pretty_names_list for error messages but for the real name that goes
+        # into the symbol table because it is not specific enough.
         pretty_names_list = pretty_seq(
             format_type_distinctly(*base_classes, options=self.options, bare=True), "and"
         )
@@ -5567,13 +5562,17 @@ def _make_fake_typeinfo_and_full_name(
             return None
 
         try:
-            info, full_name = _make_fake_typeinfo_and_full_name(base_classes, curr_module)
+            info, full_name = _make_fake_typeinfo_and_full_name(
+                base_classes, curr_module, self.options
+            )
             with self.msg.filter_errors() as local_errors:
                 self.check_multiple_inheritance(info)
             if local_errors.has_new_errors():
                 # "class A(B, C)" unsafe, now check "class A(C, B)":
                 base_classes = _get_base_classes(instances[::-1])
-                info, full_name = _make_fake_typeinfo_and_full_name(base_classes, curr_module)
+                info, full_name = _make_fake_typeinfo_and_full_name(
+                    base_classes, curr_module, self.options
+                )
                 with self.msg.filter_errors() as local_errors:
                     self.check_multiple_inheritance(info)
             info.is_intersection = True

mypy/lookup.py

@@ -22,9 +22,11 @@ def lookup_fully_qualified(
     This function should *not* be used to find a module. Those should be looked
     in the modules dictionary.
     """
-    head = name
+    # 1. Exclude the names of ad hoc instance intersections from step 2.
+    i = name.find("<subclass ")
+    head = name if i == -1 else name[:i]
     rest = []
-    # 1. Find a module tree in modules dictionary.
+    # 2. Find a module tree in modules dictionary.
     while True:
         if "." not in head:
             if raise_on_missing:
@@ -36,12 +38,14 @@ def lookup_fully_qualified(
         if mod is not None:
             break
     names = mod.names
-    # 2. Find the symbol in the module tree.
+    # 3. Find the symbol in the module tree.
     if not rest:
         # Looks like a module, don't use this to avoid confusions.
         if raise_on_missing:
             assert rest, f"Cannot find {name}, got a module symbol"
         return None
+    if i != -1:
+        rest[0] += name[i:]
     while True:
         key = rest.pop()
         if key not in names:

test-data/unit/check-incremental.test

@@ -5268,7 +5268,7 @@ reveal_type(Foo().x)
 [builtins fixtures/isinstance.pyi]
 [out]
 [out2]
-tmp/b.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
+tmp/b.py:2: note: Revealed type is "a.<subclass of "a.A" and "a.B">"
 
 [case testIsInstanceAdHocIntersectionIncrementalNoChangeSameName]
 import b
@@ -5291,7 +5291,7 @@ reveal_type(Foo().x)
 [builtins fixtures/isinstance.pyi]
 [out]
 [out2]
-tmp/b.py:2: note: Revealed type is "a.<subclass of "B" and "B">"
+tmp/b.py:2: note: Revealed type is "a.<subclass of "c.B" and "a.B">"
 
 
 [case testIsInstanceAdHocIntersectionIncrementalNoChangeTuple]
@@ -5313,7 +5313,7 @@ reveal_type(Foo().x)
 [builtins fixtures/isinstance.pyi]
 [out]
 [out2]
-tmp/b.py:2: note: Revealed type is "a.<subclass of "tuple" and "B">"
+tmp/b.py:2: note: Revealed type is "a.<subclass of "Tuple[builtins.int, ...]" and "a.B">"
 
 [case testIsInstanceAdHocIntersectionIncrementalIsInstanceChange]
 import c
@@ -5347,9 +5347,9 @@ from b import y
 reveal_type(y)
 [builtins fixtures/isinstance.pyi]
 [out]
-tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
+tmp/c.py:2: note: Revealed type is "a.<subclass of "a.A" and "a.B">"
 [out2]
-tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "C">"
+tmp/c.py:2: note: Revealed type is "a.<subclass of "a.A" and "a.C">"
 
 [case testIsInstanceAdHocIntersectionIncrementalUnderlyingObjChang]
 import c
@@ -5375,9 +5375,9 @@ from b import y
 reveal_type(y)
 [builtins fixtures/isinstance.pyi]
 [out]
-tmp/c.py:2: note: Revealed type is "b.<subclass of "A" and "B">"
+tmp/c.py:2: note: Revealed type is "b.<subclass of "a.A" and "a.B">"
 [out2]
-tmp/c.py:2: note: Revealed type is "b.<subclass of "A" and "C">"
+tmp/c.py:2: note: Revealed type is "b.<subclass of "a.A" and "a.C">"
 
 [case testIsInstanceAdHocIntersectionIncrementalIntersectionToUnreachable]
 import c
@@ -5408,7 +5408,7 @@ from b import z
 reveal_type(z)
 [builtins fixtures/isinstance.pyi]
 [out]
-tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
+tmp/c.py:2: note: Revealed type is "a.<subclass of "a.A" and "a.B">"
 [out2]
 tmp/b.py:2: error: Cannot determine type of "y"
 tmp/c.py:2: note: Revealed type is "Any"
@@ -5445,7 +5445,60 @@ reveal_type(z)
 tmp/b.py:2: error: Cannot determine type of "y"
 tmp/c.py:2: note: Revealed type is "Any"
 [out2]
-tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
+tmp/c.py:2: note: Revealed type is "a.<subclass of "a.A" and "a.B">"
+
+[case testIsInstanceAdHocIntersectionIncrementalNestedClass]
+import b
+[file a.py]
+class A:
+    class B: ...
+    class C: ...
+    class D:
+        def __init__(self) -> None:
+            x: A.B
+            assert isinstance(x, A.C)
+            self.x = x
+[file b.py]
+from a import A
+[file b.py.2]
+from a import A
+reveal_type(A.D.x)
+[builtins fixtures/isinstance.pyi]
+[out]
+[out2]
+tmp/b.py:2: note: Revealed type is "a.<subclass of "a.A.B" and "a.A.C">"
+
+[case testIsInstanceAdHocIntersectionIncrementalUnions]
+import c
+[file a.py]
+import b
+class A:
+    p: b.D
+class B:
+    p: b.D
+class C:
+    p: b.D
+    c: str
+x: A
+assert isinstance(x, (B, C))
+y = x
+[file b.py]
+class D:
+    p: int
+[file c.py]
+from a import y
+[file c.py.2]
+from a import y, C
+reveal_type(y)
+reveal_type(y.p.p)
+assert isinstance(y, C)
+reveal_type(y.c)
+[builtins fixtures/isinstance.pyi]
+[out]
+[out2]
+tmp/c.py:2: note: Revealed type is "Union[a.<subclass of "a.A" and "a.B">, a.<subclass of "a.A" and "a.C">]"
+tmp/c.py:3: note: Revealed type is "builtins.int"
+tmp/c.py:5: note: Revealed type is "builtins.str"
 
 [case testStubFixupIssues]
 import a