Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Doc/library/ast.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ The abstract grammar is currently defined as follows:
:language: asdl


.. _ast_nodes:

Node classes
------------

Expand Down Expand Up @@ -158,8 +160,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
Expand Down
10 changes: 10 additions & 0 deletions Doc/whatsnew/3.15.rst
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,16 @@ csv
Removed
=======

ast
---

* The constructors of :ref:`AST nodes <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`.)


ctypes
------

Expand Down
105 changes: 54 additions & 51 deletions Lib/test/test_ast/test_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,18 @@ def test_field_attr_existence(self):
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)
del x.foo
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed? Doesn't the object get deleted anyway?


def _construct_ast_class(self, cls):
kwargs = {}
for name, typ in cls.__annotations__.items():
Expand Down Expand Up @@ -459,14 +471,12 @@ def test_field_attr_writable(self):
self.assertEqual(x._fields, 666)

def test_classattrs(self):
with self.assertWarns(DeprecationWarning):
x = ast.Constant()
self.assertEqual(x._fields, ('value', 'kind'))

with self.assertRaises(AttributeError):
x.value
msg = "ast.Constant.__init__ missing 1 required positional argument: 'value'"
Copy link
Member

Choose a reason for hiding this comment

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

This test now feels out of scope for the test function. If we're already sufficiently testing that this case errors out (I assume we are), let's just remove this line.

self.assertRaisesRegex(TypeError, re.escape(msg), ast.Constant)

x = ast.Constant(42)
self.assertEqual(x._fields, ('value', 'kind'))

self.assertEqual(x.value, 42)

with self.assertRaises(AttributeError):
Expand All @@ -486,11 +496,13 @@ def test_classattrs(self):
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')
# Arbitrary keyword arguments are not supported
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this is now testing something unrelated to what this function is about.

Copy link
Member

Choose a reason for hiding this comment

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

My intent here was to be sure we were ignoring kwargs.

msg = "ast.Constant.__init__ got an unexpected keyword argument 'foo'"
with self.assertRaisesRegex(TypeError, re.escape(msg)):
ast.Constant(1, 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)
Expand Down Expand Up @@ -529,23 +541,24 @@ def test_module(self):
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)

# Random attribute allowed too
x.foobarbaz = 5
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if we already cover this, but maybe we should instead:

  • Find a way to construct all nodes "correctly" or have at least one instance of each nodes.
  • For all these instances, check that we can set random attributes and delete them properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, added a test in the style of the existing test_field_attr_existence test.

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)
Expand All @@ -569,10 +582,10 @@ def test_nodeclasses(self):
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)
# Random kwargs 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
Expand Down Expand Up @@ -1374,14 +1387,14 @@ def test_replace_ignore_known_custom_instance_fields(self):
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
Expand Down Expand Up @@ -1418,7 +1431,7 @@ def test_replace_reject_known_custom_instance_fields_commits(self):

# 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
Expand All @@ -1432,7 +1445,7 @@ def test_replace_reject_unknown_instance_fields(self):

# 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
Expand Down Expand Up @@ -3178,11 +3191,10 @@ def test_FunctionDef(self):
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, [])
Expand All @@ -3200,9 +3212,8 @@ def test_expr_context(self):
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):
Expand Down Expand Up @@ -3241,20 +3252,18 @@ class MyAttrs(ast.AST):
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)

Expand All @@ -3265,13 +3274,8 @@ class MoreFieldsThanTypes(ast.AST):
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)
Expand All @@ -3283,8 +3287,7 @@ class BadFields(ast.AST):
_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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Loading
Loading