Also fixes #18334 and maybe something else.

@sterliakov I added #18334 as a unit test.

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@sterliakov This and a few other small PRs (#19517, #19471, #19449) have been sitting since mid-July, I'm guessing you are rather swamped. Who else should I ask for review?

I am a bit swamped indeed, and also I don't have merge powers here anyway:)

This change is really non-trivial. I have already looked at this PR and, tbh, I'm still not 100% certain that binding self to Any-filled self is the correct move here. It makes some sense to me, but I'd love to see more tests with manually verified logic.

I just took another look, and IMO the following snippet will be handled incorrectly:

class Mixed[T, U]:
    def new[S](self: "Mixed[S, U]", key: S, val: U) -> None: ...

x = Mixed[str, int]()
x.new(object(), 0)  # Should error
# So this upcast is not really an upcast and should be rejected, T should be contravariant?
y: Mixed[object, int] = x
y.new(object(), 0)  # Should not error

check_contra: Mixed[str, int] = Mixed[object, int]()
check_co: Mixed[object, int] = Mixed[str, int]()

It's not strictly incorrect (because resolving S to object would be valid for x.new() call), but is handled by mypy that way, and errors should never disappear when a value is upcasted (assigned to without ignore, type error or cast) to some wider type.

I didn't try to compare this to Pyright or other type checkers.

I'd suggest to also ping @JukkaL for review - he authored a huge part of PEP695 implementation, including this inference.

I tested a variation of your example pyright-playground, mypy-playground

from typing import cast

class Mixed[T, U]:
    def new[S](self: "Mixed[S, U]", key: S, val: U) -> None: ...

def test_co(x: Mixed[str, int]) -> Mixed[object, int]:
    return x                    # master: ❌ PR: ✅, pyright: ✅

def test_contra(x: Mixed[object, int]) -> Mixed[str, int]:
    return x                    # master: ✅ PR: ❌, pyright: ❌

def test_now_sub(y: Mixed[object, int]) -> None:
    # str value is assignable to object type.
    y.new(key=str(), val=0)     # master: ✅ PR: ✅, pyright: ✅

def test_new_super(x: Mixed[str, int]) -> None:
    # object type is not assignable to str variable.
    x.new(key=object(), val=1)  # master: ❌ PR: ❌, pyright: ❌
    
def test_new_upcast(x: Mixed[str, int]) -> None:
    # technically, one could upcast first:
    z: Mixed[object, int] = x   # master: ❌ PR: ✅, pyright: ✅
    z = Mixed[object, int]()
    z.new(key=object(), val=1)  # master: ✅ PR: ✅, pyright: ✅

So both pyright and the PR treat T as covariant, whereas on master mypy treats it as contravariant. The new-cast still fails because object cannot be assigned to key. On PR and pyright, we can make it work by upcasting first, which is how it should be, since calling a method isn't allowed to mutate the type.

On PR and pyright, we can make it work by upcasting first, which is how it should be, since calling a method isn't allowed to mutate the type.

That just feels horribly wrong. LSP asserts that, given two types T <: U, any valid operation on U must be valid on T as well. If an upcast removes type checking errors, then something went wrong.

Please also note that Pyright isn't bug-free, so it makes sense to compare and look closer at any deviations, but it isn't a "golden standard" we aim to match perfectly.

Right now I'm moderately certain that Mixed should be contravariant in T (as inferred by current mypy master), not covariant as this PR and Pyright think. test_co should produce an error, test_contra should check cleanly - at least until mypy starts inferring S to object in a Mixed[str, int]().new(object(), 0) call, which would render polymorphic methods with unbound vars completely useless and probably isn't going to happen (not sure if this behavior is specced, but it is the most natural one anyway).

I crafted the example to demo the problem with Any substitution. It also means some weird inconsistency: now T is inferred covariant (my vision: contravariant). You can add def run(self) -> T, and T will remain covariant (my vision: should become invariant). Replace it with def run(self, _: T) -> None method with T in contravariant position, and T will be inferred contravariant (my vision: still contravariant).

NB: this is the only category of problems with Any substitution I see right now - when a method has its own type variables parameterizing self type. This PR is already a huge improvement over current behavior on master, so maybe it's a good idea to merge this as-is, but I'm not ready to say "yes, this is definitely correct" now.

That just feels horribly wrong. LSP asserts that, given two types T <: U, any valid operation on U must be valid on T as well. If an upcast removes type checking errors, then something went wrong.

But where is this violated? Given T@Mixed is treated as covariant, then test_new_sub passes. What doesn't pass without upcasting is the contravariant case test_new_super, where the key is a supertype of T, and that is fine, because upcasting here would mean we start treating the Mixed[str, int] instance as if it were Mixed[object, int].

Right now I'm moderately certain that Mixed should be contravariant in T (as inferred by current mypy master), not covariant as this PR and Pyright think.

Technically, in this example T should be both co- and contravariant simultaneously ("bi-variant"), since there is nothing inside the body of Mixed that would impose variance constraints on T. Usually in this case, both mypy and pyright would prefer covariance over contravariance by convention, for example both treat an empty generic class Foo[T]: pass as covariant (mypy-play, pyright-play.).

Whether Mixed ought to be co- or contravariant in T depends on how T is used within its body. But T is absent. polymorphic methods, like new in the example, that use a method-bound typevar do not impose constraints on the variance. (and this is the bug, mypy incorrectly infers a constraint here).

So the result is not technically wrong, as explained above T is technically both co- and contravariant, but (A) the way it is inferred is incorrect, and (B) it goes against the usual preference for covariance in the case of no constraints.

Moreover, if we added a covariant constraint like def get(self) -> T, then master would incorrectly infer Mixed as invariant, when it should be covariant. We can double-check this by using old-style typevars. If Mixed ought to be contravariant in T, then mypy should complain here, but it doesn't: (see also the original example of #19439)

from typing import TypeVar, Generic

T = TypeVar("T", covariant=True,  contravariant=False)
U = TypeVar("U", covariant=False, contravariant=False)
S = TypeVar("S", covariant=False, contravariant=False)

class Mixed(Generic[T, U]):  # OK, no variance error detected.
    def get(self) -> T: ...  # force covariance
    def new(self: "Mixed[S, U]", key: S, val: U) -> None: ...  # does not impose constraints on T

https://mypy-play.net/?mypy=latest&python=3.12&gist=811ae6429e7e0e05ba06e34d2daed000

But where is this violated?

Here:

def test_new_upcast(x: Mixed[str, int]) -> None:
    x.new(object(), 1)  # master: err, PR: err, pyright: err
    # technically, one could upcast first:
    z: Mixed[object, int] = x   # master: ❌ PR: ✅, pyright: ✅
    z = Mixed[object, int]()  # Only to counter weird narrowing by pyright
    z.new(key=object(), val=1)  # master: ✅ PR: ✅, pyright: ✅

So z still points to the same x object, and upcast (allowed in this PR) removes a type error. In this PR and pyright, Mixed[str, int] <: Mixed[object, int], and the outcome is incorrect (safe upcast removes a typing error), so probably Mixed should not be covariant in T.

Moreover, if we added a covariant constraint like def get(self) -> T, then master would incorrectly infer Mixed as invariant

I'm trying to say that IMO T should be inferred invariant in this case to reject the upcast shown above. That new definition should already impose contravariance, and only invariance remains if other uses reject that.

I'm trying to say that IMO T should be inferred invariant in this case to reject the upcast shown above.

I think that's just incorrect. Again, there's nothing inside the body of Mixed that should impose any constraints on the variance of T, so it should be covariant by default.

The relevant section of the typing spec states:

3. Create two specialized versions of the class. We’ll refer to these as upper and lower specializations. In both of these specializations, replace all type parameters other than the one being inferred by a dummy type instance (a concrete anonymous class that is assumed to meet the bounds or constraints of the type parameter). In the upper specialized class, specialize the target type parameter with an object instance. This specialization ignores the type parameter’s upper bound or constraints. In the lower specialized class, specialize the target type parameter with itself (i.e. the corresponding type argument is the type parameter itself).
4. Determine whether lower can be assigned to upper using normal assignability rules. If so, the target type parameter is covariant. If not, determine whether upper can be assigned to lower. If so, the target type parameter is contravariant. If neither of these combinations are assignable, the target type parameter is invariant.

So in our case, when we infer T, that means we should check whether

def [S] (self: Mixed[S, DummyU], key: S, val: DummyU) -> None: ...

being a subtype of

def [S] (self: Mixed[S, DummyU], key: S, val: DummyU) -> None: ...

Imposes any constraints on T. It doesn't, T doesn't even appear in the expression, and they are identical regardless.

(though "replace all type parameters other than the one being inferred by a dummy type instance" definitely does not mean substituting dummies for S, but that will not impact the outcome here)

Right. I'll edit my comment; but I do not see how it makes any difference whatsoever.