Swapping some type variables gives incorrect results

[randolf-scholz]

Two related issues (#19444 and #19362) were fixed in #19449, but I discovered that swapping type variables still gives incorrect results if the generic type contains some additional optional type variables.

class Foo[X, Y, Z=object]:
    def test(self, swapped: Foo[Y, X]) -> None:
        reveal_type(swapped)  # "Foo[X`1, Y`2, builtins.object]" ❓️❗️

[sterliakov]

Oh. This is not a PEP695 issue, "old-style" variables suffer from the same. The problem is ID collision in typeanal.fix_instance.

We see Foo[Y`2, X`1], and Y is the same (same ID) as the outer Y. That's correct, the variable is still the same, the namespace is the same. We compare against the declared generic args: [X`1, Y`2, Z], and they pair perfectly: two vars to two other vars, third one to default. So we create a mapping {1: Y, 2: X, 3: object}, and then we sub all variables... and get [X, Y, object].

To make it more obvious, here's how Y can interfere with X value (with the substitution chain X -> Y -> int):

from typing import Generic, TypeVar

X = TypeVar("X")
Y = TypeVar("Y")
Z = TypeVar("Z", default=object)

class Foo(Generic[X, Y, Z]):
    def test(self, swapped: Foo[Y, int]) -> None:
        reveal_type(swapped)  # "Foo[builtins.int, builtins.int, builtins.object]"

I'm feeling dumb a bit, but I do not immediately see how to fix this. We do not want to allocate new namespace for that - type vars should remain intact. Temporary meta_level change, perhaps?..

cc @ilevkivskyi, I suggest raising this to p-1-normal - looks really annoying! (but I still don't feel like applying priority labels here)

[ilevkivskyi]

Yeah this is quite annoying. But just to be clear: is this specific only to situations with type variables with defaults? If yes, perhaps some logic is simply wrong in fix_instance()?

[sterliakov]

It goes roughly as follows: for every (declared_typearg, provided_typearg) pair (zip_longest with None), map declared ID to the provided type (if provided is missing, we pick default or Any). Then expand the instance according to that mapping.

Keys and values can overlap in the generated mapping: we say "replace X with Y, replace Y with int", and both Y-s refer to the same variable. This is problematic here, because we want to treat them as distinct for the duration of this substitution?

[sterliakov]

...wait, can we just skip the provided args?

diff --git a/mypy/typeanal.py b/mypy/typeanal.py
index 99cc56ae0..251b6d46a 100644
--- a/mypy/typeanal.py
+++ b/mypy/typeanal.py
@@ -2099,7 +2099,7 @@ def fix_instance(
                     )
                 arg = any_type
             args.append(arg)
-        env[tv.id] = arg
+            env[tv.id] = arg
     t.args = tuple(args)
     fix_type_var_tuple_argument(t)
     if not t.type.has_type_var_tuple_type:

[ilevkivskyi]

So essentially the problem is because the substitution is "non-atomic"? Anyway, I will need to look a bit deeper into this. I mentioned couple times type variables need some re-work on a fundamental level (in particular I really don't like the in-place modifications in expand_type).

[ilevkivskyi]

...wait, can we just skip the provided args?

If there is some hot-fix I am fine with this (since I could imagine this issue may be quite annoying), but my point still stands, support for typevar defaults needs re-thinking.

[sterliakov]

No, my patch is still wrong - it fixes this case but not the following (expected Foo[Y, X, Y]):

class Foo[X, Y, Z=X]:
    def test(self, swapped: Foo[Y, X]) -> None:
        reveal_type(swapped)  # N: Revealed type is "a.Foo[Y`2, X`1, X`1]"

I'll get back to this today after work.

[ayushdoesdev]

Please check this PR:
Fix incorrect TypeVar default expansion order (Fixes #20336) #20343

Input

class Foo[X, Y, Z=object]:
    def test(self, swapped: Foo[Y, X]) -> None:
        reveal_type(swapped) 

Output

repro-test.py:3: note: Revealed type is "repro-test.Foo[X`1, Y`2, builtins.object]"
Success: no issues found in 1 source file