From 50944603afef1522b0777a26821de04859186a1b Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Sun, 25 Sep 2022 14:05:41 -0700 Subject: [PATCH] Fix crash with generic class definition in function (#13678) Fixes #12112. The reason why mypy was crashing with a "Must not defer during final iteration" error in the following snippet: from typing import TypeVar def test() -> None: T = TypeVar('T', bound='Foo') class Foo: def bar(self, foo: T) -> None: pass ...was because mypy did not seem to be updating the types of the `bar` callable on each pass: the `bind_function_type_variables` method in `typeanal.py` always returned the _old_ type variables instead of using the new updated ones we found by calling `self.lookup_qualified(...)`. This in turn prevented us from making any forward progress when mypy generated a CallableType containing a placedholder type variable. So, we repeated the semanal passes until we hit the limit and crashed. I opted to fix this by having the function always return the newly-bound TypeVarLikeType instead. (Hopefully this is safe -- the way mypy likes mutating types always makes it hard to reason about this sort of stuff). Interestingly, my fix for this bug introduced a regression in one of our existing tests: from typing import NamedTuple, TypeVar T = TypeVar("T") NT = NamedTuple("NT", [("key", int), ("value", T)]) # Test thinks the revealed type should be: # def [T] (key: builtins.int, value: T`-1) -> Tuple[builtins.int, T`-1, fallback=__main__.NT[T`-1]] # # ...but we started seeing: # def [T, _NT <: Tuple[builtins.int, T`-1]] (key: builtins.int, value: T`-1) -> Tuple[builtins.int, T`-1, fallback=test.WTF[T`-1]] reveal_type(NT) What seems to be happening here is that during the first pass, we add two type vars to the `tvar_scope` inside `bind_function_type_variables`: `T` with id -1 and `_NT` with id -2. But in the second pass, we lose track of the `T` typevar definition and/or introduce a fresh scope somewhere and infer `_NT` with id -1 instead? So now mypy thinks there are two type variables associated with this NamedTuple, which results in the screwed-up type definition. I wasn't really sure how to fix this, but I thought it was weird that: 1. We were using negative IDs instead of positive ones. (Class typevars are supposed to use the latter). 2. We weren't wrapping this whole thing in a new tvar scope frame, given we're nominally synthesizing a new class. So I did that, and the tests started passing? I wasn't able to repro this issue for TypedDicts, but opted to introduce a new tvar scope frame there as well for consistency. --- mypy/semanal.py | 80 +++++++++++++++------------- mypy/semanal_namedtuple.py | 3 +- mypy/typeanal.py | 12 ++--- test-data/unit/check-classes.test | 29 ++++++++++ test-data/unit/check-namedtuple.test | 2 +- test-data/unit/check-typeddict.test | 2 +- 6 files changed, 81 insertions(+), 47 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index acd962c674ee..81f50b8d75a5 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2752,30 +2752,32 @@ def analyze_namedtuple_assign(self, s: AssignmentStmt) -> bool: return False lvalue = s.lvalues[0] name = lvalue.name - internal_name, info, tvar_defs = self.named_tuple_analyzer.check_namedtuple( - s.rvalue, name, self.is_func_scope() - ) - if internal_name is None: - return False - if isinstance(lvalue, MemberExpr): - self.fail("NamedTuple type as an attribute is not supported", lvalue) - return False - if internal_name != name: - self.fail( - 'First argument to namedtuple() should be "{}", not "{}"'.format( - name, internal_name - ), - s.rvalue, - code=codes.NAME_MATCH, + namespace = self.qualified_name(name) + with self.tvar_scope_frame(self.tvar_scope.class_frame(namespace)): + internal_name, info, tvar_defs = self.named_tuple_analyzer.check_namedtuple( + s.rvalue, name, self.is_func_scope() ) + if internal_name is None: + return False + if isinstance(lvalue, MemberExpr): + self.fail("NamedTuple type as an attribute is not supported", lvalue) + return False + if internal_name != name: + self.fail( + 'First argument to namedtuple() should be "{}", not "{}"'.format( + name, internal_name + ), + s.rvalue, + code=codes.NAME_MATCH, + ) + return True + # Yes, it's a valid namedtuple, but defer if it is not ready. + if not info: + self.mark_incomplete(name, lvalue, becomes_typeinfo=True) + else: + self.setup_type_vars(info.defn, tvar_defs) + self.setup_alias_type_vars(info.defn) return True - # Yes, it's a valid namedtuple, but defer if it is not ready. - if not info: - self.mark_incomplete(name, lvalue, becomes_typeinfo=True) - else: - self.setup_type_vars(info.defn, tvar_defs) - self.setup_alias_type_vars(info.defn) - return True def analyze_typeddict_assign(self, s: AssignmentStmt) -> bool: """Check if s defines a typed dict.""" @@ -2789,22 +2791,24 @@ def analyze_typeddict_assign(self, s: AssignmentStmt) -> bool: return False lvalue = s.lvalues[0] name = lvalue.name - is_typed_dict, info, tvar_defs = self.typed_dict_analyzer.check_typeddict( - s.rvalue, name, self.is_func_scope() - ) - if not is_typed_dict: - return False - if isinstance(lvalue, MemberExpr): - self.fail("TypedDict type as attribute is not supported", lvalue) - return False - # Yes, it's a valid typed dict, but defer if it is not ready. - if not info: - self.mark_incomplete(name, lvalue, becomes_typeinfo=True) - else: - defn = info.defn - self.setup_type_vars(defn, tvar_defs) - self.setup_alias_type_vars(defn) - return True + namespace = self.qualified_name(name) + with self.tvar_scope_frame(self.tvar_scope.class_frame(namespace)): + is_typed_dict, info, tvar_defs = self.typed_dict_analyzer.check_typeddict( + s.rvalue, name, self.is_func_scope() + ) + if not is_typed_dict: + return False + if isinstance(lvalue, MemberExpr): + self.fail("TypedDict type as attribute is not supported", lvalue) + return False + # Yes, it's a valid typed dict, but defer if it is not ready. + if not info: + self.mark_incomplete(name, lvalue, becomes_typeinfo=True) + else: + defn = info.defn + self.setup_type_vars(defn, tvar_defs) + self.setup_alias_type_vars(defn) + return True def analyze_lvalues(self, s: AssignmentStmt) -> None: # We cannot use s.type, because analyze_simple_literal_type() will set it. diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index 4375602b5076..6cb42d6c3ede 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -321,11 +321,12 @@ def parse_namedtuple_args( ) -> None | (tuple[list[str], list[Type], list[Expression], str, list[TypeVarLikeType], bool]): """Parse a namedtuple() call into data needed to construct a type. - Returns a 5-tuple: + Returns a 6-tuple: - List of argument names - List of argument types - List of default values - First argument of namedtuple + - All typevars found in the field definition - Whether all types are ready. Return None if the definition didn't typecheck. diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 37f00841562f..0c84f2a0ffb5 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -1331,19 +1331,21 @@ def bind_function_type_variables( ) -> Sequence[TypeVarLikeType]: """Find the type variables of the function type and bind them in our tvar_scope""" if fun_type.variables: + defs = [] for var in fun_type.variables: var_node = self.lookup_qualified(var.name, defn) assert var_node, "Binding for function type variable not found within function" var_expr = var_node.node assert isinstance(var_expr, TypeVarLikeExpr) - self.tvar_scope.bind_new(var.name, var_expr) - return fun_type.variables + binding = self.tvar_scope.bind_new(var.name, var_expr) + defs.append(binding) + return defs typevars = self.infer_type_variables(fun_type) # Do not define a new type variable if already defined in scope. typevars = [ (name, tvar) for name, tvar in typevars if not self.is_defined_type_var(name, defn) ] - defs: list[TypeVarLikeType] = [] + defs = [] for name, tvar in typevars: if not self.tvar_scope.allow_binding(tvar.fullname): self.fail( @@ -1351,9 +1353,7 @@ def bind_function_type_variables( defn, code=codes.VALID_TYPE, ) - self.tvar_scope.bind_new(name, tvar) - binding = self.tvar_scope.get_binding(tvar.fullname) - assert binding is not None + binding = self.tvar_scope.bind_new(name, tvar) defs.append(binding) return defs diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 5f1c23b756ed..30a900d63f2a 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -1060,6 +1060,35 @@ def f() -> None: a.g(a) # E: Too many arguments for "g" of "A" [targets __main__, __main__.f] +[case testGenericClassWithinFunction] +from typing import TypeVar + +def test() -> None: + T = TypeVar('T', bound='Foo') + class Foo: + def returns_int(self) -> int: + return 0 + + def bar(self, foo: T) -> T: + x: T = foo + reveal_type(x) # N: Revealed type is "T`-1" + reveal_type(x.returns_int()) # N: Revealed type is "builtins.int" + return foo + reveal_type(Foo.bar) # N: Revealed type is "def [T <: __main__.Foo@5] (self: __main__.Foo@5, foo: T`-1) -> T`-1" + +[case testGenericClassWithInvalidTypevarUseWithinFunction] +from typing import TypeVar + +def test() -> None: + T = TypeVar('T', bound='Foo') + class Foo: + invalid: T # E: Type variable "T" is unbound \ + # N: (Hint: Use "Generic[T]" or "Protocol[T]" base class to bind "T" inside a class) \ + # N: (Hint: Use "T" in function signature to bind "T" inside a function) + + def bar(self, foo: T) -> T: + pass + [case testConstructNestedClass] import typing class A: diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index e4f75f57280c..4552cfb118cc 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -1284,7 +1284,7 @@ from typing import NamedTuple, TypeVar T = TypeVar("T") NT = NamedTuple("NT", [("key", int), ("value", T)]) -reveal_type(NT) # N: Revealed type is "def [T] (key: builtins.int, value: T`-1) -> Tuple[builtins.int, T`-1, fallback=__main__.NT[T`-1]]" +reveal_type(NT) # N: Revealed type is "def [T] (key: builtins.int, value: T`1) -> Tuple[builtins.int, T`1, fallback=__main__.NT[T`1]]" nts: NT[str] reveal_type(nts) # N: Revealed type is "Tuple[builtins.int, builtins.str, fallback=__main__.NT[builtins.str]]" diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index 7fba4da071f3..5bfe9f4c5555 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -2550,7 +2550,7 @@ from typing import TypedDict, TypeVar T = TypeVar("T") TD = TypedDict("TD", {"key": int, "value": T}) -reveal_type(TD) # N: Revealed type is "def [T] (*, key: builtins.int, value: T`-1) -> TypedDict('__main__.TD', {'key': builtins.int, 'value': T`-1})" +reveal_type(TD) # N: Revealed type is "def [T] (*, key: builtins.int, value: T`1) -> TypedDict('__main__.TD', {'key': builtins.int, 'value': T`1})" tds: TD[str] reveal_type(tds) # N: Revealed type is "TypedDict('__main__.TD', {'key': builtins.int, 'value': builtins.str})"