Skip to content

bpo-43497: Emit SyntaxWarnings for assertions with tuple constants. #24867

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

Merged
merged 5 commits into from
Mar 16, 2021
Merged
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
30 changes: 28 additions & 2 deletions Lib/test/test_grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ class GrammarTests(unittest.TestCase):

from test.support import check_syntax_error
from test.support.warnings_helper import check_syntax_warning
from test.support.warnings_helper import check_no_warnings

# single_input: NEWLINE | simple_stmt | compound_stmt NEWLINE
# XXX can't test in a script -- this rule is only used when interactive
Expand Down Expand Up @@ -1194,7 +1195,7 @@ def test_assert(self):

# these tests fail if python is run with -O, so check __debug__
@unittest.skipUnless(__debug__, "Won't work if __debug__ is False")
def testAssert2(self):
def test_assert_failures(self):
try:
assert 0, "msg"
except AssertionError as e:
Expand All @@ -1209,11 +1210,36 @@ def testAssert2(self):
else:
self.fail("AssertionError not raised by 'assert False'")

def test_assert_syntax_warnings(self):
# Ensure that we warn users if they provide a non-zero length tuple as
# the assertion test.
self.check_syntax_warning('assert(x, "msg")',
'assertion is always true')
self.check_syntax_warning('assert(False, "msg")',
'assertion is always true')
Copy link
Member

Choose a reason for hiding this comment

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

Could you also add to test that this doesn't warn on assert False, "msg".

These tests shouldn't be guarded by @unittest.skipUnless(__debug__, ...).
The warning should be issued regardless of __debug__.
See below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

self.check_syntax_warning('assert(False,)',
'assertion is always true')

with self.check_no_warnings(category=SyntaxWarning):
compile('assert x, "msg"', '<testcase>', 'exec')
compile('assert False, "msg"', '<testcase>', 'exec')

def test_assert_warning_promotes_to_syntax_error(self):
# If SyntaxWarning is configured to be an error, it actually raises a
# SyntaxError.
# https://bugs.python.org/issue35029
with warnings.catch_warnings():
warnings.simplefilter('error', SyntaxWarning)
compile('assert x, "msg"', '<testcase>', 'exec')
try:
compile('assert x, "msg" ', '<testcase>', 'exec')
except SyntaxError:
self.fail('SyntaxError incorrectly raised for \'assert x, "msg"\'')
with self.assertRaises(SyntaxError):
compile('assert(x, "msg")', '<testcase>', 'exec')
with self.assertRaises(SyntaxError):
compile('assert(False, "msg")', '<testcase>', 'exec')
with self.assertRaises(SyntaxError):
compile('assert(False,)', '<testcase>', 'exec')


### compound_stmt: if_stmt | while_stmt | for_stmt | try_stmt | funcdef | classdef
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Emit SyntaxWarnings for assertions with tuple constants, this is a regression introduced in python3.7
12 changes: 8 additions & 4 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -3355,17 +3355,21 @@ compiler_assert(struct compiler *c, stmt_ty s)
{
basicblock *end;

if (c->c_optimize)
return 1;
if (s->v.Assert.test->kind == Tuple_kind &&
asdl_seq_LEN(s->v.Assert.test->v.Tuple.elts) > 0)
/* Always emit a warning if the test is a non-zero length tuple */
if ((s->v.Assert.test->kind == Tuple_kind &&
asdl_seq_LEN(s->v.Assert.test->v.Tuple.elts) > 0) ||
(s->v.Assert.test->kind == Constant_kind &&
PyTuple_Check(s->v.Assert.test->v.Constant.value) &&
PyTuple_Size(s->v.Assert.test->v.Constant.value) > 0))
{
if (!compiler_warn(c, "assertion is always true, "
"perhaps remove parentheses?"))
{
return 0;
}
}
if (c->c_optimize)
return 1;
end = compiler_new_block(c);
if (end == NULL)
return 0;
Expand Down