MyPy should report error if variable imported in TYPE_CHECKING block?

[sid-kap]

Currently, MyPy doesn't report an error when a variable is imported in the if TYPE_CHECKING block and then used elsewhere.
For example, MyPy accepts this code:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import NamedTuple

class C(NamedTuple):
    x: int
    y: int

even though, at runtime, it fails:

Traceback (most recent call last):
  File "type_checking_test.py", line 6, in <module>
    class C(NamedTuple):
NameError: name 'NamedTuple' is not defined

(I'm using MyPy version 0.650.)

Should this be fixed? I think this could be fixed by adding an is_type_checking_only variable to SymbolTableNode, which is True if the variable was imported or assigned within a TYPE_CHECKING block, and ensuring that this flag is false for any variables used in places other than type annotations in the actual code.

[ilevkivskyi]

Should this be fixed?

This is highly questionable, TBH. Although implementation is not hard, it still will require some work, while the value is relatively small. Also this fails immediately at runtime with a (relatively) clear error.

[sid-kap]

Thanks for your response! It doesn't always fail immediately—I have had situations where the imports are used somewhere other than the top-level, so the failure sometimes happens an hour into a long-running job.

I'd be interested to hear thoughts from other maintainers, but if you're sure this is not worth doing, then feel free to close the issue.

[gvanrossum]

I think it’s worth fixing.

[JukkaL]

I agree that this would be worth fixing.

The implementation would have to be clever enough to catch cast(A, x) if A is not defined at runtime but to allow cast('A', x).

If A is also defined in an else: block, it should not be treated as undefined at runtime:

if TYPE_CHECKING:
    from m import A
else:
    class A: pass

class B(A): pass  # Should be OK

It would also be nice to require quotes to be used in type annotations (if not using from __future__ import annotations).

[ilevkivskyi]

@JukkaL

It would also be nice to require quotes to be used in type annotations:

We already have #948 to track this.

[NiklasRosenstein]

I would like to raise this point again.

For a command-line application I am optimising global imports to increase its performance; that often means I need to move imports into a if TYPE_CHECKING block and repeat the import at the place in the code where the type is referenced by value.

Since Mypy doesn't catch that the value is undefined at runtime, if I make the mistake of not adding the local import, it results in a NameError at runtime that should be really easy for Mypy to catch.

With some pointers, I'm happy to look into a contribution for this fix. (I am very unfamiliar with the Mypy codebase).

[Viicos]

Related use case:

module1 has the following:

if TYPE_CHECKING:
    MyType = int

module2:

# No if TYPE_CHECKING block
from module1 import MyType

• As IDEs usually evaluate TYPE_CHECKING to True, it's hard to catch the resulting ImportError

[bluenote10]

@JelleZijlstra To quote from the linked duplicate issue #16587 (comment)

I don't think we should warn here. if TYPE_CHECKING means that the type checker should treat this condition as true, and therefore assume that FunctionType is imported. It's a structured way of lying to the type checker. And if you lie to the type checker, you're responsible for the consequences.

I would argue that this is a special case, because it is a required technique for breaking import cycles as documented. Without this feature, breaking import cycles cannot be done cleanly, or even the contrary: It is likely to cause bugs!

First a general observation: import cycles are in fact more prominent when using strict typing compared to untyped Python, because even techniques like dependency injection (which typically is intended to break runtime cyclic dependencies) now require imports, but just for the type annotations (i.e., untyped Python would not have these imports). Since import cycles are a more common problem now, it is sensible to provide a clean way to support breaking them.

Now without having a check that a symbol got only imported for type annotation usage, we are actually opening the door for a lot of runtime bugs! In other words, without this feature, the recommendation given in the documentation is actually very likely cause bugs, i.e., "type checks fine, but crashes at runtime" -- which is exactly what we want to prevent with type checking. To demonstrate:

from dataclasses import dataclass, field
from typing import TYPE_CHECKING

# Pure "type import" to break runtime import cycle e.g. for dependency injection.
if TYPE_CHECKING:
    from other_module import Foo

# The following usages are valid, because they only use the symbol in type
# annotation contexts. Note that such usages are common for dependency
# injection.

Alias = list[Foo]

def f1(x: Foo):
    ...

@dataclass
class ClassUsage1:
    foo: Foo

# On the other hand, all the following usages are invalid, because they make
# runtime use of the symbol. Unfortunately the type checker happily accepts
# all these usages in the same module, even though they are all bugs leading
# to crashes at runtime. Ideally the type checker should be aware of the fact
# that the symbol does not exist at runtime, and report all these usages as type
# check errors.

foo = Foo()

def f2(x: Foo = Foo()):
    ...

@dataclass
class ClassUsage2:
    foo: Foo = field(default_factory=lambda: Foo())

We've been running into such bugs rather frequently now.