From d2f506ae07e0bc097039634a28cf85b5d804ef72 Mon Sep 17 00:00:00 2001 From: Brian Schubert Date: Mon, 27 Apr 2026 22:51:06 -0400 Subject: [PATCH] gh-137600: Promote `ast` node constructor deprecation warnings to errors (#137601) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Jelle Zijlstra --- Doc/library/ast.rst | 5 +- Doc/whatsnew/3.15.rst | 10 + Lib/test/test_ast/test_ast.py | 98 +++--- ...-08-09-19-00-36.gh-issue-137600.p_p6OU.rst | 4 + Parser/asdl_c.py | 302 ++++++------------ Python/Python-ast.c | 302 ++++++------------ 6 files changed, 255 insertions(+), 466 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-08-09-19-00-36.gh-issue-137600.p_p6OU.rst diff --git a/Doc/library/ast.rst b/Doc/library/ast.rst index e23506768a77..3c6e87454743 100644 --- a/Doc/library/ast.rst +++ b/Doc/library/ast.rst @@ -35,6 +35,8 @@ The abstract grammar is currently defined as follows: :language: asdl +.. _ast_nodes: + Node classes ------------ @@ -164,8 +166,7 @@ Node classes Previous versions of Python allowed the creation of AST nodes that were missing required fields. Similarly, AST node constructors allowed arbitrary keyword arguments that were set as attributes of the AST node, even if they did not - match any of the fields of the AST node. This behavior is deprecated and will - be removed in Python 3.15. + match any of the fields of the AST node. These cases now raise a :exc:`TypeError`. .. note:: The descriptions of the specific node classes displayed here diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index 0a96a970ba23..65965d504c09 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -1620,6 +1620,16 @@ This was made possible by a refactoring of JIT data structures. Removed ======== +ast +--- + +* The constructors of :ref:`AST nodes ` now raise a :exc:`TypeError` + when a required argument is omitted or when a keyword argument that does not + map to a field on the AST node is passed. These cases had previously raised a + :exc:`DeprecationWarning` since Python 3.13. + (Contributed by Brian Schubert and Jelle Zijlstra in :gh:`137600` and :gh:`105858`.) + + collections.abc --------------- diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 75d553e6f777..a7d5a51a2aa4 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -418,6 +418,17 @@ class AST_Tests(unittest.TestCase): if isinstance(x, ast.AST): self.assertIs(type(x._fields), tuple) + def test_dynamic_attr(self): + for name, item in ast.__dict__.items(): + # constructor has a different signature + if name == 'Index': + continue + if self._is_ast_node(name, item): + x = self._construct_ast_class(item) + # Custom attribute assignment is allowed + x.foo = 5 + self.assertEqual(x.foo, 5) + def _construct_ast_class(self, cls): kwargs = {} for name, typ in cls.__annotations__.items(): @@ -459,14 +470,9 @@ class AST_Tests(unittest.TestCase): self.assertEqual(x._fields, 666) def test_classattrs(self): - with self.assertWarns(DeprecationWarning): - x = ast.Constant() + x = ast.Constant(42) self.assertEqual(x._fields, ('value', 'kind')) - with self.assertRaises(AttributeError): - x.value - - x = ast.Constant(42) self.assertEqual(x.value, 42) with self.assertRaises(AttributeError): @@ -486,11 +492,8 @@ class AST_Tests(unittest.TestCase): self.assertRaises(TypeError, ast.Constant, 1, None, 2) self.assertRaises(TypeError, ast.Constant, 1, None, 2, lineno=0) - # Arbitrary keyword arguments are supported (but deprecated) - with self.assertWarns(DeprecationWarning): - self.assertEqual(ast.Constant(1, foo='bar').foo, 'bar') - - with self.assertRaisesRegex(TypeError, "Constant got multiple values for argument 'value'"): + msg = "ast.Constant got multiple values for argument 'value'" + with self.assertRaisesRegex(TypeError, re.escape(msg)): ast.Constant(1, value=2) self.assertEqual(ast.Constant(42).value, 42) @@ -529,23 +532,24 @@ class AST_Tests(unittest.TestCase): self.assertEqual(x.body, body) def test_nodeclasses(self): - # Zero arguments constructor explicitly allowed (but deprecated) - with self.assertWarns(DeprecationWarning): - x = ast.BinOp() - self.assertEqual(x._fields, ('left', 'op', 'right')) - - # Random attribute allowed too - x.foobarbaz = 5 - self.assertEqual(x.foobarbaz, 5) + # Zero arguments constructor is not allowed + msg = "ast.BinOp.__init__ missing 3 required positional arguments: 'left', 'op', and 'right'" + self.assertRaisesRegex(TypeError, re.escape(msg), ast.BinOp) n1 = ast.Constant(1) n3 = ast.Constant(3) addop = ast.Add() x = ast.BinOp(n1, addop, n3) + self.assertEqual(x._fields, ('left', 'op', 'right')) self.assertEqual(x.left, n1) self.assertEqual(x.op, addop) self.assertEqual(x.right, n3) + # Arbitrary attributes are allowed + x.foobarbaz = 5 + self.assertEqual(x.foobarbaz, 5) + self.assertEqual(x._fields, ('left', 'op', 'right')) + x = ast.BinOp(1, 2, 3) self.assertEqual(x.left, 1) self.assertEqual(x.op, 2) @@ -569,10 +573,10 @@ class AST_Tests(unittest.TestCase): self.assertEqual(x.right, 3) self.assertEqual(x.lineno, 0) - # Random kwargs also allowed (but deprecated) - with self.assertWarns(DeprecationWarning): - x = ast.BinOp(1, 2, 3, foobarbaz=42) - self.assertEqual(x.foobarbaz, 42) + # Arbitrary keyword arguments are not allowed + msg = "ast.BinOp.__init__ got an unexpected keyword argument 'foobarbaz'" + with self.assertRaisesRegex(TypeError, re.escape(msg)): + ast.BinOp(1, 2, 3, foobarbaz=42) def test_no_fields(self): # this used to fail because Sub._fields was None @@ -1377,14 +1381,14 @@ class CopyTests(unittest.TestCase): self.assertRaises(AttributeError, getattr, repl, 'extra') def test_replace_reject_missing_field(self): - # case: warn if deleted field is not replaced + # case: raise if deleted field is not replaced node = ast.parse('x').body[0].value context = node.ctx del node.id self.assertRaises(AttributeError, getattr, node, 'id') self.assertIs(node.ctx, context) - msg = "Name.__replace__ missing 1 keyword argument: 'id'." + msg = "ast.Name.__init__ missing 1 required positional argument: 'id'" with self.assertRaisesRegex(TypeError, re.escape(msg)): copy.replace(node) # assert that there is no side-effect @@ -1421,7 +1425,7 @@ class CopyTests(unittest.TestCase): # explicit rejection of known instance fields self.assertHasAttr(node, 'extra') - msg = "Name.__replace__ got an unexpected keyword argument 'extra'." + msg = "ast.Name.__init__ got an unexpected keyword argument 'extra'" with self.assertRaisesRegex(TypeError, re.escape(msg)): copy.replace(node, extra=1) # assert that there is no side-effect @@ -1435,7 +1439,7 @@ class CopyTests(unittest.TestCase): # explicit rejection of unknown extra fields self.assertRaises(AttributeError, getattr, node, 'unknown') - msg = "Name.__replace__ got an unexpected keyword argument 'unknown'." + msg = "ast.Name.__init__ got an unexpected keyword argument 'unknown'" with self.assertRaisesRegex(TypeError, re.escape(msg)): copy.replace(node, unknown=1) # assert that there is no side-effect @@ -3200,11 +3204,10 @@ class ASTConstructorTests(unittest.TestCase): args = ast.arguments() self.assertEqual(args.args, []) self.assertEqual(args.posonlyargs, []) - with self.assertWarnsRegex(DeprecationWarning, - r"FunctionDef\.__init__ missing 1 required positional argument: 'name'"): - node = ast.FunctionDef(args=args) - self.assertNotHasAttr(node, "name") - self.assertEqual(node.decorator_list, []) + msg = "ast.FunctionDef.__init__ missing 1 required positional argument: 'name'" + with self.assertRaisesRegex(TypeError, re.escape(msg)): + ast.FunctionDef(args=args) + node = ast.FunctionDef(name='foo', args=args) self.assertEqual(node.name, 'foo') self.assertEqual(node.decorator_list, []) @@ -3222,9 +3225,8 @@ class ASTConstructorTests(unittest.TestCase): self.assertEqual(name3.id, "x") self.assertIsInstance(name3.ctx, ast.Del) - with self.assertWarnsRegex(DeprecationWarning, - r"Name\.__init__ missing 1 required positional argument: 'id'"): - name3 = ast.Name() + msg = "ast.Name.__init__ missing 1 required positional argument: 'id'" + self.assertRaisesRegex(TypeError, re.escape(msg), ast.Name) def test_custom_subclass_with_no_fields(self): class NoInit(ast.AST): @@ -3263,20 +3265,18 @@ class ASTConstructorTests(unittest.TestCase): self.assertEqual(obj.a, 1) self.assertEqual(obj.b, 2) - with self.assertWarnsRegex(DeprecationWarning, - r"MyAttrs.__init__ got an unexpected keyword argument 'c'."): - obj = MyAttrs(c=3) + msg = "MyAttrs.__init__ got an unexpected keyword argument 'c'" + with self.assertRaisesRegex(TypeError, re.escape(msg)): + MyAttrs(c=3) def test_fields_and_types_no_default(self): class FieldsAndTypesNoDefault(ast.AST): _fields = ('a',) _field_types = {'a': int} - with self.assertWarnsRegex(DeprecationWarning, - r"FieldsAndTypesNoDefault\.__init__ missing 1 required positional argument: 'a'\."): - obj = FieldsAndTypesNoDefault() - with self.assertRaises(AttributeError): - obj.a + msg = "FieldsAndTypesNoDefault.__init__ missing 1 required positional argument: 'a'" + self.assertRaisesRegex(TypeError, re.escape(msg), FieldsAndTypesNoDefault) + obj = FieldsAndTypesNoDefault(a=1) self.assertEqual(obj.a, 1) @@ -3287,13 +3287,8 @@ class ASTConstructorTests(unittest.TestCase): a: int | None = None b: int | None = None - with self.assertWarnsRegex( - DeprecationWarning, - r"Field 'b' is missing from MoreFieldsThanTypes\._field_types" - ): - obj = MoreFieldsThanTypes() - self.assertIs(obj.a, None) - self.assertIs(obj.b, None) + msg = r"Field 'b' is missing from .*\.MoreFieldsThanTypes\._field_types" + self.assertRaisesRegex(TypeError, msg, MoreFieldsThanTypes) obj = MoreFieldsThanTypes(a=1, b=2) self.assertEqual(obj.a, 1) @@ -3305,8 +3300,7 @@ class ASTConstructorTests(unittest.TestCase): _field_types = {'a': int} # This should not crash - with self.assertWarnsRegex(DeprecationWarning, r"Field b'\\xff\\xff.*' .*"): - obj = BadFields() + self.assertRaisesRegex(TypeError, r"Field b'\\xff\\xff.*' .*", BadFields) def test_complete_field_types(self): class _AllFieldTypes(ast.AST): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-08-09-19-00-36.gh-issue-137600.p_p6OU.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-08-09-19-00-36.gh-issue-137600.p_p6OU.rst new file mode 100644 index 000000000000..d1d1e36215da --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-08-09-19-00-36.gh-issue-137600.p_p6OU.rst @@ -0,0 +1,4 @@ +:mod:`ast`: The constructors of AST nodes now raise a :exc:`TypeError` when +a required argument is omitted or when a keyword argument that does not map to +a field on the AST node is passed. These cases had previously raised a +:exc:`DeprecationWarning` since Python 3.13. Patch by Brian Schubert. diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index 71a164fbec5a..df4454cf9486 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -873,6 +873,70 @@ ast_clear(PyObject *op) return 0; } +/* + * Format the names in the set 'missing' into a natural language list, + * sorted in the order in which they appear in 'fields'. + * + * Similar to format_missing() from 'Python/ceval.c'. + * + * Parameters + * + * missing Set of missing field names to render. + * fields Sequence of AST node field names (self._fields). + */ +static PyObject * +format_missing(PyObject *missing, PyObject *fields) +{ + Py_ssize_t num_fields, num_total, num_left; + num_fields = PySequence_Size(fields); + if (num_fields == -1) { + return NULL; + } + num_total = num_left = PySet_GET_SIZE(missing); + PyUnicodeWriter *writer = PyUnicodeWriter_Create(0); + if (writer == NULL) { + goto error; + } + // Iterate all AST node fields in order so that the missing positional + // arguments are rendered in the order in which __init__ expects them. + for (Py_ssize_t i = 0; i < num_fields; i++) { + PyObject *name = PySequence_GetItem(fields, i); + if (name == NULL) { + goto error; + } + int contains = PySet_Contains(missing, name); + if (contains == -1) { + Py_DECREF(name); + goto error; + } + else if (contains == 1) { + const char* fmt = NULL; + if (num_left == 1) { + fmt = "'%U'"; + } + else if (num_total == 2) { + fmt = "'%U' and "; + } + else if (num_left == 2) { + fmt = "'%U', and "; + } + else { + fmt = "'%U', "; + } + num_left--; + if (PyUnicodeWriter_Format(writer, fmt, name) < 0) { + Py_DECREF(name); + goto error; + } + } + Py_DECREF(name); + } + return PyUnicodeWriter_Finish(writer); +error: + PyUnicodeWriter_Discard(writer); + return NULL; +} + static int ast_type_init(PyObject *self, PyObject *args, PyObject *kw) { @@ -942,8 +1006,8 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } if (p == 0) { PyErr_Format(PyExc_TypeError, - "%.400s got multiple values for argument %R", - Py_TYPE(self)->tp_name, key); + "%T got multiple values for argument %R", + self, key); res = -1; goto cleanup; } @@ -963,16 +1027,11 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) goto cleanup; } else if (contains == 0) { - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "%.400s.__init__ got an unexpected keyword argument %R. " - "Support for arbitrary keyword arguments is deprecated " - "and will be removed in Python 3.15.", - Py_TYPE(self)->tp_name, key - ) < 0) { - res = -1; - goto cleanup; - } + PyErr_Format(PyExc_TypeError, + "%T.__init__ got an unexpected keyword argument %R", + self, key); + res = -1; + goto cleanup; } } res = PyObject_SetAttr(self, key, value); @@ -982,7 +1041,7 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } } Py_ssize_t size = PySet_Size(remaining_fields); - PyObject *field_types = NULL, *remaining_list = NULL; + PyObject *field_types = NULL, *remaining_list = NULL, *missing_names = NULL; if (size > 0) { if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), &field_types) < 0) { @@ -999,6 +1058,10 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) if (!remaining_list) { goto set_remaining_cleanup; } + missing_names = PySet_New(NULL); + if (!missing_names) { + goto set_remaining_cleanup; + } for (Py_ssize_t i = 0; i < size; i++) { PyObject *name = PyList_GET_ITEM(remaining_list, i); PyObject *type = PyDict_GetItemWithError(field_types, name); @@ -1007,14 +1070,10 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) goto set_remaining_cleanup; } else { - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "Field %R is missing from %.400s._field_types. " - "This will become an error in Python 3.15.", - name, Py_TYPE(self)->tp_name - ) < 0) { - goto set_remaining_cleanup; - } + PyErr_Format(PyExc_TypeError, + "Field %R is missing from %T._field_types", + name, self); + goto set_remaining_cleanup; } } else if (_PyUnion_Check(type)) { @@ -1042,16 +1101,25 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } else { // simple field (e.g., identifier) - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "%.400s.__init__ missing 1 required positional argument: %R. " - "This will become an error in Python 3.15.", - Py_TYPE(self)->tp_name, name - ) < 0) { + res = PySet_Add(missing_names, name); + if (res < 0) { goto set_remaining_cleanup; } } } + Py_ssize_t num_missing = PySet_GET_SIZE(missing_names); + if (num_missing > 0) { + PyObject *name_str = format_missing(missing_names, fields); + if (!name_str) { + goto set_remaining_cleanup; + } + PyErr_Format(PyExc_TypeError, + "%T.__init__ missing %d required positional argument%s: %U", + self, num_missing, num_missing == 1 ? "" : "s", name_str); + Py_DECREF(name_str); + goto set_remaining_cleanup; + } + Py_DECREF(missing_names); Py_DECREF(remaining_list); Py_DECREF(field_types); } @@ -1061,6 +1129,7 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) Py_XDECREF(remaining_fields); return res; set_remaining_cleanup: + Py_XDECREF(missing_names); Py_XDECREF(remaining_list); Py_XDECREF(field_types); res = -1; @@ -1144,182 +1213,6 @@ cleanup: return result; } -/* - * Perform the following validations: - * - * - All keyword arguments are known 'fields' or 'attributes'. - * - No field or attribute would be left unfilled after copy.replace(). - * - * On success, this returns 1. Otherwise, set a TypeError - * exception and returns -1 (no exception is set if some - * other internal errors occur). - * - * Parameters - * - * self The AST node instance. - * dict The AST node instance dictionary (self.__dict__). - * fields The list of fields (self._fields). - * attributes The list of attributes (self._attributes). - * kwargs Keyword arguments passed to ast_type_replace(). - * - * The 'dict', 'fields', 'attributes' and 'kwargs' arguments can be NULL. - * - * Note: this function can be removed in 3.15 since the verification - * will be done inside the constructor. - */ -static inline int -ast_type_replace_check(PyObject *self, - PyObject *dict, - PyObject *fields, - PyObject *attributes, - PyObject *kwargs) -{ - // While it is possible to make some fast paths that would avoid - // allocating objects on the stack, this would cost us readability. - // For instance, if 'fields' and 'attributes' are both empty, and - // 'kwargs' is not empty, we could raise a TypeError immediately. - PyObject *expecting = PySet_New(fields); - if (expecting == NULL) { - return -1; - } - if (attributes) { - if (_PySet_Update(expecting, attributes) < 0) { - Py_DECREF(expecting); - return -1; - } - } - // Any keyword argument that is neither a field nor attribute is rejected. - // We first need to check whether a keyword argument is accepted or not. - // If all keyword arguments are accepted, we compute the required fields - // and attributes. A field or attribute is not needed if: - // - // 1) it is given in 'kwargs', or - // 2) it already exists on 'self'. - if (kwargs) { - Py_ssize_t pos = 0; - PyObject *key, *value; - while (PyDict_Next(kwargs, &pos, &key, &value)) { - int rc = PySet_Discard(expecting, key); - if (rc < 0) { - Py_DECREF(expecting); - return -1; - } - if (rc == 0) { - PyErr_Format(PyExc_TypeError, - "%.400s.__replace__ got an unexpected keyword " - "argument %R.", Py_TYPE(self)->tp_name, key); - Py_DECREF(expecting); - return -1; - } - } - } - // check that the remaining fields or attributes would be filled - if (dict) { - Py_ssize_t pos = 0; - PyObject *key, *value; - while (PyDict_Next(dict, &pos, &key, &value)) { - // Mark fields or attributes that are found on the instance - // as non-mandatory. If they are not given in 'kwargs', they - // will be shallow-coied; otherwise, they would be replaced - // (not in this function). - if (PySet_Discard(expecting, key) < 0) { - Py_DECREF(expecting); - return -1; - } - } - if (attributes) { - // Some attributes may or may not be present at runtime. - // In particular, now that we checked whether 'kwargs' - // is correct or not, we allow any attribute to be missing. - // - // Note that fields must still be entirely determined when - // calling the constructor later. - PyObject *unused = PyObject_CallMethodOneArg(expecting, - &_Py_ID(difference_update), - attributes); - if (unused == NULL) { - Py_DECREF(expecting); - return -1; - } - Py_DECREF(unused); - } - } - - // Discard fields from 'expecting' that default to None - PyObject *field_types = NULL; - if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), - &_Py_ID(_field_types), - &field_types) < 0) - { - Py_DECREF(expecting); - return -1; - } - if (field_types != NULL) { - Py_ssize_t pos = 0; - PyObject *field_name, *field_type; - while (PyDict_Next(field_types, &pos, &field_name, &field_type)) { - if (_PyUnion_Check(field_type)) { - // optional field - if (PySet_Discard(expecting, field_name) < 0) { - Py_DECREF(expecting); - Py_DECREF(field_types); - return -1; - } - } - } - Py_DECREF(field_types); - } - - // Now 'expecting' contains the fields or attributes - // that would not be filled inside ast_type_replace(). - Py_ssize_t m = PySet_GET_SIZE(expecting); - if (m > 0) { - PyObject *names = PyList_New(m); - if (names == NULL) { - Py_DECREF(expecting); - return -1; - } - Py_ssize_t i = 0, pos = 0; - PyObject *item; - Py_hash_t hash; - while (_PySet_NextEntry(expecting, &pos, &item, &hash)) { - PyObject *name = PyObject_Repr(item); - if (name == NULL) { - Py_DECREF(expecting); - Py_DECREF(names); - return -1; - } - // steal the reference 'name' - PyList_SET_ITEM(names, i++, name); - } - Py_DECREF(expecting); - if (PyList_Sort(names) < 0) { - Py_DECREF(names); - return -1; - } - PyObject *sep = PyUnicode_FromString(", "); - if (sep == NULL) { - Py_DECREF(names); - return -1; - } - PyObject *str_names = PyUnicode_Join(sep, names); - Py_DECREF(sep); - Py_DECREF(names); - if (str_names == NULL) { - return -1; - } - PyErr_Format(PyExc_TypeError, - "%.400s.__replace__ missing %ld keyword argument%s: %U.", - Py_TYPE(self)->tp_name, m, m == 1 ? "" : "s", str_names); - Py_DECREF(str_names); - return -1; - } - else { - Py_DECREF(expecting); - return 1; - } -} - /* * Python equivalent: * @@ -1409,9 +1302,6 @@ ast_type_replace(PyObject *self, PyObject *args, PyObject *kwargs) if (PyObject_GetOptionalAttr(self, state->__dict__, &dict) < 0) { goto cleanup; } - if (ast_type_replace_check(self, dict, fields, attributes, kwargs) < 0) { - goto cleanup; - } empty_tuple = PyTuple_New(0); if (empty_tuple == NULL) { goto cleanup; diff --git a/Python/Python-ast.c b/Python/Python-ast.c index dad1530e343a..6bcf57bdd6b4 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5197,6 +5197,70 @@ ast_clear(PyObject *op) return 0; } +/* + * Format the names in the set 'missing' into a natural language list, + * sorted in the order in which they appear in 'fields'. + * + * Similar to format_missing() from 'Python/ceval.c'. + * + * Parameters + * + * missing Set of missing field names to render. + * fields Sequence of AST node field names (self._fields). + */ +static PyObject * +format_missing(PyObject *missing, PyObject *fields) +{ + Py_ssize_t num_fields, num_total, num_left; + num_fields = PySequence_Size(fields); + if (num_fields == -1) { + return NULL; + } + num_total = num_left = PySet_GET_SIZE(missing); + PyUnicodeWriter *writer = PyUnicodeWriter_Create(0); + if (writer == NULL) { + goto error; + } + // Iterate all AST node fields in order so that the missing positional + // arguments are rendered in the order in which __init__ expects them. + for (Py_ssize_t i = 0; i < num_fields; i++) { + PyObject *name = PySequence_GetItem(fields, i); + if (name == NULL) { + goto error; + } + int contains = PySet_Contains(missing, name); + if (contains == -1) { + Py_DECREF(name); + goto error; + } + else if (contains == 1) { + const char* fmt = NULL; + if (num_left == 1) { + fmt = "'%U'"; + } + else if (num_total == 2) { + fmt = "'%U' and "; + } + else if (num_left == 2) { + fmt = "'%U', and "; + } + else { + fmt = "'%U', "; + } + num_left--; + if (PyUnicodeWriter_Format(writer, fmt, name) < 0) { + Py_DECREF(name); + goto error; + } + } + Py_DECREF(name); + } + return PyUnicodeWriter_Finish(writer); +error: + PyUnicodeWriter_Discard(writer); + return NULL; +} + static int ast_type_init(PyObject *self, PyObject *args, PyObject *kw) { @@ -5266,8 +5330,8 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } if (p == 0) { PyErr_Format(PyExc_TypeError, - "%.400s got multiple values for argument %R", - Py_TYPE(self)->tp_name, key); + "%T got multiple values for argument %R", + self, key); res = -1; goto cleanup; } @@ -5287,16 +5351,11 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) goto cleanup; } else if (contains == 0) { - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "%.400s.__init__ got an unexpected keyword argument %R. " - "Support for arbitrary keyword arguments is deprecated " - "and will be removed in Python 3.15.", - Py_TYPE(self)->tp_name, key - ) < 0) { - res = -1; - goto cleanup; - } + PyErr_Format(PyExc_TypeError, + "%T.__init__ got an unexpected keyword argument %R", + self, key); + res = -1; + goto cleanup; } } res = PyObject_SetAttr(self, key, value); @@ -5306,7 +5365,7 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } } Py_ssize_t size = PySet_Size(remaining_fields); - PyObject *field_types = NULL, *remaining_list = NULL; + PyObject *field_types = NULL, *remaining_list = NULL, *missing_names = NULL; if (size > 0) { if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), &field_types) < 0) { @@ -5323,6 +5382,10 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) if (!remaining_list) { goto set_remaining_cleanup; } + missing_names = PySet_New(NULL); + if (!missing_names) { + goto set_remaining_cleanup; + } for (Py_ssize_t i = 0; i < size; i++) { PyObject *name = PyList_GET_ITEM(remaining_list, i); PyObject *type = PyDict_GetItemWithError(field_types, name); @@ -5331,14 +5394,10 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) goto set_remaining_cleanup; } else { - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "Field %R is missing from %.400s._field_types. " - "This will become an error in Python 3.15.", - name, Py_TYPE(self)->tp_name - ) < 0) { - goto set_remaining_cleanup; - } + PyErr_Format(PyExc_TypeError, + "Field %R is missing from %T._field_types", + name, self); + goto set_remaining_cleanup; } } else if (_PyUnion_Check(type)) { @@ -5366,16 +5425,25 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) } else { // simple field (e.g., identifier) - if (PyErr_WarnFormat( - PyExc_DeprecationWarning, 1, - "%.400s.__init__ missing 1 required positional argument: %R. " - "This will become an error in Python 3.15.", - Py_TYPE(self)->tp_name, name - ) < 0) { + res = PySet_Add(missing_names, name); + if (res < 0) { goto set_remaining_cleanup; } } } + Py_ssize_t num_missing = PySet_GET_SIZE(missing_names); + if (num_missing > 0) { + PyObject *name_str = format_missing(missing_names, fields); + if (!name_str) { + goto set_remaining_cleanup; + } + PyErr_Format(PyExc_TypeError, + "%T.__init__ missing %d required positional argument%s: %U", + self, num_missing, num_missing == 1 ? "" : "s", name_str); + Py_DECREF(name_str); + goto set_remaining_cleanup; + } + Py_DECREF(missing_names); Py_DECREF(remaining_list); Py_DECREF(field_types); } @@ -5385,6 +5453,7 @@ ast_type_init(PyObject *self, PyObject *args, PyObject *kw) Py_XDECREF(remaining_fields); return res; set_remaining_cleanup: + Py_XDECREF(missing_names); Py_XDECREF(remaining_list); Py_XDECREF(field_types); res = -1; @@ -5468,182 +5537,6 @@ cleanup: return result; } -/* - * Perform the following validations: - * - * - All keyword arguments are known 'fields' or 'attributes'. - * - No field or attribute would be left unfilled after copy.replace(). - * - * On success, this returns 1. Otherwise, set a TypeError - * exception and returns -1 (no exception is set if some - * other internal errors occur). - * - * Parameters - * - * self The AST node instance. - * dict The AST node instance dictionary (self.__dict__). - * fields The list of fields (self._fields). - * attributes The list of attributes (self._attributes). - * kwargs Keyword arguments passed to ast_type_replace(). - * - * The 'dict', 'fields', 'attributes' and 'kwargs' arguments can be NULL. - * - * Note: this function can be removed in 3.15 since the verification - * will be done inside the constructor. - */ -static inline int -ast_type_replace_check(PyObject *self, - PyObject *dict, - PyObject *fields, - PyObject *attributes, - PyObject *kwargs) -{ - // While it is possible to make some fast paths that would avoid - // allocating objects on the stack, this would cost us readability. - // For instance, if 'fields' and 'attributes' are both empty, and - // 'kwargs' is not empty, we could raise a TypeError immediately. - PyObject *expecting = PySet_New(fields); - if (expecting == NULL) { - return -1; - } - if (attributes) { - if (_PySet_Update(expecting, attributes) < 0) { - Py_DECREF(expecting); - return -1; - } - } - // Any keyword argument that is neither a field nor attribute is rejected. - // We first need to check whether a keyword argument is accepted or not. - // If all keyword arguments are accepted, we compute the required fields - // and attributes. A field or attribute is not needed if: - // - // 1) it is given in 'kwargs', or - // 2) it already exists on 'self'. - if (kwargs) { - Py_ssize_t pos = 0; - PyObject *key, *value; - while (PyDict_Next(kwargs, &pos, &key, &value)) { - int rc = PySet_Discard(expecting, key); - if (rc < 0) { - Py_DECREF(expecting); - return -1; - } - if (rc == 0) { - PyErr_Format(PyExc_TypeError, - "%.400s.__replace__ got an unexpected keyword " - "argument %R.", Py_TYPE(self)->tp_name, key); - Py_DECREF(expecting); - return -1; - } - } - } - // check that the remaining fields or attributes would be filled - if (dict) { - Py_ssize_t pos = 0; - PyObject *key, *value; - while (PyDict_Next(dict, &pos, &key, &value)) { - // Mark fields or attributes that are found on the instance - // as non-mandatory. If they are not given in 'kwargs', they - // will be shallow-coied; otherwise, they would be replaced - // (not in this function). - if (PySet_Discard(expecting, key) < 0) { - Py_DECREF(expecting); - return -1; - } - } - if (attributes) { - // Some attributes may or may not be present at runtime. - // In particular, now that we checked whether 'kwargs' - // is correct or not, we allow any attribute to be missing. - // - // Note that fields must still be entirely determined when - // calling the constructor later. - PyObject *unused = PyObject_CallMethodOneArg(expecting, - &_Py_ID(difference_update), - attributes); - if (unused == NULL) { - Py_DECREF(expecting); - return -1; - } - Py_DECREF(unused); - } - } - - // Discard fields from 'expecting' that default to None - PyObject *field_types = NULL; - if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), - &_Py_ID(_field_types), - &field_types) < 0) - { - Py_DECREF(expecting); - return -1; - } - if (field_types != NULL) { - Py_ssize_t pos = 0; - PyObject *field_name, *field_type; - while (PyDict_Next(field_types, &pos, &field_name, &field_type)) { - if (_PyUnion_Check(field_type)) { - // optional field - if (PySet_Discard(expecting, field_name) < 0) { - Py_DECREF(expecting); - Py_DECREF(field_types); - return -1; - } - } - } - Py_DECREF(field_types); - } - - // Now 'expecting' contains the fields or attributes - // that would not be filled inside ast_type_replace(). - Py_ssize_t m = PySet_GET_SIZE(expecting); - if (m > 0) { - PyObject *names = PyList_New(m); - if (names == NULL) { - Py_DECREF(expecting); - return -1; - } - Py_ssize_t i = 0, pos = 0; - PyObject *item; - Py_hash_t hash; - while (_PySet_NextEntry(expecting, &pos, &item, &hash)) { - PyObject *name = PyObject_Repr(item); - if (name == NULL) { - Py_DECREF(expecting); - Py_DECREF(names); - return -1; - } - // steal the reference 'name' - PyList_SET_ITEM(names, i++, name); - } - Py_DECREF(expecting); - if (PyList_Sort(names) < 0) { - Py_DECREF(names); - return -1; - } - PyObject *sep = PyUnicode_FromString(", "); - if (sep == NULL) { - Py_DECREF(names); - return -1; - } - PyObject *str_names = PyUnicode_Join(sep, names); - Py_DECREF(sep); - Py_DECREF(names); - if (str_names == NULL) { - return -1; - } - PyErr_Format(PyExc_TypeError, - "%.400s.__replace__ missing %ld keyword argument%s: %U.", - Py_TYPE(self)->tp_name, m, m == 1 ? "" : "s", str_names); - Py_DECREF(str_names); - return -1; - } - else { - Py_DECREF(expecting); - return 1; - } -} - /* * Python equivalent: * @@ -5733,9 +5626,6 @@ ast_type_replace(PyObject *self, PyObject *args, PyObject *kwargs) if (PyObject_GetOptionalAttr(self, state->__dict__, &dict) < 0) { goto cleanup; } - if (ast_type_replace_check(self, dict, fields, attributes, kwargs) < 0) { - goto cleanup; - } empty_tuple = PyTuple_New(0); if (empty_tuple == NULL) { goto cleanup; -- 2.47.3