[stubtest] Check runtime availability of private types not marked @type_check_only (#19574)

Currently, stubtest ignores symbols that are 1) private and 2) not
present at runtime. Private names are often used for stub-only
constructs (which we don't want stubtest to complain about), but they
can also denote actual private runtime types. Without other context,
it's often ambiguous which category a private names falls into. For
class definitions, we can make the intent explicit by decorating the
class with `@type_check_only`. Typeshed already does this for the most
part, but there's still quite a few cases of stub-only private types
that aren't marked `@type_check_only` (especially in older stubs).

Going forward, I think it could a good idea to make stubtest start
checking for `@type_check_only` on private types that are not available
at runtime.

A few special cases are needed to avoid false positives on types that
cannot be marked with `@type_check_only`, namely:
* NewType declarations
* TypedDicts with fields that are not valid identifiers (and therefore
need to use the functional syntax)

~This PR creates quite a few new errors on typeshed (\~100 in stdlib,
\~300 in third-party stubs), which we'll probably want to sift through
before merging this.~ As of this writing, all typeshed hits have been
fixed.

Author: brianschubert

docs/source/stubtest.rst

@@ -183,6 +183,11 @@ The rest of this section documents the command line interface of stubtest.
 
     Ignore errors for whether an argument should or shouldn't be positional-only
 
+.. option:: --strict-type-check-only
+
+    Require :py:func:`@type_check_only <typing.type_check_only>` on private types
+    that are not present at runtime.
+
 .. option:: --allowlist FILE
 
     Use file as an allowlist. Can be passed multiple times to combine multiple

mypy/stubtest.py

@@ -13,6 +13,7 @@
 import functools
 import importlib
 import inspect
+import keyword
 import os
 import pkgutil
 import re
@@ -145,6 +146,11 @@ def is_disjoint_base_related(self) -> bool:
         # TODO: This is hacky, use error codes or something more resilient
         return "@disjoint_base" in self.message
 
+    def is_private_type_check_only_related(self) -> bool:
+        """Whether or not the error is related to @type_check_only on private types."""
+        # TODO: This is hacky, use error codes or something more resilient
+        return self.message.endswith('Maybe mark it as "@type_check_only"?')
+
     def get_description(self, concise: bool = False) -> str:
         """Returns a description of the error.
 
@@ -365,11 +371,7 @@ def verify_mypyfile(
         runtime_all_as_set = None
 
     # Check things in the stub
-    to_check = {
-        m
-        for m, o in stub.names.items()
-        if not o.module_hidden and (not is_probably_private(m) or hasattr(runtime, m))
-    }
+    to_check = {m for m, o in stub.names.items() if not o.module_hidden}
 
     def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
         """Heuristics to determine whether a name originates from another module."""
@@ -439,6 +441,15 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
             # Don't recursively check exported modules, since that leads to infinite recursion
             continue
         assert stub_entry is not None
+        if (
+            is_probably_private(entry)
+            and not hasattr(runtime, entry)
+            and not isinstance(stub_entry, Missing)
+            and not _is_decoratable(stub_entry)
+        ):
+            # Skip private names that don't exist at runtime and which cannot
+            # be marked with @type_check_only.
+            continue
         try:
             runtime_entry = getattr(runtime, entry, MISSING)
         except Exception:
@@ -448,6 +459,19 @@ def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
         yield from verify(stub_entry, runtime_entry, object_path + [entry])
 
 
+def _is_decoratable(stub: nodes.SymbolNode) -> bool:
+    if not isinstance(stub, nodes.TypeInfo):
+        return False
+    if stub.is_newtype:
+        return False
+    if stub.typeddict_type is not None:
+        return all(
+            name.isidentifier() and not keyword.iskeyword(name)
+            for name in stub.typeddict_type.items.keys()
+        )
+    return True
+
+
 def _verify_final(
     stub: nodes.TypeInfo, runtime: type[Any], object_path: list[str]
 ) -> Iterator[Error]:
@@ -639,7 +663,10 @@ def verify_typeinfo(
         return
 
     if isinstance(runtime, Missing):
-        yield Error(object_path, "is not present at runtime", stub, runtime, stub_desc=repr(stub))
+        msg = "is not present at runtime"
+        if is_probably_private(stub.name):
+            msg += '. Maybe mark it as "@type_check_only"?'
+        yield Error(object_path, msg, stub, runtime, stub_desc=repr(stub))
         return
     if not isinstance(runtime, type):
         # Yes, some runtime objects can be not types, no way to tell mypy about that.
@@ -2308,6 +2335,7 @@ class _Arguments:
     ignore_missing_stub: bool
     ignore_positional_only: bool
     ignore_disjoint_bases: bool
+    strict_type_check_only: bool
     allowlist: list[str]
     generate_allowlist: bool
     ignore_unused_allowlist: bool
@@ -2404,6 +2432,8 @@ def warning_callback(msg: str) -> None:
                 continue
             if args.ignore_disjoint_bases and error.is_disjoint_base_related():
                 continue
+            if not args.strict_type_check_only and error.is_private_type_check_only_related():
+                continue
             if error.object_desc in allowlist:
                 allowlist[error.object_desc] = True
                 continue
@@ -2500,6 +2530,11 @@ def parse_options(args: list[str]) -> _Arguments:
         action="store_true",
         help="Disable checks for PEP 800 @disjoint_base classes",
     )
+    parser.add_argument(
+        "--strict-type-check-only",
+        action="store_true",
+        help="Require @type_check_only on private types that are not present at runtime",
+    )
     parser.add_argument(
         "--allowlist",
         "--whitelist",

mypy/test/teststubtest.py

@@ -59,6 +59,7 @@ def __getitem__(self, typeargs: Any) -> object: ...
 
 Final = 0
 Literal = 0
+NewType = 0
 TypedDict = 0
 
 class TypeVar:
@@ -255,7 +256,7 @@ def test(*args: Any, **kwargs: Any) -> None:
         output = run_stubtest(
             stub="\n\n".join(textwrap.dedent(c.stub.lstrip("\n")) for c in cases),
             runtime="\n\n".join(textwrap.dedent(c.runtime.lstrip("\n")) for c in cases),
-            options=["--generate-allowlist"],
+            options=["--generate-allowlist", "--strict-type-check-only"],
         )
 
         actual_errors = set(output.splitlines())
@@ -904,8 +905,9 @@ def f(a, *args): ...
     def test_decorated_overload(self) -> Iterator[Case]:
         yield Case(
             stub="""
-            from typing import overload
+            from typing import overload, type_check_only
 
+            @type_check_only
             class _dec1:
                 def __init__(self, func: object) -> None: ...
                 def __call__(self, x: str) -> str: ...
@@ -921,6 +923,7 @@ def good1(unrelated: int, whatever: str) -> str: ...
         )
         yield Case(
             stub="""
+            @type_check_only
             class _dec2:
                 def __init__(self, func: object) -> None: ...
                 def __call__(self, x: str, y: int) -> str: ...
@@ -936,6 +939,7 @@ def good2(unrelated: int, whatever: str) -> str: ...
         )
         yield Case(
             stub="""
+            @type_check_only
             class _dec3:
                 def __init__(self, func: object) -> None: ...
                 def __call__(self, x: str, y: int) -> str: ...
@@ -1245,7 +1249,7 @@ def test_type_alias(self) -> Iterator[Case]:
             import collections.abc
             import re
             import typing
-            from typing import Callable, Dict, Generic, Iterable, List, Match, Tuple, TypeVar, Union
+            from typing import Callable, Dict, Generic, Iterable, List, Match, Tuple, TypeVar, Union, type_check_only
             """,
             runtime="""
             import collections.abc
@@ -1316,6 +1320,7 @@ class Y: ...
         yield Case(
             stub="""
             _T = TypeVar("_T")
+            @type_check_only
             class _Spam(Generic[_T]):
                 def foo(self) -> None: ...
             IntFood = _Spam[int]
@@ -1693,6 +1698,7 @@ def test_missing(self) -> Iterator[Case]:
         yield Case(stub="x = 5", runtime="", error="x")
         yield Case(stub="def f(): ...", runtime="", error="f")
         yield Case(stub="class X: ...", runtime="", error="X")
+        yield Case(stub="class _X: ...", runtime="", error="_X")
         yield Case(
             stub="""
             from typing import overload
@@ -1749,6 +1755,8 @@ def __delattr__(self, name: str, /) -> None: ...
             runtime="class FakeDelattrClass: ...",
             error="FakeDelattrClass.__delattr__",
         )
+        yield Case(stub="from typing import NewType", runtime="", error=None)
+        yield Case(stub="_Int = NewType('_Int', int)", runtime="", error=None)
 
     @collect_cases
     def test_missing_no_runtime_all(self) -> Iterator[Case]:
@@ -2375,8 +2383,9 @@ def test_special_subtype(self) -> Iterator[Case]:
         )
         yield Case(
             stub="""
-            from typing import TypedDict
+            from typing import TypedDict, type_check_only
 
+            @type_check_only
             class _Options(TypedDict):
                 a: str
                 b: int
@@ -2796,6 +2805,70 @@ def func2() -> None: ...
             runtime="def func2() -> None: ...",
             error="func2",
         )
+        # The same is true for private types
+        yield Case(
+            stub="""
+            @type_check_only
+            class _P1: ...
+            """,
+            runtime="",
+            error=None,
+        )
+        yield Case(
+            stub="""
+            @type_check_only
+            class _P2: ...
+            """,
+            runtime="class _P2: ...",
+            error="_P2",
+        )
+        # Private TypedDicts which do not exist at runtime must be decorated with
+        # '@type_check_only', unless they contain keys that are not valid identifiers.
+        yield Case(
+            stub="""
+            class _TD1(TypedDict): ...
+            """,
+            runtime="",
+            error="_TD1",
+        )
+        yield Case(
+            stub="""
+            _TD2 = TypedDict("_TD2", {"foo": int})
+            """,
+            runtime="",
+            error="_TD2",
+        )
+        yield Case(
+            stub="""
+            _TD3 = TypedDict("_TD3", {"foo-bar": int})
+            """,
+            runtime="",
+            error=None,
+        )
+        yield Case(
+            stub="""
+            _TD4 = TypedDict("_TD4", {"class": int})
+            """,
+            runtime="",
+            error=None,
+        )
+        # Private NamedTuples which do not exist at runtime must be decorated with
+        # '@type_check_only'.
+        yield Case(
+            stub="""
+            class _NT1(NamedTuple): ...
+            """,
+            runtime="",
+            error="_NT1",
+        )
+        yield Case(
+            stub="""
+            @type_check_only
+            class _NT2(NamedTuple): ...
+            """,
+            runtime="",
+            error=None,
+        )
         # A type that exists at runtime is allowed to alias a type marked
         # as '@type_check_only' in the stubs.
         yield Case(
@@ -2812,8 +2885,9 @@ class _X1: ...
     def test_type_default_protocol(self) -> Iterator[Case]:
         yield Case(
             stub="""
-            from typing import Protocol
+            from typing import Protocol, type_check_only
 
+            @type_check_only
             class _FormatterClass(Protocol):
                 def __call__(self, *, prog: str) -> HelpFormatter: ...