[Python-checkins] bpo-34850: Emit a warning for "is" and "is not" with a literal. (GH-9642)
Serhiy Storchaka
webhook-mailer at python.org
Fri Jan 18 00:47:53 EST 2019
https://github.com/python/cpython/commit/3bcbedc9f1471d957a30a90f9d1251516b422416
commit: 3bcbedc9f1471d957a30a90f9d1251516b422416
branch: master
author: Serhiy Storchaka <storchaka at gmail.com>
committer: GitHub <noreply at github.com>
date: 2019-01-18T07:47:48+02:00
summary:
bpo-34850: Emit a warning for "is" and "is not" with a literal. (GH-9642)
files:
A Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst
M Doc/whatsnew/3.8.rst
M Lib/test/test_ast.py
M Lib/test/test_grammar.py
M Python/compile.c
diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst
index bf8d8f15434b..0360be604f56 100644
--- a/Doc/whatsnew/3.8.rst
+++ b/Doc/whatsnew/3.8.rst
@@ -442,6 +442,13 @@ Changes in Python behavior
in the leftmost :keyword:`!for` clause).
(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
-------------------------
diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py
index 2c8d8ab7e3fe..897e705a42ca 100644
--- a/Lib/test/test_ast.py
+++ b/Lib/test/test_ast.py
@@ -1146,11 +1146,12 @@ def test_stdlib_validates(self):
tests = [fn for fn in os.listdir(stdlib) if fn.endswith(".py")]
tests.extend(["test/test_grammar.py", "test/test_unpack_ex.py"])
for module in tests:
- fn = os.path.join(stdlib, module)
- with open(fn, "r", encoding="utf-8") as fp:
- source = fp.read()
- mod = ast.parse(source, fn)
- compile(mod, fn, "exec")
+ with self.subTest(module):
+ fn = os.path.join(stdlib, module)
+ with open(fn, "r", encoding="utf-8") as fp:
+ source = fp.read()
+ mod = ast.parse(source, fn)
+ compile(mod, fn, "exec")
class ConstantTests(unittest.TestCase):
diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py
index 3d8b1514f0cd..3ed19ff1cb04 100644
--- a/Lib/test/test_grammar.py
+++ b/Lib/test/test_grammar.py
@@ -1226,11 +1226,33 @@ def test_comparison(self):
if 1 > 1: pass
if 1 <= 1: pass
if 1 >= 1: pass
- if 1 is 1: pass
- if 1 is not 1: pass
+ if x is x: pass
+ if x is not x: pass
if 1 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):
x = 1 & 1
@@ -1520,9 +1542,11 @@ def test_paren_evaluation(self):
self.assertEqual(16 // (4 // 2), 8)
self.assertEqual((16 // 4) // 2, 2)
self.assertEqual(16 // 4 // 2, 2)
- self.assertTrue(False is (2 is 3))
- self.assertFalse((False is 2) is 3)
- self.assertFalse(False is 2 is 3)
+ x = 2
+ y = 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):
# This is not intended to be a comprehensive test, rather just to be few
diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst
new file mode 100644
index 000000000000..bc5d5d1d5808
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst
@@ -0,0 +1,5 @@
+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.
diff --git a/Python/compile.c b/Python/compile.c
index 45e78cb22cd8..5aebda0da4d1 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -2284,6 +2284,47 @@ compiler_class(struct compiler *c, stmt_ty s)
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
cmpop(cmpop_ty op)
{
@@ -2363,6 +2404,9 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond)
return 1;
}
case Compare_kind: {
+ if (!check_compare(c, e)) {
+ return 0;
+ }
Py_ssize_t i, n = asdl_seq_LEN(e->v.Compare.ops) - 1;
if (n > 0) {
basicblock *cleanup = compiler_new_block(c);
@@ -3670,6 +3714,9 @@ compiler_compare(struct compiler *c, expr_ty e)
{
Py_ssize_t i, n;
+ if (!check_compare(c, e)) {
+ return 0;
+ }
VISIT(c, expr, e->v.Compare.left);
assert(asdl_seq_LEN(e->v.Compare.ops) > 0);
n = asdl_seq_LEN(e->v.Compare.ops) - 1;
More information about the Python-checkins
mailing list