Make sure there is a useful error on impossible TypeGuard

Turns out that mypy plugin system doesn't give us an opportunity to
change the typeguard when it relies on a Type Var.

This is because we only know the value of the type var where it's
called, which is the get_method/function_hook but those can only change
the return type and don't have the ability to change the type guard

docs/api/changelog.rst

@@ -3,6 +3,13 @@
 Changelog
 ---------
 
+.. _release-0.5.4:
+
+0.5.4 - TBD
+    * Will now check return types for methods and functions more thorouhgly
+    * Will throw errors if a type guard is used with a concrete annotation that uses
+      a type var (mypy plugin system is limited in a way that makes this impossible to implement)
+
 .. _release-0.5.3:
 
 0.5.3 - 25 May 2024

extended_mypy_django_plugin/plugin/_plugin.py

@@ -12,11 +12,13 @@
     ClassDefContext,
     DynamicClassDefContext,
     FunctionContext,
+    FunctionSigContext,
     MethodContext,
+    MethodSigContext,
 )
 from mypy.semanal import SemanticAnalyzer
 from mypy.typeanal import TypeAnalyser
-from mypy.types import CallableType, Instance
+from mypy.types import CallableType, FunctionLike, Instance
 from mypy.types import Type as MypyType
 from mypy_django_plugin import main
 from mypy_django_plugin.django.context import DjangoContext
@@ -269,6 +271,7 @@ def run(self, ctx: AnalyzeTypeContext) -> MypyType:
 
             elif name is Known.DEFAULT_QUERYSET:
                 method = type_analyzer.find_default_queryset
+
             else:
                 assert_never(name)
 
@@ -299,6 +302,9 @@ class SharedCallableHookLogic:
         """
         Shared logic for modifying the return type of methods and functions that use a concrete
         annotation with a type variable.
+
+        Note that the signature hook will already raise errors if a concrete annotation is
+        used with a type var in a type guard.
         """
 
         def __init__(self, fullname: str, plugin: "ExtendedMypyStubs") -> None:
@@ -377,3 +383,87 @@ def run(self, ctx: FunctionContext) -> MypyType:
                 return self.super_hook(ctx)
 
             return ctx.default_return_type
+
+    class SharedSignatureHookLogic:
+        """
+        Shared logic for modifying the signature of methods and functions.
+
+        This is only used to find cases where a concrete annotation with a type var
+        is used in a type guard.
+
+        In this situation the mypy plugin system does not provide an opportunity to fully resolve
+        the type guard.
+        """
+
+        def __init__(self, fullname: str, plugin: "ExtendedMypyStubs") -> None:
+            self.plugin = plugin
+            self.store = plugin.store
+            self.fullname = fullname
+
+        def choose(self) -> bool:
+            """
+            Only choose methods and functions that are returning a type guard
+            """
+            if self.fullname.startswith("builtins."):
+                return False
+
+            sym = self.plugin.lookup_fully_qualified(self.fullname)
+            if not sym or not sym.node:
+                return False
+
+            call = getattr(sym.node, "type", None)
+            if not isinstance(call, CallableType):
+                return False
+
+            return call.type_guard is not None
+
+        def run(self, ctx: MethodSigContext | FunctionSigContext) -> MypyType | None:
+            assert isinstance(ctx.api, TypeChecker)
+
+            type_checking = actions.TypeChecking(
+                self.store,
+                api=ctx.api,
+                lookup_info=self.plugin._lookup_info,
+            )
+
+            return type_checking.check_typeguard(ctx.context)
+
+    @_hook.hook
+    class get_function_signature_hook(Hook[FunctionSigContext, FunctionLike]):
+        def extra_init(self) -> None:
+            self.shared_logic = self.plugin.SharedSignatureHookLogic(
+                fullname=self.fullname, plugin=self.plugin
+            )
+
+        def choose(self) -> bool:
+            return self.shared_logic.choose()
+
+        def run(self, ctx: FunctionSigContext) -> FunctionLike:
+            result = self.shared_logic.run(ctx)
+            if result is not None:
+                return ctx.default_signature.copy_modified(ret_type=result)
+
+            if self.super_hook is not None:
+                return self.super_hook(ctx)
+
+            return ctx.default_signature
+
+    @_hook.hook
+    class get_method_signature_hook(Hook[MethodSigContext, FunctionLike]):
+        def extra_init(self) -> None:
+            self.shared_logic = self.plugin.SharedSignatureHookLogic(
+                fullname=self.fullname, plugin=self.plugin
+            )
+
+        def choose(self) -> bool:
+            return self.shared_logic.choose()
+
+        def run(self, ctx: MethodSigContext) -> FunctionLike:
+            result = self.shared_logic.run(ctx)
+            if result is not None:
+                return ctx.default_signature.copy_modified(ret_type=result)
+
+            if self.super_hook is not None:
+                return self.super_hook(ctx)
+
+            return ctx.default_signature

extended_mypy_django_plugin/plugin/actions/_type_checker.py

@@ -62,6 +62,7 @@ def _find_type_vars(
 @dataclasses.dataclass
 class BasicTypeInfo:
     is_type: bool
+    is_guard: bool
     api: TypeChecker
     func: CallableType
 
@@ -72,9 +73,14 @@ class BasicTypeInfo:
     @classmethod
     def create(cls, api: TypeChecker, func: CallableType, item: MypyType | None = None) -> Self:
         is_type: bool = False
+        is_guard: bool = False
 
         if item is None:
-            item = func.ret_type
+            if func.type_guard:
+                is_guard = True
+                item = func.type_guard
+            else:
+                item = func.ret_type
 
         item = get_proper_type(item)
         if isinstance(item, TypeType):
@@ -102,6 +108,7 @@ def create(cls, api: TypeChecker, func: CallableType, item: MypyType | None = No
             func=func,
             item=item,
             is_type=is_type,
+            is_guard=is_guard,
             type_vars=type_vars,
             concrete_annotation=concrete_annotation,
         )
@@ -189,11 +196,30 @@ def _get_info(self, context: Context) -> BasicTypeInfo | None:
 
         return BasicTypeInfo.create(api=self.api, func=func)
 
+    def check_typeguard(self, context: Context) -> MypyType | None:
+        info = self._get_info(context)
+        if info is None:
+            return None
+
+        if info.is_guard and info.type_vars and info.contains_concrete_annotation:
+            # Mypy plugin system doesn't currently provide an opportunity to resolve a type guard when it's for a concrete annotation that uses a type var
+            self.api.fail(
+                "Can't use a TypeGuard that uses a Concrete Annotation that uses type variables",
+                context,
+            )
+            return AnyType(TypeOfAny.from_error)
+
+        return None
+
     def modify_return_type(self, ctx: MethodContext | FunctionContext) -> MypyType | None:
         info = self._get_info(ctx.context)
         if info is None:
             return None
 
+        if info.is_guard and info.type_vars and info.concrete_annotation is not None:
+            # Mypy plugin system doesn't currently provide an opportunity to resolve a type guard when it's for a concrete annotation that uses a type var
+            return None
+
         if not info.contains_concrete_annotation:
             return None
 

scripts/test_helpers/extended_mypy_django_plugin_test_driver/output_builder.py

@@ -1,7 +1,12 @@
+import importlib.metadata
 from collections.abc import Iterator
 
 from pytest_mypy_plugins import OutputMatcher
-from pytest_mypy_plugins.utils import DaemonOutputMatcher, FileOutputMatcher
+from pytest_mypy_plugins.utils import (
+    DaemonOutputMatcher,
+    FileOutputMatcher,
+    extract_output_matchers_from_out,
+)
 from typing_extensions import Self
 
 
@@ -57,14 +62,27 @@ def daemon_should_not_restart(self) -> Self:
     def on(self, path: str) -> Self:
         return self.__class__(build=self._build, target_file=path)
 
+    def from_out(self, out: str, regex: bool = False) -> Self:
+        self._build.result.extend(
+            extract_output_matchers_from_out(
+                out, {}, regex=regex, for_daemon=self._build.for_daemon
+            )
+        )
+        return self
+
     def add_revealed_type(self, lnum: int, revealed_type: str) -> Self:
+        if importlib.metadata.version("mypy") == "1.4.0":
+            revealed_type = revealed_type.replace("type[", "Type[")
         assert self.target_file is not None
         self._build.add(
             self.target_file, lnum, None, "note", f'Revealed type is "{revealed_type}"'
         )
         return self
 
     def change_revealed_type(self, lnum: int, message: str) -> Self:
+        if importlib.metadata.version("mypy") == "1.4.0":
+            message = message.replace("type[", "Type[")
+
         assert self.target_file is not None
 
         found: list[FileOutputMatcher] = []

tests/test_concrete_annotations.py

@@ -9,27 +9,50 @@ def _(expected: OutputBuilder) -> None:
                 "main.py",
                 """
                 from extended_mypy_django_plugin import Concrete, ConcreteQuerySet, DefaultQuerySet
+                from typing import cast, TypeGuard
 
-                from myapp.models import Parent
+                from myapp.models import Parent, Child1
 
                 models: Concrete[Parent]
                 reveal_type(models)
 
                 qs: ConcreteQuerySet[Parent]
                 reveal_type(qs)
+
+                def check_cls_with_type_guard(cls: type[Parent]) -> TypeGuard[type[Concrete[Parent]]]:
+                    return True
+
+                def check_instance_with_type_guard(cls: Parent) -> TypeGuard[Concrete[Parent]]:
+                    return True
+
+                cls: type[Parent] = Child1
+                assert check_cls_with_type_guard(cls)
+                reveal_type(cls)
+
+                instance: Parent = cast(Child1, None)
+                assert check_instance_with_type_guard(instance)
+                reveal_type(instance)
                 """,
             )
 
             (
                 expected.on("main.py")
                 .add_revealed_type(
-                    6,
+                    7,
                     "Union[myapp.models.Child1, myapp.models.Child2, myapp.models.Child3, myapp2.models.ChildOther]",
                 )
                 .add_revealed_type(
-                    9,
+                    10,
                     "Union[django.db.models.query._QuerySet[myapp.models.Child1, myapp.models.Child1], myapp.models.Child2QuerySet, django.db.models.query._QuerySet[myapp.models.Child3, myapp.models.Child3], django.db.models.query._QuerySet[myapp2.models.ChildOther, myapp2.models.ChildOther]]",
                 )
+                .add_revealed_type(
+                    20,
+                    "Union[type[myapp.models.Child1], type[myapp.models.Child2], type[myapp.models.Child3], type[myapp2.models.ChildOther]]",
+                )
+                .add_revealed_type(
+                    24,
+                    "Union[myapp.models.Child1, myapp.models.Child2, myapp.models.Child3, myapp2.models.ChildOther]",
+                )
             )
 
     def test_sees_apps_removed_when_they_still_exist_but_no_longer_installed(

tests/test_errors.py

@@ -0,0 +1,89 @@
+import importlib.metadata
+
+from extended_mypy_django_plugin_test_driver import OutputBuilder, Scenario
+
+
+class TestErrors:
+    def test_cant_use_typevar_concrete_annotation_in_function_or_method_typeguard(
+        self, scenario: Scenario
+    ) -> None:
+        @scenario.run_and_check_mypy_after
+        def _(expected: OutputBuilder) -> None:
+            scenario.make_file(
+                "main.py",
+                """
+                from typing import TypeGuard, TypeVar, cast
+
+                from myapp.models import Child1, Parent
+
+                from extended_mypy_django_plugin import Concrete, ConcreteQuerySet, DefaultQuerySet
+
+                T_Parent = Concrete.type_var("T_Parent", Parent)
+
+                def function_with_type_typeguard(
+                    cls: type[Parent], expect: type[T_Parent]
+                ) -> TypeGuard[type[Concrete[T_Parent]]]:
+                    return hasattr(cls, "objects")
+
+                cls1: type[Parent] = Child1
+                assert function_with_type_typeguard(cls1, Parent)
+                reveal_type(cls1)
+
+                def function_with_instance_typeguard(
+                    instance: Parent, expect: type[T_Parent]
+                ) -> TypeGuard[Concrete[T_Parent]]:
+                    return True
+
+                instance1: Parent = cast(Child1, None)
+                assert function_with_instance_typeguard(instance1, Parent)
+                reveal_type(instance1)
+
+                class Logic:
+                    def method_with_type_typeguard(
+                        self, cls: type[Parent], expect: type[T_Parent]
+                    ) -> TypeGuard[type[Concrete[T_Parent]]]:
+                        return hasattr(cls, "objects")
+
+                    def method_with_instance_typeguard(
+                        self, instance: T_Parent, expect: type[T_Parent]
+                    ) -> TypeGuard[Concrete[T_Parent]]:
+                        return True
+
+                logic = Logic()
+                cls2: type[Parent] = Child1
+                assert logic.method_with_type_typeguard(cls2, Parent)
+                reveal_type(cls2)
+
+                instance2: Parent = cast(Child1, None)
+                assert logic.method_with_instance_typeguard(instance2, Parent)
+                reveal_type(instance2)
+                """,
+            )
+
+            out = """
+            main:15: error: Can't use a TypeGuard that uses a Concrete Annotation that uses type variables  [misc]
+            main:15: error: Value of type variable "T_Parent" of "function_with_type_typeguard" cannot be "Parent"  [type-var]
+            main:15: error: Only concrete class can be given where "type[Parent]" is expected  [type-abstract]
+            main:16: note: Revealed type is "type[Concrete?[T_Parent?]]"
+            main:24: error: Can't use a TypeGuard that uses a Concrete Annotation that uses type variables  [misc]
+            main:24: error: Value of type variable "T_Parent" of "function_with_instance_typeguard" cannot be "Parent"  [type-var]
+            main:24: error: Only concrete class can be given where "type[Parent]" is expected  [type-abstract]
+            main:25: note: Revealed type is "Concrete?[T_Parent?]"
+            main:40: error: Can't use a TypeGuard that uses a Concrete Annotation that uses type variables  [misc]
+            main:40: error: Value of type variable "T_Parent" of "method_with_type_typeguard" of "Logic" cannot be "Parent"  [type-var]
+            main:40: error: Only concrete class can be given where "type[Parent]" is expected  [type-abstract]
+            main:41: note: Revealed type is "type[Concrete?[T_Parent?]]"
+            main:44: error: Can't use a TypeGuard that uses a Concrete Annotation that uses type variables  [misc]
+            main:44: error: Value of type variable "T_Parent" of "method_with_instance_typeguard" of "Logic" cannot be "Parent"  [type-var]
+            main:44: error: Only concrete class can be given where "type[Parent]" is expected  [type-abstract]
+            main:45: note: Revealed type is "Concrete?[T_Parent?]"
+            """
+
+            if importlib.metadata.version("mypy") == "1.4.0":
+                out = "\n".join(
+                    line
+                    for line in out.split("\n")
+                    if "Only concrete class can be given" not in line
+                ).replace('Revealed type is "type', 'Revealed type is "Type')
+
+            expected.from_out(out)