FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-32888: Improve exception message in ast.literal_eval by Rosuav · Pull Request #340 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-32888: Improve exception message in ast.literal_eval #340

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.

Sign up for GitHub

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

Closed
Rosuav wants to merge 4 commits into from
+26 −8
Conversation 24 Commits 4 Checks 0 Files changed 4
Closed

bpo-32888: Improve exception message in ast.literal_eval #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26ba98e
Improve exception message in ast.literal_eval
Rosuav Feb 27, 2017
08ffc03
Include patch author name in NEWS
Rosuav Feb 20, 2018
fb83598
Distinguish different causes of error
Rosuav Feb 20, 2018
cce0c6c
Add test of different forms of exception message
Rosuav Feb 20, 2018
File filter

Filter by extension

Filter by extension .py  (3) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
16 changes: 9 additions & 7 deletions Lib/ast.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,23 @@ def literal_eval(node_or_string):
node_or_string = parse(node_or_string, mode='eval')
if isinstance(node_or_string, Expression):
node_or_string = node_or_string.body
def _convert_num(node):
def _convert_num(node, ctx):
if isinstance(node, Constant):
if isinstance(node.value, (int, float, complex)):
return node.value
elif isinstance(node, Num):
return node.n
elif isinstance(node, AST):
raise ValueError('%s not allowed in %s' % (type(node).__name__, ctx))
raise ValueError('malformed node or string: ' + repr(node))

serhiy-storchaka Feb 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In what cases this exception is raised? Is it covered by tests?

Rosuav Feb 20, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The easiest way to trigger it is to use something that isn't permitted in literal_eval, but is valid syntax. There's already one such test - proving that f-strings aren't literal_evallable - and I guess there could be more, but since this is a whitelist, the error is basically "anything else, raise". The only difference in this patch is that the message is different for "syntactically valid but not acceptable by literal_eval" and "syntactically invalid".

serhiy-storchaka Feb 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I meant what input triggers the "malformed node or string" error now?

Rosuav Feb 20, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Calling literal_eval with an AST node that has non-AST children in it. I don't think it's possible (post-patch) to trigger this error by calling literal_eval with a string, but I can't say for sure, so I don't want to say just "malformed node".
def _convert_signed_num(node):
def _convert_signed_num(node, ctx):
if isinstance(node, UnaryOp) and isinstance(node.op, (UAdd, USub)):
operand = _convert_num(node.operand)
operand = _convert_num(node.operand, 'unary +/-')
if isinstance(node.op, UAdd):
return + operand
else:
return - operand
return _convert_num(node)
return _convert_num(node, ctx)
def _convert(node):
if isinstance(node, Constant):
return node.value
Expand All @@ -80,14 +82,14 @@ def _convert(node):
elif isinstance(node, NameConstant):
return node.value
elif isinstance(node, BinOp) and isinstance(node.op, (Add, Sub)):
left = _convert_signed_num(node.left)
right = _convert_num(node.right)
left = _convert_signed_num(node.left, 'binary +/-')
right = _convert_num(node.right, 'binary +/-')
if isinstance(left, (int, float)) and isinstance(right, complex):
if isinstance(node.op, Add):
return left + right
else:
return left - right
return _convert_signed_num(node)
return _convert_signed_num(node, 'literal')
return _convert(node_or_string)


Expand Down
13 changes: 13 additions & 0 deletions Lib/test/test_ast.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,19 @@ def test_literal_eval_complex(self):
self.assertRaises(ValueError, ast.literal_eval, '3+(0+6j)')
self.assertRaises(ValueError, ast.literal_eval, '-(3+6j)')

def test_literal_eval_message(self):
# Issue #32888
tests = {
"2 * 5": "BinOp not allowed in literal",
"[] + []": "List not allowed in binary +/-",
"+''": "Str not allowed in unary +/-",
ast.BinOp(ast.Num(1), ast.Add(), 'oops'): "malformed node or string: 'oops'",
}
for test, message in tests.items():
with self.assertRaises(ValueError) as cm:

briancurtin Jul 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A helpful thing to use in between lines 594 and 595 would be with self.subTest(test=test, message=message): and then indent the parts below it. That way each of these end up being tested independently rather than potentially having the items loop have a case fail and cause the rest of the cases to fail to execute.

Check out https://docs.python.org/3/library/unittest.html#distinguishing-test-iterations-using-subtests
ast.literal_eval(test)
self.assertIn(message, str(cm.exception))

def test_bad_integer(self):
# issue13436: Bad error message with invalid numeric values
body = [ast.ImportFrom(module='time',
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_fstring.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ def g():
self.assertIsNone(g.__doc__)

def test_literal_eval(self):
with self.assertRaisesRegex(ValueError, 'malformed node or string'):
with self.assertRaisesRegex(ValueError, 'JoinedStr not allowed in literal'):
ast.literal_eval("f'x'")

def test_ast_compile_time_concat(self):
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2018-02-21-04-39-49.bpo-32888.G5i41Q.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Improve wording of error messages from ast.literal_eval, distinguishing
valid-but-unacceptable nodes from those that are actually malformed.

serhiy-storchaka Feb 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please add "Patch by..."

Rosuav Feb 20, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should that be done by amending the current commit, or adding an additional commit?

serhiy-storchaka Feb 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It is better for review to add a new commit. In any case all commits are squashed before merging into the repository.

Rosuav Feb 20, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👍
Patch by Chris Angelico.
Back | FazBrowse Home | New Git URL