-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Support additional args to namedtuple() #5215
Changes from 4 commits
d394c6e
c73583d
ec25ae9
b02b21e
4613aa3
6c9a8b5
e977b71
4c47a0a
138b2fd
ebc66d2
ff63b1b
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 |
---|---|---|
|
@@ -14,7 +14,7 @@ | |
AssignmentStmt, PassStmt, Decorator, FuncBase, ClassDef, Expression, RefExpr, TypeInfo, | ||
NamedTupleExpr, CallExpr, Context, TupleExpr, ListExpr, SymbolTableNode, FuncDef, Block, | ||
TempNode, | ||
ARG_POS, ARG_NAMED_OPT, ARG_OPT, MDEF, GDEF | ||
ARG_POS, ARG_NAMED, ARG_NAMED_OPT, ARG_OPT, MDEF, GDEF | ||
) | ||
from mypy.options import Options | ||
from mypy.exprtotype import expr_to_unanalyzed_type, TypeTranslationError | ||
|
@@ -48,7 +48,7 @@ def analyze_namedtuple_classdef(self, defn: ClassDef) -> Optional[TypeInfo]: | |
node.node = info | ||
defn.info.replaced = info | ||
defn.info = info | ||
defn.analyzed = NamedTupleExpr(info) | ||
defn.analyzed = NamedTupleExpr(info, is_typed=True) | ||
defn.analyzed.line = defn.line | ||
defn.analyzed.column = defn.column | ||
return info | ||
|
@@ -142,35 +142,71 @@ def check_namedtuple(self, | |
if not isinstance(callee, RefExpr): | ||
return None | ||
fullname = callee.fullname | ||
if fullname not in ('collections.namedtuple', 'typing.NamedTuple'): | ||
if fullname == 'collections.namedtuple': | ||
is_typed = False | ||
elif fullname == 'typing.NamedTuple': | ||
is_typed = True | ||
else: | ||
return None | ||
items, types, ok = self.parse_namedtuple_args(call, fullname) | ||
items, types, num_defaults, ok = self.parse_namedtuple_args(call, fullname, is_typed) | ||
if not ok: | ||
# Error. Construct dummy return value. | ||
return self.build_namedtuple_typeinfo('namedtuple', [], [], {}) | ||
name = cast(StrExpr, call.args[0]).value | ||
if name != var_name or is_func_scope: | ||
# Give it a unique name derived from the line number. | ||
name += '@' + str(call.line) | ||
info = self.build_namedtuple_typeinfo(name, items, types, {}) | ||
if num_defaults > 0: | ||
default_items = { | ||
arg_name: EllipsisExpr() | ||
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 a bit hacky so at least deserves a comment why we use If possible, I would just use the actual expressions here. 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 think I can get the actual expressions, but I'm not sure I understand why that would be better. It would make the code more complicated and I don't see much of an advantage. 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. If this is really complex then it maybe doesn't make sense. In general now that we work on mypyc it is simpler if we keep all real info somewhere in the AST. 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. Oh yes, this makes sense in the context of mypyc. It isn't too bad actually, I'll just make the change. |
||
for arg_name in items[-num_defaults:] | ||
} | ||
else: | ||
default_items = {} | ||
info = self.build_namedtuple_typeinfo(name, items, types, default_items) | ||
# Store it as a global just in case it would remain anonymous. | ||
# (Or in the nearest class if there is one.) | ||
stnode = SymbolTableNode(GDEF, info) | ||
self.api.add_symbol_table_node(name, stnode) | ||
call.analyzed = NamedTupleExpr(info) | ||
call.analyzed = NamedTupleExpr(info, is_typed=is_typed) | ||
call.analyzed.set_line(call.line, call.column) | ||
return info | ||
|
||
def parse_namedtuple_args(self, call: CallExpr, | ||
fullname: str) -> Tuple[List[str], List[Type], bool]: | ||
def parse_namedtuple_args(self, call: CallExpr, fullname: str, | ||
is_typed: bool) -> Tuple[List[str], List[Type], int, bool]: | ||
"""Parse a namedtuple() call into data needed to construct a type. | ||
|
||
Returns a 4-tuple: | ||
- List of argument names | ||
- List of argument types | ||
- Number of arguments that have a default value | ||
- Whether the definition typechecked. | ||
|
||
""" | ||
# 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) | ||
num_defaults = 0 | ||
if len(args) > 2: | ||
# FIX incorrect. There are two additional parameters | ||
return self.fail_namedtuple_arg("Too many arguments for namedtuple()", call) | ||
if call.arg_kinds != [ARG_POS, ARG_POS]: | ||
# Typed namedtuple doesn't support additional arguments. | ||
if is_typed: | ||
return self.fail_namedtuple_arg("Too many arguments for namedtuple()", call) | ||
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 would capitalize |
||
for i, arg_name in enumerate(call.arg_names[2:], 2): | ||
if arg_name == 'defaults': | ||
arg = args[i] | ||
# We don't care what the values are, as long as the argument is an iterable | ||
# and we can count how many defaults there are. | ||
if isinstance(arg, (ListExpr, TupleExpr)): | ||
num_defaults = len(arg.items) | ||
else: | ||
self.fail( | ||
"List or tuple literal expected as the defaults argument to " | ||
"namedtuple()", | ||
arg | ||
) | ||
break | ||
if call.arg_kinds[:2] != [ARG_POS, ARG_POS]: | ||
return self.fail_namedtuple_arg("Unexpected arguments to namedtuple()", call) | ||
if not isinstance(args[0], (StrExpr, BytesExpr, UnicodeExpr)): | ||
return self.fail_namedtuple_arg( | ||
|
@@ -196,17 +232,27 @@ def parse_namedtuple_args(self, call: CallExpr, | |
items = [cast(StrExpr, item).value for item in listexpr.items] | ||
else: | ||
# The fields argument contains (name, type) tuples. | ||
items, types, ok = self.parse_namedtuple_fields_with_types(listexpr.items, call) | ||
items, types, _, ok = self.parse_namedtuple_fields_with_types(listexpr.items, call) | ||
if not types: | ||
types = [AnyType(TypeOfAny.unannotated) for _ in items] | ||
underscore = [item for item in items if item.startswith('_')] | ||
if underscore: | ||
self.fail("namedtuple() field names cannot start with an underscore: " | ||
+ ', '.join(underscore), call) | ||
return items, types, ok | ||
if num_defaults > len(items): | ||
self.fail("Too many defaults given in call to namedtuple()", call) | ||
num_defaults = len(items) | ||
return items, types, num_defaults, ok | ||
|
||
def extract_defaults_from_arg(self, call: CallExpr, arg: Expression) -> int: | ||
if not isinstance(arg, (ListExpr, TupleExpr)): | ||
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. Technically the defaults argument can be any iterable, but it doesn't seem particularly useful to support that. |
||
self.fail("List or tuple literal expected as the defaults argument to namedtuple()", | ||
call) | ||
return 0 | ||
|
||
|
||
def parse_namedtuple_fields_with_types(self, nodes: List[Expression], | ||
context: Context) -> Tuple[List[str], List[Type], bool]: | ||
context: Context) -> Tuple[List[str], List[Type], int, bool]: | ||
items = [] # type: List[str] | ||
types = [] # type: List[Type] | ||
for item in nodes: | ||
|
@@ -226,12 +272,12 @@ def parse_namedtuple_fields_with_types(self, nodes: List[Expression], | |
types.append(self.api.anal_type(type)) | ||
else: | ||
return self.fail_namedtuple_arg("Tuple expected as NamedTuple() field", item) | ||
return items, types, True | ||
return items, types, 0, True | ||
|
||
def fail_namedtuple_arg(self, message: str, | ||
context: Context) -> Tuple[List[str], List[Type], bool]: | ||
context: Context) -> Tuple[List[str], List[Type], int, bool]: | ||
self.fail(message, context) | ||
return [], [], False | ||
return [], [], 0, False | ||
|
||
def build_namedtuple_typeinfo(self, name: str, items: List[str], types: List[Type], | ||
default_items: Dict[str, Expression]) -> TypeInfo: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
[case testNamedTupleUsedAsTuple] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
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. My change ends up requiring |
||
x = None # type: X | ||
a, b = x | ||
b = x[0] | ||
|
@@ -28,7 +28,7 @@ X = namedtuple('X', 'x, _y, _z') # E: namedtuple() field names cannot start wit | |
[case testNamedTupleAccessingAttributes] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
x = None # type: X | ||
x.x | ||
x.y | ||
|
@@ -38,7 +38,7 @@ x.z # E: "X" has no attribute "z" | |
[case testNamedTupleAttributesAreReadOnly] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
x = None # type: X | ||
x.x = 5 # E: Property "x" defined in "X" is read-only | ||
x.y = 5 # E: Property "y" defined in "X" is read-only | ||
|
@@ -54,7 +54,7 @@ a.y = 5 # E: Property "y" defined in "X" is read-only | |
[case testNamedTupleCreateWithPositionalArguments] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
x = X(1, 'x') | ||
x.x | ||
x.z # E: "X" has no attribute "z" | ||
|
@@ -64,21 +64,48 @@ x = X(1, 2, 3) # E: Too many arguments for "X" | |
[case testCreateNamedTupleWithKeywordArguments] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
x = X(x=1, y='x') | ||
x = X(1, y='x') | ||
x = X(x=1, z=1) # E: Unexpected keyword argument "z" for "X" | ||
x = X(y=1) # E: Missing positional argument "x" in call to "X" | ||
|
||
|
||
[case testNamedTupleCreateAndUseAsTuple] | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y']) | ||
X = namedtuple('X', 'x y') | ||
x = X(1, 'x') | ||
a, b = x | ||
a, b, c = x # E: Need more than 2 values to unpack (3 expected) | ||
|
||
[case testNamedTupleAdditionalArgs] | ||
from collections import namedtuple | ||
|
||
A = namedtuple('A', 'a b') | ||
B = namedtuple('B', 'a b', rename=1) | ||
C = namedtuple('C', 'a b', rename='not a bool') # E: Argument "rename" to "namedtuple" has incompatible type "str"; expected "int" | ||
# This works correctly but also produces "mypy/test-data/unit/lib-stub/collections.pyi:3: note: "namedtuple" defined here" | ||
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. You can add an |
||
# Don't know how to express that in a test. | ||
# D = namedtuple('D', 'a b', unrecognized_arg=False) E: Unexpected keyword argument "unrecognized_arg" for "namedtuple" | ||
E = namedtuple('E', 'a b', 0) # E: Too many positional arguments for "namedtuple" | ||
|
||
[builtins fixtures/bool.pyi] | ||
|
||
[case testNamedTupleDefaults] | ||
# flags: --python-version 3.7 | ||
from collections import namedtuple | ||
|
||
X = namedtuple('X', ['x', 'y'], defaults=(1,)) | ||
|
||
X() # E: Too few arguments for "X" | ||
X(0) # ok | ||
X(0, 1) # ok | ||
X(0, 1, 2) # E: Too many arguments for "X" | ||
|
||
Y = namedtuple('Y', ['x', 'y'], defaults=(1, 2, 3)) # E: Too many defaults given in call to namedtuple() | ||
Z = namedtuple('Z', ['x', 'y'], defaults='not a tuple') # E: Argument "defaults" to "namedtuple" has incompatible type "str"; expected "Optional[Iterable[Any]]" # E: List or tuple literal expected as the defaults argument to namedtuple() | ||
|
||
[builtins fixtures/list.pyi] | ||
|
||
[case testNamedTupleWithItemTypes] | ||
from typing import NamedTuple | ||
|
@@ -223,6 +250,7 @@ import collections | |
MyNamedTuple = collections.namedtuple('MyNamedTuple', ['spam', 'eggs']) | ||
MyNamedTuple.x # E: "Type[MyNamedTuple]" has no attribute "x" | ||
|
||
[builtins fixtures/list.pyi] | ||
|
||
[case testNamedTupleEmptyItems] | ||
from typing import NamedTuple | ||
|
@@ -263,6 +291,8 @@ x._replace(x=3, y=5) | |
x._replace(z=5) # E: Unexpected keyword argument "z" for "_replace" of "X" | ||
x._replace(5) # E: Too many positional arguments for "_replace" of "X" | ||
|
||
[builtins fixtures/list.pyi] | ||
|
||
[case testNamedTupleReplaceAsClass] | ||
from collections import namedtuple | ||
|
||
|
@@ -271,6 +301,7 @@ x = None # type: X | |
X._replace(x, x=1, y=2) | ||
X._replace(x=1, y=2) # E: Missing positional argument "self" in call to "_replace" of "X" | ||
|
||
[builtins fixtures/list.pyi] | ||
|
||
[case testNamedTupleReplaceTyped] | ||
from typing import NamedTuple | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,14 +13,6 @@ s = b'foo' | |
from typing import TypeVar | ||
T = TypeVar(u'T') | ||
|
||
[case testNamedTupleUnicode] | ||
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 no longer worked because my stub for namedtuple just has str. Apparently we don't support |
||
from typing import NamedTuple | ||
from collections import namedtuple | ||
N = NamedTuple(u'N', [(u'x', int)]) | ||
n = namedtuple(u'n', u'x y') | ||
|
||
[builtins fixtures/dict.pyi] | ||
|
||
[case testPrintStatement] | ||
print ''() # E: "str" not callable | ||
print 1, 1() # E: "int" not callable | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,3 +27,4 @@ class int: | |
class str: pass | ||
class bool: pass | ||
class function: pass | ||
class ellipsis: pass | ||
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 was needed because a few tests that import collections didn't have |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,11 @@ | ||
import typing | ||
from typing import Any, Iterable, Union, Optional | ||
|
||
namedtuple = object() | ||
def namedtuple( | ||
typename: str, | ||
field_names: Union[str, Iterable[str]], | ||
*, | ||
# really int but many tests don't have bool available | ||
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 think this should read "really bool". |
||
rename: int = ..., | ||
module: Optional[str] = ..., | ||
defaults: Optional[Iterable[Any]] = ... | ||
) -> Any: ... |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,10 +138,6 @@ MypyFile:1( | |
from collections import namedtuple | ||
N = namedtuple('N') # E: Too few arguments for namedtuple() | ||
|
||
[case testNamedTupleWithTooManyArguments] | ||
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 test stopped working because this error is now caught during type checking rather than semanal. |
||
from collections import namedtuple | ||
N = namedtuple('N', ['x'], 'y') # E: Too many arguments for namedtuple() | ||
|
||
[case testNamedTupleWithInvalidName] | ||
from collections import namedtuple | ||
N = namedtuple(1, ['x']) # E: namedtuple() expects a string literal as the first argument | ||
|
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.
Could you please add a comment here that generating correct errors in call here depends on the typeshed stub?