-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Implement new NamedTuple syntax #2245
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
Changes from 3 commits
98e5cab
409abb6
aa71fda
a829987
3be0166
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,7 @@ | |
ImportFrom, ImportAll, Block, LDEF, NameExpr, MemberExpr, | ||
IndexExpr, TupleExpr, ListExpr, ExpressionStmt, ReturnStmt, | ||
RaiseStmt, AssertStmt, OperatorAssignmentStmt, WhileStmt, | ||
ForStmt, BreakStmt, ContinueStmt, IfStmt, TryStmt, WithStmt, DelStmt, | ||
ForStmt, BreakStmt, ContinueStmt, IfStmt, TryStmt, WithStmt, DelStmt, PassStmt, | ||
GlobalDecl, SuperExpr, DictExpr, CallExpr, RefExpr, OpExpr, UnaryExpr, | ||
SliceExpr, CastExpr, RevealTypeExpr, TypeApplication, Context, SymbolTable, | ||
SymbolTableNode, BOUND_TVAR, UNBOUND_TVAR, ListComprehension, GeneratorExpr, | ||
|
@@ -63,7 +63,7 @@ | |
YieldFromExpr, NamedTupleExpr, NonlocalDecl, SymbolNode, | ||
SetComprehension, DictionaryComprehension, TYPE_ALIAS, TypeAliasExpr, | ||
YieldExpr, ExecStmt, Argument, BackquoteExpr, ImportBase, AwaitExpr, | ||
IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, | ||
IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, TempNode, | ||
COVARIANT, CONTRAVARIANT, INVARIANT, UNBOUND_IMPORTED, LITERAL_YES, | ||
) | ||
from mypy.visitor import NodeVisitor | ||
|
@@ -545,7 +545,57 @@ def check_function_signature(self, fdef: FuncItem) -> None: | |
elif len(sig.arg_types) > len(fdef.arguments): | ||
self.fail('Type signature has too many arguments', fdef, blocker=True) | ||
|
||
def check_namedtuple_classdef(self, defn: ClassDef) -> Tuple[List[str], List[Type]]: | ||
NAMEDTUP_CLASS_ERROR = 'Invalid statement in NamedTuple definition; ' \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use () for the line continuation instead of |
||
'expected "field_name: field_type"' | ||
if self.options.python_version < (3, 6): | ||
self.fail('NamedTuple class syntax is only supported in Python 3.6', defn) | ||
return [], [] | ||
if len(defn.base_type_exprs) > 1: | ||
self.fail('NamedTuple should be a single base', defn) | ||
items = [] # type: List[str] | ||
types = [] # type: List[Type] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks hard to prove (and preserve in future edits) that these have the same length. Maybe make a list of tuples and separate them at the end using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, and this should be uniform in other places (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
for stmt in defn.defs.body: | ||
if isinstance(stmt, AssignmentStmt): | ||
if len(stmt.lvalues) > 1 or not isinstance(stmt.lvalues[0], NameExpr): | ||
self.fail(NAMEDTUP_CLASS_ERROR, stmt) | ||
continue | ||
name = stmt.lvalues[0].name | ||
if not isinstance(stmt.rvalue, TempNode): | ||
# x: int assigns rvalue to TempNode(AnyType()) | ||
self.fail('NamedTuple does not support initial values', stmt) | ||
if stmt.type is None: | ||
self.fail('NamedTuple field type must be specified', stmt) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you get this the previous fail() also triggers, right? I think that if someone writes (Also, what happens when they write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I changed this so that a single "generic" |
||
types.append(AnyType()) | ||
else: | ||
types.append(self.anal_type(stmt.type)) | ||
if name.startswith('_'): | ||
self.fail('NamedTuple field name cannot start with an underscore: {}' | ||
.format(name), stmt) | ||
items.append(name) | ||
else: | ||
if (not isinstance(stmt, PassStmt) and | ||
not (isinstance(stmt, ExpressionStmt) and | ||
isinstance(stmt.expr, EllipsisExpr))): | ||
self.fail(NAMEDTUP_CLASS_ERROR, stmt) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't namedtuples have methods? Subclasses can. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is because |
||
return items, types | ||
|
||
def analyze_namedtuple_classdef(self, defn: ClassDef) -> None: | ||
node = self.lookup(defn.name, defn) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Careful, this may return None. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
node.kind = GDEF # TODO in process_namedtuple_definition also applies here | ||
items, types = self.check_namedtuple_classdef(defn) | ||
node.node = self.build_namedtuple_typeinfo(defn.name, items, types) | ||
|
||
def visit_class_def(self, defn: ClassDef) -> None: | ||
# special case for NamedTuple | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd refactor this into a helper function. Also it probably should be called after clean_up_bases_and_infer_type_variables(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
for base_expr in defn.base_type_exprs: | ||
if isinstance(base_expr, NameExpr): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this should probably also check for MemberExpr (similar to what's happening in check_namedtuple()). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
sym = self.lookup_qualified(base_expr.name, base_expr) | ||
if sym is None or sym.node is None: | ||
continue | ||
if sym.node.fullname() == 'typing.NamedTuple': | ||
self.analyze_namedtuple_classdef(defn) | ||
return | ||
self.clean_up_bases_and_infer_type_variables(defn) | ||
self.setup_class_def_analysis(defn) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Believe it or not, mypy's (loose) convention for helper functions is that they follow the main (or only) call site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talking about placement, it would be nice to have it closer to other namedtuple-handling functions (~ 1600)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elazarg 1600 is probably too far. I put it in the same place where other
ClassDef
helpers are (~ 750).