[ty] Add new hover mdtest assertion

[dcreager]

This PR adds a new hover mdtest assertion. This exercises the same logic as the hover LSP action. It's useful for when reveal_type would display a different infered type. (reveal_type is not transparent to bidirectional typing, so inserting a reveal_type call can sometimes change the type that we infer for an expression!)

I used Claude to help write large parts of this, but I have reviewed each step and the resulting diff in detail.

[github-actions]

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

[github-actions]

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[AlexWaygood]

Interesting! I haven't reviewed the new ty_test code thoroughly yet, but would you be able to give some more examples of where the current behaviour of reveal_type is problematic?

[AlexWaygood]

This example confuses me somewhat, because the introduction to this mdtest suite states that

You can use the hover assertion to test the infered type of an expression

But dict[str, int] is not the inferred type of {} here. The inferred type of {} here is dict[Unknown, Unknown] (the answer reveal_type gave), but we allow {} to be passed into the x parameter of f because dict[Unknown, Unknown] is assignable to dict[str, int], the annotation for the x parameter.

[dcreager]

As of #20635, we're doing bidirectional type checking for call arguments, and using the parameter annotation as the type context. And as of #20523, we're using the type context to infer a more precise type for dictionary literals. So dict[str, int] really is the infered type of {} here, when it's passed directly in as the argument. But when passing it to reveal_type, the parameter annotation is just _T, and so we don't have the more precise type context.

That might suggest that we want to propagate the type context through these kinds of inner calls (not just for reveal_type), but even then, with bidi checking it's now the case that we can have different infered types for the same expression depending on how its used, and it's useful to have an assertion that cannot influence that type context in any way.

[AlexWaygood]

That might suggest that we want to propagate the type context through these kinds of inner calls (not just for reveal_type)

That sounds like it might make sense to me... It seems quite counterintuitive to me that hovering over the first [""] in the playground for this example gives a different type than hovering over the second [""]:

from typing import reveal_type

def needs_list_of_strings(x: list[str | None]): ...

def identity[T](x: T) -> T:
    return x

needs_list_of_strings([""])
needs_list_of_strings(identity([""]))

https://play.ty.dev/53299738-b406-4e46-ba40-c0240225f0ae

with bidi checking it's now the case that we can have different infered types for the same expression depending on how its used, and it's useful to have an assertion that cannot influence that type context in any way

Fair enough! The additional code in ty_test to make it work also isn't free, though -- there's a maintenance burden there, and a small cognitive burden (I now have to think about whether a reveal_type assertion or a hover assertion will be more appropriate when I'm writing my tests!). I still feel like I'd be interested in a few more examples of where the current reveal_type behaviour is causing issues for your generics work.

[AlexWaygood]

As of #20635, we're doing bidirectional type checking for call arguments, and using the parameter annotation as the type context. And as of #20523, we're using the type context to infer a more precise type for dictionary literals. So dict[str, int] really is the infered type of {} here, when it's passed directly in as the argument. But when passing it to reveal_type, the parameter annotation is just _T, and so we don't have the more precise type context.

Thanks BTW -- I hadn't fully internalised that this was the consequence of those changes! Makes sense.

[sharkdp]

That might suggest that we want to propagate the type context through these kinds of inner calls (not just for reveal_type)

Yes, this sounds like something that we're going to want/need anyway

but even then, with bidi checking it's now the case that we can have different infered types for the same expression depending on how its used, and it's useful to have an assertion that cannot influence that type context in any way.

… but assuming that we would already propagate the type context through a reveal_type call (which acts like an identity function), we could also solve this use case with reveal_type, right? Then there would be no need for hover-assertions anymore? You could then do:

x: list[Literal[1]] = reveal_type([1, 1])  # revealed: list[Literal[1]]
y: list[int] = reveal_type([1, 1])  # revealed: list[int]

[dcreager]

assuming that we would already propagate the type context through a reveal_type call (which acts like an identity function), we could also solve this use case with reveal_type, right? Then there would be no need for hover-assertions anymore? You could then do:

Yes, but... 😂

The particular example I want to write a (hover) test for is this:

def f[T](x: dict[str, T]) -> None: ...

# TODO: should be dict[str, Unknown]
# ↓ hover: dict[str, T@f]
f({})

In a perfect world, I should be able to do this:

# TODO: should be dict[str, Unknown]
# revealed: dict[str, T@f]
f(reveal_type({}))

But this example is more complex than the annotated assignments you mention. We don't just need to propagate the type context through from the outer call to the inner call; we also need to solve the specialization inference for both calls simultaneously (T@f and _T@reveal_type). (Or maybe allow that the inner one can refer to typevars of the outer one?)

I agree that we need to make that work — but it also seemed worth having an escape hatch that lets us test these expressions without relying on all of that machinery working correctly.

Maybe the escape hatch is to write that test over in ty_ide, where we already have hover tests that use <CURSOR> to indicate where the hover is triggered? But that also seemed less than ideal, since the test wouldn't really be exercising the LSP action; it's a test of our type inference behavior, which seems to properly belong in a ty_python_semantic mdtest.

[dcreager]

Maybe the escape hatch is to write that test over in ty_ide

Or alternatively, maybe the escape hatch is just to hover in the playground and write in the PR description that I did that

[sharkdp]

I am certainly not opposed to this approach here, but it does feel like a rather large addition (judging from the line diff alone) for a seemingly small use case.

over in ty_ide, where we already have hover tests that use <CURSOR> to indicate where the hover is triggered

Maybe there is actually interest in porting those tests to your framework here? That would certainly be a strong argument in favor.

[AlexWaygood]

I am certainly not opposed to this approach here, but it does feel like a rather large addition (judging from the line diff alone) for a seemingly small use case.

(I'm also a little surprised that mypy, for example, has never needed this feature in its test suite. Mypy has a test framework that's pretty darn similar to mdtest, and a test suite that's much bigger than ours, but it still just uses reveal_type nearly everywhere)

[MichaReiser]

Having the ability to write hover tests in mdtests is nice. However, it adds a bit of confusion: Am I supposed to write a mdtest-hover test or a ty_ide hover test when making changes to hover and why? Do we need both?

Maybe there is actually interest in porting those tests to your framework here? That would certainly be a strong argument in favor.

Having the ability to write them in mdtests would certainly be nice. However, they often test for more than just the type (e.g. doc comments). This makes it difficult to replace them entirely unless we support snapshotting the entire hover but that doesn't seem that urgent right now.

[MichaReiser]

Should these tests be in ty_ide? I'd find it confusing that a change in ty_ide can break a test in ty_python_semantic. It would probably take me a while to realize that a test in ty_python_semantic isn't failing because of a change in ty_python_semantic (which would be the first place where I go look), but because of a change in ty_ide.

[MichaReiser]

I don't have a good suggestion as an alternative, but a downside of the arrow is that I'd always have to copy paste it from somewhere because I don't know how to type it on a keyboard.

[sharkdp]

see #20786 (comment) :-)