Clarifying that type checkers should not emit diagnostics in `if not TYPE_CHECKING`

It came up recently that the conformance suite asserts that type checkers should not emit diagnostics in if not TYPE_CHECKING blocks (or the else clause of if TYPE_CHECKING blocks), but this is not clearly specified anywhere. I think it’s the correct behavior, and we should clearly specify it. if not TYPE_CHECKING expresses clear intent to do things the type checker should just ignore.

Bringing it up for discussion here since that’s the process for a spec change. Does anyone have strong use cases for wanting some kind of type diagnostics inside if not TYPE_CHECKING?

7 Likes

For a contrarian perspective: if not TYPE_CHECKING is useful as a block which hides the types of the symbols to be checked against from the rest of the scope, rather than something which completely turns off type-checking.

I find myself using its counterpart, if TYPE_CHECKING, far more frequently as a kind of .pyi-within-a-.py, and never actually wanted type checking to be off. Anything I’d like to hide under not TYPE_CHECKING frequently still benefits from type-checking for internal consistency within the same not TYPE_CHECKING block; if I wanted to actually stop type-checking something, I’d reach for one of the multiple ways which already effectively turns type-checking off at all sorts of scopes and situations, such as var: Any, @no_type_check, __getattr__(self, name: str, /) → Any, def f(*args: Any, **kwargs: Any), or just not even run a type-checker.

I sometimes reach for a mypy plugin to handle items under not TYPE_CHECKING that require special reporting. Although it is unlikely mypy will prevent error diagnostics under not TYPE_CHECKING with a plugin, it would be unfortunate if the spec mandates this and mypy follows suit in the future.


EDIT: To propose something concrete for the spec, I’d prefer it to say something along the lines of

Type-checkers must NOT account for symbols defined under not TYPE_CHECKING or the else clause of if TYPE_CHECKING: ...\nelse: ... for establishing symbol existence or symbol types in the scope:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
  a: int = 1
else:
  a: str = ""
  b: str = ""

reveal_type(a)  # Note: `builtins.int`

if not TYPE_CHECKING:
  a: str = ""  # Reassignment not seen by type-checkers
  c: str = 1   # Spec should be silent on whether type-checkers do anything here
  
reveal_type(a)  # Note: `builtins.int`
reveal_type(b)  # Error: `b` is not defined
reveal_type(c)  # Error: `c` is not defined

I don’t know about completely forbidding type checkers from emitting any diagnostics here. There are cases where they probably should be allowed to emit diagnostics still. The most obvious one is if there’s invalid syntax recognized by a type checker.

I think a better rule here would be for type checkers to suppress any type related diagnostics within that condition, but allow any other diagnostics a typechecker has chosen to provide at their own discretion.

3 Likes

I agree; I mentioned “type diagnostics” specifically in the final question in my OP, but I guess I didn’t clarify in the first paragraph.

I’m not sure if we can provide a precise definition for which diagnostics are “type diagnostics”, but it also maybe isn’t necessary, if we provide some concrete examples in the conformance suite.

Yes, I think this is clearly implied by the existing language (and conformance tests) in the spec. It’s a necessary consequence of treating TYPE_CHECKING as statically True. But it wouldn’t hurt to be extra clear about it.

It seems like currently all type checkers effectively “turn off type checking” in if not TYPE_CHECKING blocks. Until recently ty did not, and we had several users surprised that we would emit any type diagnostic in a region which (in their mind) they had clearly asked the type checker to ignore. So I’m not sure that yours is the majority expectation – and I think this is an area where we have to pick some behavior.

Clearly the conformance suite can’t mandate this, since it doesn’t use mypy plugins. So I think this would always remain up to mypy’s discretion. And if we clarify that it’s only “type diagnostics” which are disabled in these regions, that seems alone sufficient to clarify that it is not required to suppress every possible diagnostic.

I partially agree here; I would also not expect any (type-related) diagnostics to be shown under not TYPE_CHECKING either, for the baseline behaviour of a type-checker.

However, I would not expect the spec to detail whether the lack of diagnostic visibility is because (1) they are emitted-but-suppressed, (2) the code is simply not checked, or (3) the type-checker treats everything underneath not TYPE_CHECKING as typed with Any. It sounds like the proposal is for (2) to be the conformant way and others to be non-conformant, but I would expect an end-user to not think about this at all.

Maybe this is a non-issue now, since as you point out no major type-checker emits errors here, but I’m not sure about the spec change proposed, as I do not know the purpose or usefulness of declaring a future type-checker as non-conformant if they emit certain errors that would be currently considered type-related errors, like Name `var` is not defined under not TYPE_CHECKING.

I’d like to think we can word it loosely enough, while still making the intention clear. This one’s a case where I’m happy to leave it mostly up to type checkers unless it becomes an issue for users requiring reconciliation, which I doubt will happen if the intent is clear.

I think the main issue with this is up until now, there hasn’t been that much specification about other static analysis a typechecker might offer, or rules that some typecheckers offer that aren’t at all based in type theory, such as the choice for some typechekcers to error upon implicit any (eg. Any as a default for generic parameters)

If you’re wanting specification language here, then I think a path forward would require differentiating between classes of diagnostics. Based on the diagnostics typecheckers currently offer, I think that’s something along the lines of these four categories:

  • rules that are directly in specification or logical conclusions of type theory and the specification.
  • rules that catch things that are unambiguously an error, even when everything is typed as Any (such as invalid syntax)
  • rules that prevent accidental gradual typing where fully static typing might be possible.
  • rules that catch common issues statically, but that aren’t always a mistake (such as mutable defaults or non-initialized slots)

of those, the second is the category I would say should still be enabled under if not TYPE_CHECKING, while the fourth one could be left to typecheckers. The first and third should be supressed.

I actually made some changes to some of my public code as a result of ty’s prior behavior there. While it wasn’t strictly a fix, it did result in easier-to-read code.

1 Like

Well, we had one user report an issue on our tracker. Were there any others? I don’t have a strong opinion on this change, but I also really don’t remember this coming up all that often for us

I would not support mandating (2) specifically. ty’s current approach is (1), not (2), and I don’t expect that to change. I expect the guidance here to be fairly loose and permit any of these approaches, as long as the result is that users don’t receive type diagnostics inside areas of code they’ve asked the type checker to pretend don’t exist.

3 Likes

I will be happy either way on this one, but the specific point about diagnostics seems low value to standardise. It doesn’t affect interoperability and if some type checker has a theory that users would be served better by diagnostics, that seems fine to me.

I do think the spec needs to be clearer that type checkers should avoid making types and symbols recorded in if not TYPE_CHECKING branches visible elsewhere in the analysis, because that does affect interoperability.

5 Likes

@carljm are you still interested in following up with a spec change here?

Yes, if we have consensus on what the spec and conformance suite should say. At the moment that consensus is not clear to me.

I’ve changed my mind since my OP here, and I agree with @hauntsaninja – that there’s no real compatibility motivation for the conformance suite to standardize whether type checkers can emit diagnostics in if not TYPE_CHECKING blocks, and we can safely just leave that up to type checkers. We don’t know the full range of possible diagnostics that a type checker might emit, and it feels unnecessarily presumptuous to declare that none could possibly be useful in an if not TYPE_CHECKING block. The important thing to standardize is that definitions in if not TYPE_CHECKING should be considered not to exist.

But “leave diagnostics in if TYPE_CHECKING up to type checkers” was previously proposed in a conformance suite PR, and multiple typing council members (you and @rchen152 ) pushed back on it a bit, so my question would be whether you are both OK with that approach.

2 Likes

Not specifying what diagnostics ought to be (not) emitted in if not TYPE_CHECKING seems like it would be bad for multi-checker compatibility. Today, if I’m a library maintainer and want to ensure my users can use any type checker of their choice, I can do something like:

if TYPE_CHECKING:
  class Foo[T]: ...
else:
  class Foo:
    # some crazy runtime logic that approximates generics.

and be confident that no type checker will error on whatever non-statically-analyzable code I put in that else block. IMO that’s a nice feature of if TYPE_CHECKING that is worth standardizing.

I’d also be okay with specifying something like “no type-related diagnostics should be emitted in if not TYPE_CHECKING” if we can specify that precisely enough, but that strikes me as challenging. E.g., emitting missing attribute errors would almost certainly be bad, given all the dynamic things you can do to add attributes on a class that type checkers can’t follow, but they’re not type errors, per se.

I don’t really understand the compatibility argument here. How would diagnostics in an if not TYPE_CHECKING block of a library affect users of that library?

The thing that would affect library users is whether the block is considered reachable/existing from the point of view of other code outside the block; that’s the part I definitely agree should be more clearly specified than it is now (though I think all type checkers agree on it anyway.)

No, you’re right, that didn’t make sense. I overthought and managed to confuse myself - sorry!

Any diagnostics in an if not TYPE_CHECKING block wouldn’t affect downstream users. They would affect the project itself if it’s running multiple type checkers over its own code, since it now has to make sure whatever is in that block satisfies every checker it’s running, which becomes harder than before if errors might appear in that block. To be fair, I don’t think it’s particularly common to check your own code with multiple checkers (versus checking typing tests that use your public interface), so the practical concerns here are fairly weak.

It still feels sort of un-intuitive to me for a type checker to not be silent in an if not TYPE_CHECKING block, but I see the argument for not being overly prescriptive and only standardizing what’s important for compatibility.