Skip to content

[needs tests] TypedDict: Recognize declaration of TypedDict('Point', {'x': int, 'y': int}). #2206

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 8 commits into from
Oct 19, 2016
14 changes: 13 additions & 1 deletion extensions/mypy_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,16 @@

# NOTE: This module must support Python 2.7 in addition to Python 3.x

# (TODO: Declare TypedDict and other extensions here)

def TypedDict(typename, fields):
"""TypedDict creates a dictionary type that expects all of its
instances to have a certain set of keys, with each key
associated with a value of a consistent type. This expectation
is not checked at runtime but is only enforced by typecheckers.
"""
def new_dict(*args, **kwargs):
return dict(*args, **kwargs)

new_dict.__name__ = typename
new_dict.__supertype__ = dict
return new_dict
6 changes: 5 additions & 1 deletion mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
Context, ListComprehension, ConditionalExpr, GeneratorExpr,
Decorator, SetExpr, TypeVarExpr, NewTypeExpr, PrintStmt,
LITERAL_TYPE, BreakStmt, PassStmt, ContinueStmt, ComparisonExpr, StarExpr,
YieldFromExpr, NamedTupleExpr, SetComprehension,
YieldFromExpr, NamedTupleExpr, TypedDictExpr, SetComprehension,
DictionaryComprehension, ComplexExpr, EllipsisExpr, TypeAliasExpr,
RefExpr, YieldExpr, BackquoteExpr, ImportFrom, ImportAll, ImportBase,
AwaitExpr,
Expand Down Expand Up @@ -2082,6 +2082,10 @@ def visit_namedtuple_expr(self, e: NamedTupleExpr) -> Type:
# TODO: Perhaps return a type object type?
return AnyType()

def visit_typeddict_expr(self, e: TypedDictExpr) -> Type:
# TODO: Perhaps return a type object type?
return AnyType()

def visit_list_expr(self, e: ListExpr) -> Type:
return self.expr_checker.visit_list_expr(e)

Expand Down
20 changes: 18 additions & 2 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,7 @@ def accept(self, visitor: NodeVisitor[T]) -> T:


class NamedTupleExpr(Expression):
"""Named tuple expression namedtuple(...)."""
"""Named tuple expression namedtuple(...) or NamedTuple(...)."""

# The class representation of this named tuple (its tuple_type attribute contains
# the tuple item types)
Expand All @@ -1760,6 +1760,19 @@ def accept(self, visitor: NodeVisitor[T]) -> T:
return visitor.visit_namedtuple_expr(self)


class TypedDictExpr(Expression):
"""Typed dict expression TypedDict(...)."""

# The class representation of this typed dict
info = None # type: TypeInfo

def __init__(self, info: 'TypeInfo') -> None:
self.info = info

def accept(self, visitor: NodeVisitor[T]) -> T:
return visitor.visit_typeddict_expr(self)


class PromoteExpr(Expression):
"""Ducktype class decorator expression _promote(...)."""

Expand Down Expand Up @@ -1882,6 +1895,9 @@ class is generic then it will be a type constructor of higher kind.
# Is this a named tuple type?
is_named_tuple = False

# Is this a typed dict type?
is_typed_dict = False

# Is this a newtype type?
is_newtype = False

Expand All @@ -1893,7 +1909,7 @@ class is generic then it will be a type constructor of higher kind.

FLAGS = [
'is_abstract', 'is_enum', 'fallback_to_any', 'is_named_tuple',
'is_newtype', 'is_dummy'
'is_typed_dict', 'is_newtype', 'is_dummy'
]

def __init__(self, names: 'SymbolTable', defn: ClassDef, module_name: str) -> None:
Expand Down
126 changes: 117 additions & 9 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
FuncExpr, MDEF, FuncBase, Decorator, SetExpr, TypeVarExpr, NewTypeExpr,
StrExpr, BytesExpr, PrintStmt, ConditionalExpr, PromoteExpr,
ComparisonExpr, StarExpr, ARG_POS, ARG_NAMED, MroError, type_aliases,
YieldFromExpr, NamedTupleExpr, NonlocalDecl, SymbolNode,
YieldFromExpr, NamedTupleExpr, TypedDictExpr, NonlocalDecl, SymbolNode,
SetComprehension, DictionaryComprehension, TYPE_ALIAS, TypeAliasExpr,
YieldExpr, ExecStmt, Argument, BackquoteExpr, ImportBase, AwaitExpr,
IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr,
Expand Down Expand Up @@ -1127,6 +1127,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
self.process_newtype_declaration(s)
self.process_typevar_declaration(s)
self.process_namedtuple_definition(s)
self.process_typeddict_definition(s)

if (len(s.lvalues) == 1 and isinstance(s.lvalues[0], NameExpr) and
s.lvalues[0].name == '__all__' and s.lvalues[0].kind == GDEF and
Expand Down Expand Up @@ -1498,9 +1499,9 @@ def get_typevar_declaration(self, s: AssignmentStmt) -> Optional[CallExpr]:
if not isinstance(s.rvalue, CallExpr):
return None
call = s.rvalue
if not isinstance(call.callee, RefExpr):
return None
callee = call.callee
if not isinstance(callee, RefExpr):
return None
if callee.fullname != 'typing.TypeVar':
return None
return call
Expand Down Expand Up @@ -1579,10 +1580,9 @@ def process_namedtuple_definition(self, s: AssignmentStmt) -> None:
# Yes, it's a valid namedtuple definition. Add it to the symbol table.
node = self.lookup(name, s)
node.kind = GDEF # TODO locally defined namedtuple
# TODO call.analyzed
node.node = named_tuple

def check_namedtuple(self, node: Expression, var_name: str = None) -> TypeInfo:
def check_namedtuple(self, node: Expression, var_name: str = None) -> Optional[TypeInfo]:
"""Check if a call defines a namedtuple.

The optional var_name argument is the name of the variable to
Expand All @@ -1596,9 +1596,9 @@ def check_namedtuple(self, node: Expression, var_name: str = None) -> TypeInfo:
if not isinstance(node, CallExpr):
return None
call = node
if not isinstance(call.callee, RefExpr):
return None
callee = call.callee
if not isinstance(callee, RefExpr):
return None
fullname = callee.fullname
if fullname not in ('collections.namedtuple', 'typing.NamedTuple'):
return None
Expand All @@ -1607,9 +1607,9 @@ def check_namedtuple(self, node: Expression, var_name: str = None) -> TypeInfo:
# Error. Construct dummy return value.
return self.build_namedtuple_typeinfo('namedtuple', [], [])
else:
# Give it a unique name derived from the line number.
name = cast(StrExpr, call.args[0]).value
if name != var_name:
# Give it a unique name derived from the line number.
name += '@' + str(call.line)
info = self.build_namedtuple_typeinfo(name, items, types)
# Store it as a global just in case it would remain anonymous.
Expand All @@ -1620,7 +1620,7 @@ def check_namedtuple(self, node: Expression, var_name: str = None) -> TypeInfo:

def parse_namedtuple_args(self, call: CallExpr,
fullname: str) -> Tuple[List[str], List[Type], bool]:
# TODO Share code with check_argument_count in checkexpr.py?
# TODO: Share code with check_argument_count in checkexpr.py?
args = call.args
if len(args) < 2:
return self.fail_namedtuple_arg("Too few arguments for namedtuple()", call)
Expand Down Expand Up @@ -1777,6 +1777,114 @@ def analyze_types(self, items: List[Expression]) -> List[Type]:
result.append(AnyType())
return result

def process_typeddict_definition(self, s: AssignmentStmt) -> None:
"""Check if s defines a TypedDict; if yes, store the definition in symbol table."""
if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], NameExpr):
return
lvalue = s.lvalues[0]
name = lvalue.name
typed_dict = self.check_typeddict(s.rvalue, name)
if typed_dict is None:
return
# Yes, it's a valid TypedDict definition. Add it to the symbol table.
node = self.lookup(name, s)
node.kind = GDEF # TODO locally defined TypedDict
node.node = typed_dict

def check_typeddict(self, node: Expression, var_name: str = None) -> Optional[TypeInfo]:
"""Check if a call defines a TypedDict.

The optional var_name argument is the name of the variable to
which this is assigned, if any.

If it does, return the corresponding TypeInfo. Return None otherwise.

If the definition is invalid but looks like a TypedDict,
report errors but return (some) TypeInfo.
"""
if not isinstance(node, CallExpr):
return None
call = node
callee = call.callee
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Micro-optimization: you can move this before the isinstance() check -- it's still the callee even if we don't like it. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied globally.

if not isinstance(callee, RefExpr):
return None
fullname = callee.fullname
if fullname != 'mypy_extensions.TypedDict':
return None
items, types, ok = self.parse_typeddict_args(call, fullname)
if not ok:
# Error. Construct dummy return value.
return self.build_typeddict_typeinfo('TypedDict', [], [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this isn't going to cause a cascade of spurious errors. Might be better to just return AnyType().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I alter this line's twin in check_namedtuple() to return None, which means that something other than a NamedTuple was found (as opposed to an invalid NamedTuple), the error detected in the test suite (semanal-namedtuple.test:136) seems less helpful.

Old error: Tuple expected as NamedTuple() field
New error: Invalid base class

Honestly I don't think either error message is particularly good but at least the first one mentions that is has something to do with a NamedTuple.

Based on this regression in error message quality I've left this line as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be possible to return an AnyType with a more-invasive change, as I'd need to alter the signature of check_namedtuple and its callers. I'd prefer to tread lightly.

else:
name = cast(StrExpr, call.args[0]).value
if name != var_name:
# Give it a unique name derived from the line number.
name += '@' + str(call.line)
info = self.build_typeddict_typeinfo(name, items, types)
# Store it as a global just in case it would remain anonymous.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this sounds risky. Consider:

foo = 12
class C:
    foo = TypedDict('foo', {...})

Now it looks like the global foo will be overwritten for no reason. Maybe only do that if it's anonymous?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand the implications of changing this line. I will however note that if I remove the line entirely from check_namedtuple() and check_typeddict() then the test suite continues to pass.

Agreed that it seems weird that a type declaration performed anywhere would want to be injected into the global scope. I can remove this line if someone else can confirm that I'm not going to break some subtle preexisting behavior.

self.globals[name] = SymbolTableNode(GDEF, info, self.cur_mod_id)
call.analyzed = TypedDictExpr(info)
call.analyzed.set_line(call.line, call.column)
return info

def parse_typeddict_args(self, call: CallExpr,
fullname: str) -> Tuple[List[str], List[Type], bool]:
# TODO: Share code with check_argument_count in checkexpr.py?
args = call.args
if len(args) < 2:
return self.fail_typeddict_arg("Too few arguments for TypedDict()", call)
if len(args) > 2:
return self.fail_typeddict_arg("Too many arguments for TypedDict()", call)
# TODO: Support keyword arguments
if call.arg_kinds != [ARG_POS, ARG_POS]:
return self.fail_typeddict_arg("Unexpected arguments to TypedDict()", call)
if not isinstance(args[0], (StrExpr, BytesExpr, UnicodeExpr)):
return self.fail_typeddict_arg(
"TypedDict() expects a string literal as the first argument", call)
if not isinstance(args[1], DictExpr):
return self.fail_typeddict_arg(
"TypedDict() expects a dictionary literal as the second argument", call)
dictexpr = args[1]
items, types, ok = self.parse_typeddict_fields_with_types(dictexpr.items, call)
return items, types, ok

def parse_typeddict_fields_with_types(self, dict_items: List[Tuple[Expression, Expression]],
context: Context) -> Tuple[List[str], List[Type], bool]:
items = [] # type: List[str]
types = [] # type: List[Type]
for (field_name_expr, field_type_expr) in dict_items:
if isinstance(field_name_expr, (StrExpr, BytesExpr, UnicodeExpr)):
items.append(field_name_expr.value)
else:
return self.fail_typeddict_arg("Invalid TypedDict() field name", field_name_expr)
try:
type = expr_to_unanalyzed_type(field_type_expr)
except TypeTranslationError:
return self.fail_typeddict_arg('Invalid field type', field_type_expr)
types.append(self.anal_type(type))
return items, types, True

def fail_typeddict_arg(self, message: str,
context: Context) -> Tuple[List[str], List[Type], bool]:
self.fail(message, context)
return [], [], False

def build_typeddict_typeinfo(self, name: str, items: List[str],
types: List[Type]) -> TypeInfo:
strtype = self.named_type('__builtins__.str') # type: Type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Not your fault, but don't you find the difference between named_type() and named_type_or_none() jarring? I have to carefully read the implementation every time I see this... :-( )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know what the difference is. Presumably named_type_or_none() can fail but there's no documentation explaining what None means in that context.

The underlying lookup_fully_qualified() and lookup_fully_qualified_or_none() have identical documentation even though the latter can presumably fail in some way.

Also named_type_or_none() and lookup_fully_qualified_or_none() can return Optional[...] but aren't marked as such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're missing many Optional annotations, so not to worry about those. The difference that was jarring to me is that one starts the lookup in the local namespace (hence __builtins__, which is a secret dict with all the builtins) while the other ("fully qualified") always starts in the module table (hence "builtins"). This is the difference between lookup_qualified() and lookup_fully_qualified_or_none() -- note, "fully".

dictype = (self.named_type_or_none('builtins.dict', [strtype, AnyType()])
or self.object_type())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this 'or' branch is when the test fixtures don't define dict? It's a fine hack but might deserve a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this 'or' branch is when the test fixtures don't define dict?

Perhaps. It's the same pattern that build_namedtuple_typeinfo() uses.

I'm not qualified to leave a comment explaining the pattern.

fallback = dictype

info = self.basic_new_typeinfo(name, fallback)
info.is_typed_dict = True

# (TODO: Store {items, types} inside "info" somewhere for use later.
# Probably inside a new "info.keys" field which
# would be analogous to "info.names".)

return info

def visit_decorator(self, dec: Decorator) -> None:
for d in dec.decorators:
d.accept(self)
Expand Down
4 changes: 4 additions & 0 deletions mypy/strconv.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,10 @@ def visit_namedtuple_expr(self, o: 'mypy.nodes.NamedTupleExpr') -> str:
o.info.name(),
o.info.tuple_type)

def visit_typeddict_expr(self, o: 'mypy.nodes.TypedDictExpr') -> str:
return 'TypedDictExpr:{}({})'.format(o.line,
o.info.name())

def visit__promote_expr(self, o: 'mypy.nodes.PromoteExpr') -> str:
return 'PromoteExpr:{}({})'.format(o.line, o.type)

Expand Down
2 changes: 2 additions & 0 deletions mypy/test/testsemanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
'semanal-statements.test',
'semanal-abstractclasses.test',
'semanal-namedtuple.test',
'semanal-typeddict.test',
'semanal-python2.test']


Expand Down Expand Up @@ -78,6 +79,7 @@ def test_semanal(testcase):
# TODO the test is not reliable
if (not f.path.endswith((os.sep + 'builtins.pyi',
'typing.pyi',
'mypy_extensions.pyi',
'abc.pyi',
'collections.pyi'))
and not os.path.basename(f.path).startswith('_')
Expand Down
5 changes: 4 additions & 1 deletion mypy/treetransform.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
SliceExpr, OpExpr, UnaryExpr, FuncExpr, TypeApplication, PrintStmt,
SymbolTable, RefExpr, TypeVarExpr, NewTypeExpr, PromoteExpr,
ComparisonExpr, TempNode, StarExpr, Statement, Expression,
YieldFromExpr, NamedTupleExpr, NonlocalDecl, SetComprehension,
YieldFromExpr, NamedTupleExpr, TypedDictExpr, NonlocalDecl, SetComprehension,
DictionaryComprehension, ComplexExpr, TypeAliasExpr, EllipsisExpr,
YieldExpr, ExecStmt, Argument, BackquoteExpr, AwaitExpr,
)
Expand Down Expand Up @@ -492,6 +492,9 @@ def visit_newtype_expr(self, node: NewTypeExpr) -> NewTypeExpr:
def visit_namedtuple_expr(self, node: NamedTupleExpr) -> NamedTupleExpr:
return NamedTupleExpr(node.info)

def visit_typeddict_expr(self, node: TypedDictExpr) -> Node:
return TypedDictExpr(node.info)

def visit__promote_expr(self, node: PromoteExpr) -> PromoteExpr:
return PromoteExpr(node.type)

Expand Down
3 changes: 3 additions & 0 deletions mypy/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ def visit_type_alias_expr(self, o: 'mypy.nodes.TypeAliasExpr') -> T:
def visit_namedtuple_expr(self, o: 'mypy.nodes.NamedTupleExpr') -> T:
pass

def visit_typeddict_expr(self, o: 'mypy.nodes.TypedDictExpr') -> T:
pass

def visit_newtype_expr(self, o: 'mypy.nodes.NewTypeExpr') -> T:
pass

Expand Down
6 changes: 6 additions & 0 deletions test-data/unit/lib-stub/mypy_extensions.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from typing import Dict, Type, TypeVar

T = TypeVar('T')


def TypedDict(typename: str, fields: Dict[str, Type[T]]) -> Type[dict]: ...
6 changes: 4 additions & 2 deletions test-data/unit/semanal-namedtuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,11 @@ N = namedtuple('N', 1) # E: List or tuple literal expected as the second argumen
from collections import namedtuple
N = namedtuple('N', ['x', 1]) # E: String literal expected as namedtuple() item

[case testNamedTupleWithInvalidArgs]
-- NOTE: The following code works at runtime but is not yet supported by mypy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, in Python 3.6 I'd love NamedTuple to take keyword args so you can say

Point = typing.NamedTuple('Point', x=int, y=int)

It requires Python 3.6+ because that promises to preserve keyword arg order.

But this would require support in typing too. (@ilevkivskyi?) Not for this PR obviously...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be easy to implement in typing.py
I will open an issue there now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that keyword args would be a nice syntax to use for NamedTuple.

-- Keyword arguments may potentially be supported in the future.
[case testNamedTupleWithNonpositionalArgs]
from collections import namedtuple
N = namedtuple('N', x=['x']) # E: Unexpected arguments to namedtuple()
N = namedtuple(typename='N', field_names=['x']) # E: Unexpected arguments to namedtuple()

[case testInvalidNamedTupleBaseClass]
from typing import NamedTuple
Expand Down
Loading