Skip to content

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

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
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
7 changes: 7 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,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
-------------------------
Expand Down
11 changes: 6 additions & 5 deletions Lib/test/test_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -1098,11 +1098,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):
Expand Down
36 changes: 30 additions & 6 deletions Lib/test/test_grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
47 changes: 47 additions & 0 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,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
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't special case True and False, those are not normally identity checked. I don't doubt that someone may have create APIs returning multiple possible types that requires this to differentiate between "real" values and bools... but yuck.

|| 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)
{
Expand Down Expand Up @@ -2235,6 +2276,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);
Expand Down Expand Up @@ -3542,6 +3586,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;
Expand Down