Skip to content

Commit

Permalink
Fix crash with malformed TypedDicts and disllow-any-expr
Browse files Browse the repository at this point in the history
Fixes python#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 python#10007
and supersede my other diff python#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.
  • Loading branch information
Michael0x2a committed Oct 29, 2022
1 parent 2d70ac0 commit 67ba6f2
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 9 deletions.
32 changes: 23 additions & 9 deletions mypy/semanal_typeddict.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
NameExpr,
PassStmt,
RefExpr,
Statement,
StrExpr,
TempNode,
TupleExpr,
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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:
Expand All @@ -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):
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions test-data/unit/check-typeddict.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 67ba6f2

Please sign in to comment.