-
-
Notifications
You must be signed in to change notification settings - Fork 34k
gh-144169: Fix three crashes in AST objects with non-str kwargs #144178
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?
Conversation
|
I closed my PR #144177 since this one is more complete. |
vstinner
left a comment
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.
LGTM. I just left minor suggestions.
|
|
||
| def test_replace_non_str_kwarg(self): | ||
| node = ast.Name(id="x") | ||
| with self.assertRaisesRegex(TypeError, "got an unexpected keyword argument <object object"): |
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.
This line is longer than 80 columns, I suggest to write it like that:
| with self.assertRaisesRegex(TypeError, "got an unexpected keyword argument <object object"): | |
| errmsg = "got an unexpected keyword argument <object object" | |
| with self.assertRaisesRegex(TypeError, errmsg): |
| with ( | ||
| self.assertRaises(TypeError), | ||
| self.assertWarnsRegex(DeprecationWarning, "got an unexpected keyword argument <object object"), | ||
| ): |
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.
The last line is longer than 80 columns. I suggest to write it like that:
| with ( | |
| self.assertRaises(TypeError), | |
| self.assertWarnsRegex(DeprecationWarning, "got an unexpected keyword argument <object object"), | |
| ): | |
| warn_msg = "got an unexpected keyword argument <object object" | |
| with ( | |
| self.assertRaises(TypeError), | |
| self.assertWarnsRegex(DeprecationWarning, warn_msg), | |
| ): |
| "%.400s got multiple values for argument %R", | ||
| Py_TYPE(self)->tp_name, key); |
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.
Since we are modifying the error message, you must use the safer %T format:
| "%.400s got multiple values for argument %R", | |
| Py_TYPE(self)->tp_name, key); | |
| "%T got multiple values for argument %R", | |
| self, key); |
Same remark for the two other changes below.
Uh oh!
There was an error while loading. Please reload this page.