-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
gh-137600: Promote ast node constructor deprecation warnings to errors
#137601
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
d834320
de5564a
4f4466f
de883a5
39cdf20
1d3a3b5
c066338
48267a3
fa270ff
606708a
f5b2582
92ac6d1
6ccb623
b0e1c67
2d7d5da
b282f96
85986cd
309bc4e
278388b
f3e21e6
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 |
|---|---|---|
|
|
@@ -458,14 +458,13 @@ def test_field_attr_writable(self): | |
| self.assertEqual(x._fields, 666) | ||
|
|
||
| def test_classattrs(self): | ||
| with self.assertWarns(DeprecationWarning): | ||
| msg = "ast.Constant.__init__ missing 1 required positional argument: 'value'" | ||
|
Member
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 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. |
||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| x = ast.Constant() | ||
| self.assertEqual(x._fields, ('value', 'kind')) | ||
|
|
||
| with self.assertRaises(AttributeError): | ||
| x.value | ||
|
|
||
| x = ast.Constant(42) | ||
| self.assertEqual(x._fields, ('value', 'kind')) | ||
|
|
||
| self.assertEqual(x.value, 42) | ||
|
|
||
| with self.assertRaises(AttributeError): | ||
|
|
@@ -485,11 +484,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 | ||
|
Member
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. Same here, this is now testing something unrelated to what this function is about.
Member
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 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) | ||
|
|
@@ -528,23 +529,25 @@ def test_module(self): | |
| self.assertEqual(x.body, body) | ||
|
|
||
| def test_nodeclasses(self): | ||
| # Zero arguments constructor explicitly allowed (but deprecated) | ||
| with self.assertWarns(DeprecationWarning): | ||
| # Zero arguments constructor is not allowed | ||
| msg = "ast.BinOp.__init__ missing 3 required positional arguments: 'left', 'op', and 'right'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| x = ast.BinOp() | ||
| self.assertEqual(x._fields, ('left', 'op', 'right')) | ||
|
|
||
| # Random attribute allowed too | ||
| x.foobarbaz = 5 | ||
| self.assertEqual(x.foobarbaz, 5) | ||
|
|
||
| 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 | ||
brianschubert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| x.foobarbaz = 5 | ||
|
Member
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 don't know if we already cover this, but maybe we should instead:
Contributor
Author
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. Good idea, added a test in the style of the existing |
||
| 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) | ||
|
|
@@ -568,10 +571,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): | ||
| # Random kwargs are not allowed | ||
| msg = "ast.BinOp.__init__ got an unexpected keyword argument 'foobarbaz'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| x = ast.BinOp(1, 2, 3, foobarbaz=42) | ||
| self.assertEqual(x.foobarbaz, 42) | ||
|
|
||
| def test_no_fields(self): | ||
| # this used to fail because Sub._fields was None | ||
|
|
@@ -1412,7 +1415,7 @@ def test_replace_reject_missing_field(self): | |
|
|
||
| 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'" | ||
brianschubert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| copy.replace(node) | ||
| # assert that there is no side-effect | ||
|
|
@@ -1449,7 +1452,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 | ||
|
|
@@ -1463,7 +1466,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 | ||
|
|
@@ -3209,11 +3212,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'"): | ||
| msg = "ast.FunctionDef.__init__ missing 1 required positional argument: 'name'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| node = ast.FunctionDef(args=args) | ||
| self.assertNotHasAttr(node, "name") | ||
| self.assertEqual(node.decorator_list, []) | ||
|
|
||
| node = ast.FunctionDef(name='foo', args=args) | ||
| self.assertEqual(node.name, 'foo') | ||
| self.assertEqual(node.decorator_list, []) | ||
|
|
@@ -3231,8 +3233,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'"): | ||
| msg = "ast.Name.__init__ missing 1 required positional argument: 'id'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| name3 = ast.Name() | ||
|
|
||
| def test_custom_subclass_with_no_fields(self): | ||
|
|
@@ -3272,20 +3274,19 @@ 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'."): | ||
| msg = "MyAttrs.__init__ got an unexpected keyword argument 'c'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| obj = 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'\."): | ||
| msg = "FieldsAndTypesNoDefault.__init__ missing 1 required positional argument: 'a'" | ||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| obj = FieldsAndTypesNoDefault() | ||
| with self.assertRaises(AttributeError): | ||
| obj.a | ||
|
|
||
| obj = FieldsAndTypesNoDefault(a=1) | ||
| self.assertEqual(obj.a, 1) | ||
|
|
||
|
|
@@ -3296,13 +3297,9 @@ 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" | ||
| ): | ||
| msg = "Field 'b' is missing from test.test_ast.test_ast.ASTConstructorTests.test_incomplete_field_types.<locals>.MoreFieldsThanTypes._field_types" | ||
brianschubert marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| with self.assertRaisesRegex(TypeError, re.escape(msg)): | ||
| obj = MoreFieldsThanTypes() | ||
| self.assertIs(obj.a, None) | ||
| self.assertIs(obj.b, None) | ||
|
|
||
| obj = MoreFieldsThanTypes(a=1, b=2) | ||
| self.assertEqual(obj.a, 1) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| The constructors of node types in the :mod:`ast` module 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. |
Uh oh!
There was an error while loading. Please reload this page.