From fce50fc91e02393ed6495e7c275b39ae4313dc76 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 14 May 2024 21:18:04 -0400 Subject: [PATCH 1/5] Support rename=True in collections.namedtuple A pretty marginal feature but it's now tested in the typing conformance suite, and it was easy to add support. --- mypy/semanal_namedtuple.py | 54 +++++++++++++++++++++----- test-data/unit/check-namedtuple.test | 26 ++++++++++++- test-data/unit/semanal-namedtuple.test | 4 +- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index 9a0be9d9c14c..f3479a854a5a 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -5,8 +5,9 @@ from __future__ import annotations +import keyword from contextlib import contextmanager -from typing import Final, Iterator, List, Mapping, cast +from typing import Container, Final, Iterator, List, Mapping, Optional, cast from mypy.exprtotype import TypeTranslationError, expr_to_unanalyzed_type from mypy.messages import MessageBuilder @@ -352,6 +353,7 @@ def parse_namedtuple_args( self.fail(f'Too few arguments for "{type_name}()"', call) return None defaults: list[Expression] = [] + rename = False if len(args) > 2: # Typed namedtuple doesn't support additional arguments. if fullname in TYPED_NAMEDTUPLE_NAMES: @@ -370,7 +372,16 @@ def parse_namedtuple_args( "{}()".format(type_name), arg, ) - break + elif arg_name == "rename": + arg = args[i] + if isinstance(arg, NameExpr) and arg.name in ("True", "False"): + rename = arg.name == "True" + else: + self.fail( + "Boolean literal expected as the \"rename\" argument to " + f"{type_name}()", + arg, + ) if call.arg_kinds[:2] != [ARG_POS, ARG_POS]: self.fail(f'Unexpected arguments to "{type_name}()"', call) return None @@ -417,17 +428,28 @@ def parse_namedtuple_args( return [], [], [], typename, [], False if not types: types = [AnyType(TypeOfAny.unannotated) for _ in items] - underscore = [item for item in items if item.startswith("_")] - if underscore: - self.fail( - f'"{type_name}()" field names cannot start with an underscore: ' - + ", ".join(underscore), - call, - ) + processed_items = [] + seen_names = set() + for i, item in enumerate(items): + problem = self.check_namedtuple_field_name(item, seen_names) + if problem is None: + processed_items.append(item) + seen_names.add(item) + else: + if not rename: + self.fail(f'"{type_name}()" {problem}', call) + # Even if rename=False, we pretend that it is True. + # At runtime namedtuple creation would throw an error; + # applying the rename logic means we create a more sensible + # namedtuple. + new_name = f"_{i}" + processed_items.append(new_name) + seen_names.add(new_name) + if len(defaults) > len(items): self.fail(f'Too many defaults given in call to "{type_name}()"', call) defaults = defaults[: len(items)] - return items, types, defaults, typename, tvar_defs, True + return processed_items, types, defaults, typename, tvar_defs, True def parse_namedtuple_fields_with_types( self, nodes: list[Expression], context: Context @@ -666,5 +688,17 @@ def save_namedtuple_body(self, named_tuple_info: TypeInfo) -> Iterator[None]: # Helpers + def check_namedtuple_field_name(self, field: str, seen_names: Container[str]) -> Optional[str]: + """Return None for valid fields, a string description for invalid ones.""" + if field in seen_names: + return f"has duplicate field name \"{field}\"" + elif not field.isidentifier(): + return f'field name "{field}" is not a valid identifier' + elif field.startswith("_"): + return f'field name "{field}" starts with an underscore' + elif keyword.iskeyword(field): + return f'field name "{field}" is a keyword' + return None + def fail(self, msg: str, ctx: Context) -> None: self.api.fail(msg, ctx) diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index 23e109e1af78..5e7c730162d8 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -22,10 +22,13 @@ a, b, c = x # E: Need more than 2 values to unpack (3 expected) x[2] # E: Tuple index out of range [builtins fixtures/tuple.pyi] -[case testNamedTupleNoUnderscoreFields] +[case testNamedTupleInvalidFields] from collections import namedtuple -X = namedtuple('X', 'x, _y, _z') # E: "namedtuple()" field names cannot start with an underscore: _y, _z +X = namedtuple('X', 'x, _y') # E: "namedtuple()" field name "_y" starts with an underscore +Y = namedtuple('Y', ['x', '1']) # E: "namedtuple()" field name "1" is not a valid identifier +Z = namedtuple('Z', ['x', 'def']) # E: "namedtuple()" field name "def" is a keyword +A = namedtuple('A', ['x', 'x']) # E: "namedtuple()" has duplicate field name "x" [builtins fixtures/tuple.pyi] [case testNamedTupleAccessingAttributes] @@ -125,6 +128,8 @@ E = namedtuple('E', 'a b', 0) [builtins fixtures/bool.pyi] [out] +main:4: error: Boolean literal expected as the "rename" argument to namedtuple() +main:5: error: Boolean literal expected as the "rename" argument to namedtuple() main:5: error: Argument "rename" to "namedtuple" has incompatible type "str"; expected "int" main:6: error: Unexpected keyword argument "unrecognized_arg" for "namedtuple" /test-data/unit/lib-stub/collections.pyi:3: note: "namedtuple" defined here @@ -145,6 +150,23 @@ Z = namedtuple('Z', ['x', 'y'], defaults='not a tuple') # E: List or tuple lite [builtins fixtures/list.pyi] +[case testNamedTupleRename] +from collections import namedtuple + +X = namedtuple('X', ['abc', 'def'], rename=False) # E: "namedtuple()" field name "def" is a keyword +Y = namedtuple('Y', ['x', 'x', 'def', '42', '_x'], rename=True) +y = Y(x=0, _1=1, _2=2, _3=3, _4=4) +reveal_type(y.x) # N: Revealed type is "Any" +reveal_type(y._1) # N: Revealed type is "Any" +reveal_type(y._2) # N: Revealed type is "Any" +reveal_type(y._3) # N: Revealed type is "Any" +reveal_type(y._4) # N: Revealed type is "Any" +y._0 # E: "Y" has no attribute "_0" +y._5 # E: "Y" has no attribute "_5" +y._x # E: "Y" has no attribute "_x" + +[builtins fixtures/list.pyi] + [case testNamedTupleWithItemTypes] from typing import NamedTuple N = NamedTuple('N', [('a', int), diff --git a/test-data/unit/semanal-namedtuple.test b/test-data/unit/semanal-namedtuple.test index f396f799028f..16944391da86 100644 --- a/test-data/unit/semanal-namedtuple.test +++ b/test-data/unit/semanal-namedtuple.test @@ -165,7 +165,7 @@ N = namedtuple('N', ['x', 1]) # E: String literal expected as "namedtuple()" ite [case testNamedTupleWithUnderscoreItemName] from collections import namedtuple -N = namedtuple('N', ['_fallback']) # E: "namedtuple()" field names cannot start with an underscore: _fallback +N = namedtuple('N', ['_fallback']) # E: "namedtuple()" field name "_fallback" starts with an underscore [builtins fixtures/tuple.pyi] -- NOTE: The following code works at runtime but is not yet supported by mypy. @@ -197,7 +197,7 @@ N = NamedTuple('N', 1) # E: List or tuple literal expected as the second argumen [case testTypingNamedTupleWithUnderscoreItemName] from typing import NamedTuple -N = NamedTuple('N', [('_fallback', int)]) # E: "NamedTuple()" field names cannot start with an underscore: _fallback +N = NamedTuple('N', [('_fallback', int)]) # E: "NamedTuple()" field name "_fallback" starts with an underscore [builtins fixtures/tuple.pyi] [case testTypingNamedTupleWithUnexpectedNames] From 0873df68d733673d8c4a811e49ad232cc36bacdf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 01:21:12 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- mypy/semanal_namedtuple.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index f3479a854a5a..54a37e781983 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -378,7 +378,7 @@ def parse_namedtuple_args( rename = arg.name == "True" else: self.fail( - "Boolean literal expected as the \"rename\" argument to " + 'Boolean literal expected as the "rename" argument to ' f"{type_name}()", arg, ) @@ -691,7 +691,7 @@ def save_namedtuple_body(self, named_tuple_info: TypeInfo) -> Iterator[None]: def check_namedtuple_field_name(self, field: str, seen_names: Container[str]) -> Optional[str]: """Return None for valid fields, a string description for invalid ones.""" if field in seen_names: - return f"has duplicate field name \"{field}\"" + return f'has duplicate field name "{field}"' elif not field.isidentifier(): return f'field name "{field}" is not a valid identifier' elif field.startswith("_"): From 577d276f447ccff1e8d0c570c88b11d7ff9696c1 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 14 May 2024 21:26:35 -0400 Subject: [PATCH 3/5] add annotation --- mypy/semanal_namedtuple.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index 54a37e781983..56104c1ffc46 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -429,7 +429,7 @@ def parse_namedtuple_args( if not types: types = [AnyType(TypeOfAny.unannotated) for _ in items] processed_items = [] - seen_names = set() + seen_names: set[str] = set() for i, item in enumerate(items): problem = self.check_namedtuple_field_name(item, seen_names) if problem is None: From 180f4193dcf3b28c3f67c969f74a99a7bf1c35fe Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 14 May 2024 21:30:02 -0400 Subject: [PATCH 4/5] | None --- mypy/semanal_namedtuple.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index 56104c1ffc46..4c4caa80353a 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -7,7 +7,7 @@ import keyword from contextlib import contextmanager -from typing import Container, Final, Iterator, List, Mapping, Optional, cast +from typing import Container, Final, Iterator, List, Mapping, cast from mypy.exprtotype import TypeTranslationError, expr_to_unanalyzed_type from mypy.messages import MessageBuilder @@ -688,7 +688,7 @@ def save_namedtuple_body(self, named_tuple_info: TypeInfo) -> Iterator[None]: # Helpers - def check_namedtuple_field_name(self, field: str, seen_names: Container[str]) -> Optional[str]: + def check_namedtuple_field_name(self, field: str, seen_names: Container[str]) -> str | None: """Return None for valid fields, a string description for invalid ones.""" if field in seen_names: return f'has duplicate field name "{field}"' From 365433c6e7b78bdb78f6c02baa4d8b3bac570fc7 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 18 May 2024 16:17:51 -0400 Subject: [PATCH 5/5] Use ARG_TYPE error code --- mypy/semanal_namedtuple.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py index 4c4caa80353a..f051c4ee36e9 100644 --- a/mypy/semanal_namedtuple.py +++ b/mypy/semanal_namedtuple.py @@ -9,6 +9,7 @@ from contextlib import contextmanager from typing import Container, Final, Iterator, List, Mapping, cast +from mypy.errorcodes import ARG_TYPE, ErrorCode from mypy.exprtotype import TypeTranslationError, expr_to_unanalyzed_type from mypy.messages import MessageBuilder from mypy.nodes import ( @@ -381,6 +382,7 @@ def parse_namedtuple_args( 'Boolean literal expected as the "rename" argument to ' f"{type_name}()", arg, + code=ARG_TYPE, ) if call.arg_kinds[:2] != [ARG_POS, ARG_POS]: self.fail(f'Unexpected arguments to "{type_name}()"', call) @@ -700,5 +702,5 @@ def check_namedtuple_field_name(self, field: str, seen_names: Container[str]) -> return f'field name "{field}" is a keyword' return None - def fail(self, msg: str, ctx: Context) -> None: - self.api.fail(msg, ctx) + def fail(self, msg: str, ctx: Context, code: ErrorCode | None = None) -> None: + self.api.fail(msg, ctx, code=code)