type guarded value is same type as typing.TypeGuard[T]?

[A5rocks]

Bug Report

Okay so, let's say for a second you have this code:

from typing_extensions import TypeGuard

def type_guard_function(a: object) -> TypeGuard[int]:
    return isinstance(a, int)

reveal_type(type_guard_function)

n: object = 5
reveal_type(n)

if type_guard_function(n):
    reveal_type(n)
else:
    reveal_type(n)

Now then, it turns out that in this bit:

if type_guard_function(n):
    reveal_type(n)

mypy actually says n is a TypeGuard! If you look at ConditionalTypeBinder#frames during the reveal_type, you'll note a: {('Var', <mypy.nodes.Var object at 0x7f7a76e32580>): TypeGuard(builtins.int)}.

ConditionalTypeBinder stores types. To prove this to yourself, add a n = '5' after n: object = 5. You'll see a frame now looks like: {('Var', <mypy.nodes.Var object at 0x7f7a76e32580>): builtins.str} (note that since it's the same variable, it's the same memory address :P)...

Anyways, this seems completely off, because in types.py's TypeStrVisitor, you have this:

    def visit_type_guard_type(self, t: TypeGuardType) -> str:
        return 'TypeGuard[{}]'.format(t.type_guard.accept(self))

Here are the relevant lines to the above:

1. Type narrowing working around TypeGuardType:

mypy/mypy/checkexpr.py

Lines 4188 to 4190 in 7edcead

	if isinstance(restriction, TypeGuardType): # type: ignore[misc]
	# A type guard forces the new type even if it doesn't overlap the old.
	return restriction.type_guard

This is what causes reveal_type(n) in the if statement to be int.

2. The implementation that adds TypeGuardType to the value in the first place:

mypy/mypy/checker.py

Line 4147 in 7edcead

return {expr: TypeGuardType(node.callee.type_guard)}, {}

3. TypeStrVisitor#visit_type_guard_type

mypy/mypy/types.py

Lines 2197 to 2198 in 7edcead

	def visit_type_guard_type(self, t: TypeGuardType) -> str:
	return 'TypeGuard[{}]'.format(t.type_guard.accept(self))

Expected Behavior

This doesn't happen. TypeGuardType is only for TypeGuard[T]. Instead, there's another way to signal to type narrowing that a value became the way it is because of a type guard (necessary because of differences in behavior).

Actual Behavior

This does happen. Chaos ensues, causing things like #10647 and #10665. (confirmed this by removing relevant line number 2 and testing both -- no crashes. However, I did this as described in #10671 (comment) and well, the tests show the problem themselves.)

Your Environment

The source code.

I was redirected here by the contributing document, as this is a moderate (I think?) effort change, and I have no clue what exactly should change. Like, should type guarded values use their own ProperType? Should there be a bool flag somewhere? No clue.

Full disclaimer that I probably messed a few things up in my explanation, and that I may have misspoken a few terms. Hopefully I'm wrong and this isn't actually an issue 😰 .

[hauntsaninja]

Thanks, I was looking into fixing #10647 and found this, which was super helpful and helped me pick up context on this stuff.

After staring at this a little bit, I think #10496 was just confused. That is, I think TypeGuardType is not for TypeGuard[T] / is only to signal that a value was type narrowed via type guard / the TypeStrVisitor that got added in #10496 is just wrong / maybe it should be renamed TypeGuardedType. I think if we just fix the changes in #10496 to always look through to the type guarded type we will be happy. It's of course possible that the binder could track this state better, but in general I'm scared of the binder.

I'll try drafting up a PR and see what that surfaces, but let me know if I'm way off base.

[A5rocks]

That sounds about right to me! That it is a typo sounds conceivable, I don't believe TypeGuard[T] needs much special-casing (other than allowing it to be bool, but that's a typeshed thing), so it probably doesn't need a proper type to itself.