From 7569d88d0d650be73070c2536ddc3746d61237dd Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Mon, 31 Oct 2022 07:24:42 -0700 Subject: [PATCH] Fix crash with malformed TypedDicts and disllow-any-expr (#13963) Fixes #13066 During the semanal phase, mypy opts to ignore and skip processing any malformed or illegal statements inside of a TypedDict class definition, such as method definitions. Skipping semanal analysis on these statements can cause any number of odd downstream problems: the type-checking phase assumes that all semanal-only semantic constructs (e.g. FakeInfo) have been purged by this point, and so can crash at any point once this precondition has been violated. This diff opts to solve this problem by filtering down the list of statements so we keep only the ones we know are legal within a TypedDict definition. The other possible solution to this problem is to modify mypy so we skip checking TypedDict class bodies entirely during type checking and fine-grained deps analysis. Doing this would also let address #10007 and supersede my other diff #13732. I decided against doing this for now because: 1. I wasn't sure if this was actually safe, especially in the fine-grained deps phase and for mypyc. 2. I think no matter what, the semanal phase should not leak semanal-only types: relaxing this postcondition would make it harder to reason about mypy. So, we'd probably want to make this change regardless of what we do in the later phases. --- mypy/semanal_typeddict.py | 32 +++++++++++++++++++++-------- test-data/unit/check-typeddict.test | 13 ++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/mypy/semanal_typeddict.py b/mypy/semanal_typeddict.py index fd6b1bbd2bbf..b864c2a30615 100644 --- a/mypy/semanal_typeddict.py +++ b/mypy/semanal_typeddict.py @@ -23,6 +23,7 @@ NameExpr, PassStmt, RefExpr, + Statement, StrExpr, TempNode, TupleExpr, @@ -93,7 +94,7 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> tuple[bool, TypeInfo | N and defn.base_type_exprs[0].fullname in TPDICT_NAMES ): # Building a new TypedDict - fields, types, required_keys = self.analyze_typeddict_classdef_fields(defn) + fields, types, statements, required_keys = self.analyze_typeddict_classdef_fields(defn) if fields is None: return True, None # Defer info = self.build_typeddict_typeinfo( @@ -102,6 +103,7 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> tuple[bool, TypeInfo | N defn.analyzed = TypedDictExpr(info) defn.analyzed.line = defn.line defn.analyzed.column = defn.column + defn.defs.body = statements return True, info # Extending/merging existing TypedDicts @@ -139,7 +141,12 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> tuple[bool, TypeInfo | N # Iterate over bases in reverse order so that leftmost base class' keys take precedence for base in reversed(typeddict_bases): self.add_keys_and_types_from_base(base, keys, types, required_keys, defn) - new_keys, new_types, new_required_keys = self.analyze_typeddict_classdef_fields(defn, keys) + ( + new_keys, + new_types, + new_statements, + new_required_keys, + ) = self.analyze_typeddict_classdef_fields(defn, keys) if new_keys is None: return True, None # Defer keys.extend(new_keys) @@ -151,6 +158,7 @@ def analyze_typeddict_classdef(self, defn: ClassDef) -> tuple[bool, TypeInfo | N defn.analyzed = TypedDictExpr(info) defn.analyzed.line = defn.line defn.analyzed.column = defn.column + defn.defs.body = new_statements return True, info def add_keys_and_types_from_base( @@ -250,7 +258,7 @@ def map_items_to_base( def analyze_typeddict_classdef_fields( self, defn: ClassDef, oldfields: list[str] | None = None - ) -> tuple[list[str] | None, list[Type], set[str]]: + ) -> tuple[list[str] | None, list[Type], list[Statement], set[str]]: """Analyze fields defined in a TypedDict class definition. This doesn't consider inherited fields (if any). Also consider totality, @@ -259,17 +267,22 @@ def analyze_typeddict_classdef_fields( Return tuple with these items: * List of keys (or None if found an incomplete reference --> deferral) * List of types for each key + * List of statements from defn.defs.body that are legally allowed to be a + part of a TypedDict definition * Set of required keys """ fields: list[str] = [] types: list[Type] = [] + statements: list[Statement] = [] for stmt in defn.defs.body: if not isinstance(stmt, AssignmentStmt): - # Still allow pass or ... (for empty TypedDict's). - if not isinstance(stmt, PassStmt) and not ( + # Still allow pass or ... (for empty TypedDict's) and docstrings + if isinstance(stmt, PassStmt) or ( isinstance(stmt, ExpressionStmt) and isinstance(stmt.expr, (EllipsisExpr, StrExpr)) ): + statements.append(stmt) + else: self.fail(TPDICT_CLASS_ERROR, stmt) elif len(stmt.lvalues) > 1 or not isinstance(stmt.lvalues[0], NameExpr): # An assignment, but an invalid one. @@ -281,8 +294,9 @@ def analyze_typeddict_classdef_fields( if name in fields: self.fail(f'Duplicate TypedDict key "{name}"', stmt) continue - # Append name and type in this case... + # Append stmt, name, and type in this case... fields.append(name) + statements.append(stmt) if stmt.type is None: types.append(AnyType(TypeOfAny.unannotated)) else: @@ -293,9 +307,9 @@ def analyze_typeddict_classdef_fields( and not self.api.is_func_scope(), ) if analyzed is None: - return None, [], set() # Need to defer + return None, [], [], set() # Need to defer types.append(analyzed) - # ...despite possible minor failures that allow further analyzis. + # ...despite possible minor failures that allow further analysis. if stmt.type is None or hasattr(stmt, "new_syntax") and not stmt.new_syntax: self.fail(TPDICT_CLASS_ERROR, stmt) elif not isinstance(stmt.rvalue, TempNode): @@ -317,7 +331,7 @@ def analyze_typeddict_classdef_fields( t.item if isinstance(t, RequiredType) else t for t in types ] - return fields, types, required_keys + return fields, types, statements, required_keys def check_typeddict( self, node: Expression, var_name: str | None, is_func_scope: bool diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index 4c68b7b692ff..796f2f547528 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -221,6 +221,19 @@ reveal_type(d) # N: Revealed type is "TypedDict('__main__.D', {'y': builtins.in [builtins fixtures/dict.pyi] [typing fixtures/typing-typeddict.pyi] +[case testCannotCreateTypedDictWithDecoratedFunction] +# flags: --disallow-any-expr +# https://github.com/python/mypy/issues/13066 +from typing import TypedDict +class D(TypedDict): + @classmethod # E: Invalid statement in TypedDict definition; expected "field_name: field_type" + def m(self) -> D: + pass +d = D() +reveal_type(d) # N: Revealed type is "TypedDict('__main__.D', {})" +[builtins fixtures/dict.pyi] +[typing fixtures/typing-typeddict.pyi] + [case testTypedDictWithClassmethodAlternativeConstructorDoesNotCrash] # https://github.com/python/mypy/issues/5653 from typing import TypedDict