Skip to content

Fix crash on partial type used as context #19216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

ilevkivskyi
Copy link
Member

Fixes #19213

@sterliakov
Copy link
Collaborator

Are you sure it won't be better to check all type variables in return type and erase them when a violation is detected? Something like

diff --git a/mypy/checker.py b/mypy/checker.py
index e83473492..b3d47a9b8 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -160,6 +160,7 @@ from mypy.typeops import (
     false_only,
     fixup_partial_type,
     function_type,
+    get_type_vars,
     is_literal_type_like,
     is_singleton_type,
     make_simplified_union,
@@ -1296,13 +1297,17 @@ class TypeChecker(NodeVisitor[None], TypeCheckerSharedApi):
                         self.fail(
                             message_registry.RETURN_TYPE_CANNOT_BE_CONTRAVARIANT, typ.ret_type
                         )
-                    self.check_unbound_return_typevar(typ)
                 elif (
                     isinstance(original_typ.ret_type, TypeVarType) and original_typ.ret_type.values
                 ):
                     # Since type vars with values are expanded, the return type is changed
                     # to a raw value. This is a hack to get it back.
-                    self.check_unbound_return_typevar(original_typ)
+                    if not self.check_unbound_return_typevar(original_typ):
+                        typ = typ.copy_modified(ret_type=erase_typevars(typ.ret_type))
+                        defn.type = typ
+                if not self.check_unbound_return_typevar(typ):
+                    typ = typ.copy_modified(ret_type=erase_typevars(typ.ret_type))
+                    defn.type = typ
 
                 # Check that Generator functions have the appropriate return type.
                 if defn.is_generator:
@@ -1571,25 +1576,38 @@ class TypeChecker(NodeVisitor[None], TypeCheckerSharedApi):
                     return True
         return False
 
-    def check_unbound_return_typevar(self, typ: CallableType) -> None:
-        """Fails when the return typevar is not defined in arguments."""
-        if isinstance(typ.ret_type, TypeVarType) and typ.ret_type in typ.variables:
-            arg_type_visitor = CollectArgTypeVarTypes()
-            for argtype in typ.arg_types:
-                argtype.accept(arg_type_visitor)
+    def check_unbound_return_typevar(self, typ: CallableType) -> bool:
+        """Fails when the return typevar is not defined in arguments.
+
+        Returns:
+            success status (True if no violations were found).
+        """
+        tvars = get_type_vars(typ.ret_type)
+        if not tvars:
+            return True
+
+        arg_type_visitor = CollectArgTypeVarTypes()
+        for argtype in typ.arg_types:
+            argtype.accept(arg_type_visitor)
 
-            if typ.ret_type not in arg_type_visitor.arg_types:
+        fail = False
+        for tvar in tvars:
+            if tvar not in typ.variables:
+                continue
+            if tvar not in arg_type_visitor.arg_types:
+                fail = True
                 self.fail(message_registry.UNBOUND_TYPEVAR, typ.ret_type, code=TYPE_VAR)
-                upper_bound = get_proper_type(typ.ret_type.upper_bound)
+                upper_bound = get_proper_type(tvar.upper_bound)
                 if not (
                     isinstance(upper_bound, Instance)
                     and upper_bound.type.fullname == "builtins.object"
                 ):
                     self.note(
                         "Consider using the upper bound "
-                        f"{format_type(typ.ret_type.upper_bound, self.options)} instead",
+                        f"{format_type(tvar.upper_bound, self.options)} instead",
                         context=typ.ret_type,
                     )
+        return not fail
 
     def check_default_args(self, item: FuncItem, body_is_trivial: bool) -> None:
         for arg in item.arguments:

@ilevkivskyi
Copy link
Member Author

@sterliakov
Yes, I am sure. There can be other ways how a partial type ends up there.

Independently of that, we may want to tighten the unbound type var check as a separate thing. I remember I proposed to do this in the past (since IMO such signatures are meaningless), but some people (don't remember who and why) were against this, so we ended up only disabling the plain unbound type variable.

Copy link
Contributor

github-actions bot commented Jun 3, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

pip (https://github.com/pypa/pip): 1.12x faster (54.7s -> 48.6s in single noisy sample)

typeshed-stats (https://github.com/AlexWaygood/typeshed-stats): 1.78x slower (53.1s -> 94.5s in single noisy sample)

prefect (https://github.com/PrefectHQ/prefect): 1.06x slower (67.5s -> 71.7s in single noisy sample)

discord.py (https://github.com/Rapptz/discord.py): 1.07x slower (270.3s -> 288.4s in single noisy sample)

@sterliakov
Copy link
Collaborator

Hm, ok, maybe I'll try to look into that change later separately.

@ilevkivskyi ilevkivskyi merged commit 29d8f06 into python:master Jun 3, 2025
19 checks passed
@ilevkivskyi ilevkivskyi deleted the fix-partial-crash-2 branch June 3, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.16 regression] Internal error: must never apply partial type
2 participants