Skip to content

bpo-41520: codeop no longer ignores SyntaxWarning #21838

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

Merged
merged 1 commit into from
Aug 12, 2020
Merged

bpo-41520: codeop no longer ignores SyntaxWarning #21838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions Lib/codeop.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@ def _maybe_compile(compiler, source, filename, symbol):
except SyntaxError:
pass

# Suppress warnings after the first compile to avoid duplication.
# Catch syntax warnings after the first compile
# to emit SyntaxWarning at most once.
with warnings.catch_warnings():
warnings.simplefilter("ignore")
warnings.simplefilter("error", SyntaxWarning)
Copy link
Member

@terryjreedy terryjreedy Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ignoring all warnings was intentional. Compile also issues DeprecationWarnings. Narrowing the filter to SyntaxWarning only reintroduces the error of DeprecationWarning being issued thrice, which was fixed by the previous code. I will write more on the issue.

Followup PR fixes regression by removing SyntaxWarning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then maybe the catch warnings need to be put in there 3 times, to reset the warning filter on each subsequent compilations ?


try:
code1 = compiler(source + "\n", filename, symbol)
except SyntaxError as e:
Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_codeop.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
import sys
import unittest
import warnings
from test import support
from test.support import warnings_helper

Expand Down Expand Up @@ -310,5 +311,11 @@ def test_warning(self):
compile_command("0 is 0")
self.assertEqual(len(w.warnings), 1)

# bpo-41520: check SyntaxWarning treated as an SyntaxError
with self.assertRaises(SyntaxError):
warnings.simplefilter('error', SyntaxWarning)
compile_command('1 is 1\n', symbol='exec')
Copy link
Member

@terryjreedy terryjreedy Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This source should not have '\n' as single lines are passed without it. That is why to recompile with \n and \n\n added. Fixed in followup PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even w/o the \n 3.8.5 does not raise the SyntaxError.

$ cat foo.py
import warnings
from codeop import compile_command

warnings.simplefilter('error', SyntaxWarning)
res = compile_command('1 is 1', symbol='exec')
print('Res', res)

$ python foo.py
Res None

Copy link
Member

Choose a reason for hiding this comment

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

I expect that, but aside from your issue, the test if better if it calls compile_command the way it is intended to be used and is used by code.InteractiveInterpreter.



if __name__ == "__main__":
unittest.main()
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2020-08-12-13-25-16.bpo-41520.BEUWa4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix :mod:`codeop` regression: it no longer ignores :exc:`SyntaxWarning`.