Commit 3bcbedc9 authored by Serhiy Storchaka's avatar Serhiy Storchaka Committed by GitHub

bpo-34850: Emit a warning for "is" and "is not" with a literal. (GH-9642)

parent e55cf024
...@@ -442,6 +442,13 @@ Changes in Python behavior ...@@ -442,6 +442,13 @@ Changes in Python behavior
in the leftmost :keyword:`!for` clause). in the leftmost :keyword:`!for` clause).
(Contributed by Serhiy Storchaka in :issue:`10544`.) (Contributed by Serhiy Storchaka in :issue:`10544`.)
* The compiler now produces a :exc:`SyntaxWarning` when identity checks
(``is`` and ``is not``) are used with certain types of literals
(e.g. strings, ints). These can often work by accident in CPython,
but are not guaranteed by the language spec. The warning advises users
to use equality tests (``==`` and ``!=``) instead.
(Contributed by Serhiy Storchaka in :issue:`34850`.)
Changes in the Python API Changes in the Python API
------------------------- -------------------------
......
...@@ -1146,6 +1146,7 @@ class ASTValidatorTests(unittest.TestCase): ...@@ -1146,6 +1146,7 @@ class ASTValidatorTests(unittest.TestCase):
tests = [fn for fn in os.listdir(stdlib) if fn.endswith(".py")] tests = [fn for fn in os.listdir(stdlib) if fn.endswith(".py")]
tests.extend(["test/test_grammar.py", "test/test_unpack_ex.py"]) tests.extend(["test/test_grammar.py", "test/test_unpack_ex.py"])
for module in tests: for module in tests:
with self.subTest(module):
fn = os.path.join(stdlib, module) fn = os.path.join(stdlib, module)
with open(fn, "r", encoding="utf-8") as fp: with open(fn, "r", encoding="utf-8") as fp:
source = fp.read() source = fp.read()
......
...@@ -1226,11 +1226,33 @@ class GrammarTests(unittest.TestCase): ...@@ -1226,11 +1226,33 @@ class GrammarTests(unittest.TestCase):
if 1 > 1: pass if 1 > 1: pass
if 1 <= 1: pass if 1 <= 1: pass
if 1 >= 1: pass if 1 >= 1: pass
if 1 is 1: pass if x is x: pass
if 1 is not 1: pass if x is not x: pass
if 1 in (): pass if 1 in (): pass
if 1 not in (): pass if 1 not in (): pass
if 1 < 1 > 1 == 1 >= 1 <= 1 != 1 in 1 not in 1 is 1 is not 1: pass if 1 < 1 > 1 == 1 >= 1 <= 1 != 1 in 1 not in x is x is not x: pass
def test_comparison_is_literal(self):
def check(test, msg='"is" with a literal'):
with self.assertWarnsRegex(SyntaxWarning, msg):
compile(test, '<testcase>', 'exec')
with warnings.catch_warnings():
warnings.filterwarnings('error', category=SyntaxWarning)
with self.assertRaisesRegex(SyntaxError, msg):
compile(test, '<testcase>', 'exec')
check('x is 1')
check('x is "thing"')
check('1 is x')
check('x is y is 1')
check('x is not 1', '"is not" with a literal')
with warnings.catch_warnings():
warnings.filterwarnings('error', category=SyntaxWarning)
compile('x is None', '<testcase>', 'exec')
compile('x is False', '<testcase>', 'exec')
compile('x is True', '<testcase>', 'exec')
compile('x is ...', '<testcase>', 'exec')
def test_binary_mask_ops(self): def test_binary_mask_ops(self):
x = 1 & 1 x = 1 & 1
...@@ -1520,9 +1542,11 @@ class GrammarTests(unittest.TestCase): ...@@ -1520,9 +1542,11 @@ class GrammarTests(unittest.TestCase):
self.assertEqual(16 // (4 // 2), 8) self.assertEqual(16 // (4 // 2), 8)
self.assertEqual((16 // 4) // 2, 2) self.assertEqual((16 // 4) // 2, 2)
self.assertEqual(16 // 4 // 2, 2) self.assertEqual(16 // 4 // 2, 2)
self.assertTrue(False is (2 is 3)) x = 2
self.assertFalse((False is 2) is 3) y = 3
self.assertFalse(False is 2 is 3) self.assertTrue(False is (x is y))
self.assertFalse((False is x) is y)
self.assertFalse(False is x is y)
def test_matrix_mul(self): def test_matrix_mul(self):
# This is not intended to be a comprehensive test, rather just to be few # This is not intended to be a comprehensive test, rather just to be few
......
The compiler now produces a :exc:`SyntaxWarning` when identity checks
(``is`` and ``is not``) are used with certain types of literals
(e.g. strings, ints). These can often work by accident in CPython,
but are not guaranteed by the language spec. The warning advises users
to use equality tests (``==`` and ``!=``) instead.
...@@ -2284,6 +2284,47 @@ compiler_class(struct compiler *c, stmt_ty s) ...@@ -2284,6 +2284,47 @@ compiler_class(struct compiler *c, stmt_ty s)
return 1; return 1;
} }
/* Return 0 if the expression is a constant value except named singletons.
Return 1 otherwise. */
static int
check_is_arg(expr_ty e)
{
if (e->kind != Constant_kind) {
return 1;
}
PyObject *value = e->v.Constant.value;
return (value == Py_None
|| value == Py_False
|| value == Py_True
|| value == Py_Ellipsis);
}
/* Check operands of identity chacks ("is" and "is not").
Emit a warning if any operand is a constant except named singletons.
Return 0 on error.
*/
static int
check_compare(struct compiler *c, expr_ty e)
{
Py_ssize_t i, n;
int left = check_is_arg(e->v.Compare.left);
n = asdl_seq_LEN(e->v.Compare.ops);
for (i = 0; i < n; i++) {
cmpop_ty op = (cmpop_ty)asdl_seq_GET(e->v.Compare.ops, i);
int right = check_is_arg((expr_ty)asdl_seq_GET(e->v.Compare.comparators, i));
if (op == Is || op == IsNot) {
if (!right || !left) {
const char *msg = (op == Is)
? "\"is\" with a literal. Did you mean \"==\"?"
: "\"is not\" with a literal. Did you mean \"!=\"?";
return compiler_warn(c, msg);
}
}
left = right;
}
return 1;
}
static int static int
cmpop(cmpop_ty op) cmpop(cmpop_ty op)
{ {
...@@ -2363,6 +2404,9 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond) ...@@ -2363,6 +2404,9 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond)
return 1; return 1;
} }
case Compare_kind: { case Compare_kind: {
if (!check_compare(c, e)) {
return 0;
}
Py_ssize_t i, n = asdl_seq_LEN(e->v.Compare.ops) - 1; Py_ssize_t i, n = asdl_seq_LEN(e->v.Compare.ops) - 1;
if (n > 0) { if (n > 0) {
basicblock *cleanup = compiler_new_block(c); basicblock *cleanup = compiler_new_block(c);
...@@ -3670,6 +3714,9 @@ compiler_compare(struct compiler *c, expr_ty e) ...@@ -3670,6 +3714,9 @@ compiler_compare(struct compiler *c, expr_ty e)
{ {
Py_ssize_t i, n; Py_ssize_t i, n;
if (!check_compare(c, e)) {
return 0;
}
VISIT(c, expr, e->v.Compare.left); VISIT(c, expr, e->v.Compare.left);
assert(asdl_seq_LEN(e->v.Compare.ops) > 0); assert(asdl_seq_LEN(e->v.Compare.ops) > 0);
n = asdl_seq_LEN(e->v.Compare.ops) - 1; n = asdl_seq_LEN(e->v.Compare.ops) - 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment