From 5247c04513816c56b1bf57fe381631ccf8fa662f Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Wed, 12 Apr 2023 17:51:35 -0700 Subject: [PATCH 1/9] gh-103492: Clarify SyntaxWarning with literal comparison --- Lib/test/test_grammar.py | 20 +++++++++++++------- Python/compile.c | 12 ++++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index ced9000f75f2e5..7e7be5c4d30387 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -237,7 +237,7 @@ def check(test, error=False): check(f"{num}spam", error=True) with warnings.catch_warnings(): - warnings.filterwarnings('ignore', '"is" with a literal', + warnings.filterwarnings('ignore', '"is" with int literal', SyntaxWarning) with self.assertWarnsRegex(SyntaxWarning, r'invalid \w+ literal'): @@ -1467,14 +1467,16 @@ def test_comparison(self): 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'): + def check(test, msg): self.check_syntax_warning(test, msg) - 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') + check('x is 1', '"is" with int literal') + check('x is "thing"', '"is" with str literal') + check('1 is x', '"is" with int literal') + check('x is y is 1', '"is" with int literal') + check('x is not 1', '"is not" with int literal') + check('x is not (1, 2)', '"is not" with tuple literal') + check('(1, 2) is not x', '"is not" with tuple literal') with warnings.catch_warnings(): warnings.simplefilter('error', SyntaxWarning) @@ -1482,6 +1484,10 @@ def check(test, msg='"is" with a literal'): compile('x is False', '', 'exec') compile('x is True', '', 'exec') compile('x is ...', '', 'exec') + compile('None is x', '', 'exec') + compile('False is x', '', 'exec') + compile('True is x', '', 'exec') + compile('... is x', '', 'exec') def test_warn_missed_comma(self): def check(test): diff --git a/Python/compile.c b/Python/compile.c index fed9ae7066e4f0..bc42ecc0e53725 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2345,6 +2345,8 @@ check_is_arg(expr_ty e) || value == Py_Ellipsis); } +static PyTypeObject * infer_type(expr_ty e); + /* Check operands of identity checks ("is" and "is not"). Emit a warning if any operand is a constant except named singletons. */ @@ -2356,13 +2358,15 @@ check_compare(struct compiler *c, expr_ty e) 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); - bool right = check_is_arg((expr_ty)asdl_seq_GET(e->v.Compare.comparators, i)); + expr_ty right_expr = (expr_ty)asdl_seq_GET(e->v.Compare.comparators, i); + bool right = check_is_arg(right_expr); 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, LOC(e), msg); + ? "\"is\" with %.200s literal. Did you mean \"==\"?" + : "\"is not\" with %.200s literal. Did you mean \"!=\"?"; + expr_ty literal = !left ? e->v.Compare.left : right_expr; + return compiler_warn(c, LOC(e), msg, infer_type(literal)->tp_name); } } left = right; From 12ce76c3c476663d43dc0056cd4efe59c305518c Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Wed, 12 Apr 2023 17:55:58 -0700 Subject: [PATCH 2/9] . --- Lib/test/test_grammar.py | 3 +++ Python/compile.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index 7e7be5c4d30387..8247c006769259 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -1478,6 +1478,9 @@ def check(test, msg): check('x is not (1, 2)', '"is not" with tuple literal') check('(1, 2) is not x', '"is not" with tuple literal') + check('None is 1', '"is" with int literal') + check('1 is None', '"is" with int literal') + with warnings.catch_warnings(): warnings.simplefilter('error', SyntaxWarning) compile('x is None', '', 'exec') diff --git a/Python/compile.c b/Python/compile.c index bc42ecc0e53725..0572ac373ada78 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2345,8 +2345,6 @@ check_is_arg(expr_ty e) || value == Py_Ellipsis); } -static PyTypeObject * infer_type(expr_ty e); - /* Check operands of identity checks ("is" and "is not"). Emit a warning if any operand is a constant except named singletons. */ @@ -2366,7 +2364,9 @@ check_compare(struct compiler *c, expr_ty e) ? "\"is\" with %.200s literal. Did you mean \"==\"?" : "\"is not\" with %.200s literal. Did you mean \"!=\"?"; expr_ty literal = !left ? e->v.Compare.left : right_expr; - return compiler_warn(c, LOC(e), msg, infer_type(literal)->tp_name); + return compiler_warn( + c, LOC(e), msg, Py_TYPE(literal->v.Constant.value)->tp_name + ); } } left = right; From bc6d13f82f7ebedcb400a3b12217033e0451ef12 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 13 Apr 2023 00:58:57 +0000 Subject: [PATCH 3/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst new file mode 100644 index 00000000000000..4161252304f423 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst @@ -0,0 +1 @@ +Clarify :exc:`SyntaxWarning` with literal ``is`` comparison by specifying which literal is problematic, since comparisons using ``is`` with e.g. None and bool literals is idiomatic. From d0f2ea30fd50234078059b017f4cd10d5a8631de Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Wed, 12 Apr 2023 18:48:34 -0700 Subject: [PATCH 4/9] . --- Lib/test/test_codeop.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_codeop.py b/Lib/test/test_codeop.py index 6966c2ffd811b8..919442803d9c56 100644 --- a/Lib/test/test_codeop.py +++ b/Lib/test/test_codeop.py @@ -277,7 +277,7 @@ def test_filename(self): def test_warning(self): # Test that the warning is only returned once. with warnings_helper.check_warnings( - ('"is" with a literal', SyntaxWarning), + ('"is" with str literal', SyntaxWarning), ("invalid escape sequence", SyntaxWarning), ) as w: compile_command(r"'\e' is 0") From 9a13700f38bdd53622ccf5ec72749bb7acc4e368 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Wed, 12 Apr 2023 18:49:44 -0700 Subject: [PATCH 5/9] Update Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst --- .../2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst index 4161252304f423..929650968173e7 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-13-00-58-55.gh-issue-103492.P4k0Ay.rst @@ -1 +1 @@ -Clarify :exc:`SyntaxWarning` with literal ``is`` comparison by specifying which literal is problematic, since comparisons using ``is`` with e.g. None and bool literals is idiomatic. +Clarify :exc:`SyntaxWarning` with literal ``is`` comparison by specifying which literal is problematic, since comparisons using ``is`` with e.g. None and bool literals are idiomatic. From 30b29bb59bdcdb5ca0eba2178a554adf420d3fe5 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Wed, 19 Apr 2023 12:11:30 -0600 Subject: [PATCH 6/9] single quote types --- Lib/test/test_codeop.py | 2 +- Lib/test/test_grammar.py | 20 ++++++++++---------- Python/compile.c | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_codeop.py b/Lib/test/test_codeop.py index 919442803d9c56..e3c382266fa058 100644 --- a/Lib/test/test_codeop.py +++ b/Lib/test/test_codeop.py @@ -277,7 +277,7 @@ def test_filename(self): def test_warning(self): # Test that the warning is only returned once. with warnings_helper.check_warnings( - ('"is" with str literal', SyntaxWarning), + ('"is" with \'str\' literal', SyntaxWarning), ("invalid escape sequence", SyntaxWarning), ) as w: compile_command(r"'\e' is 0") diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index 8247c006769259..6617b793bce2f4 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -1470,16 +1470,16 @@ def test_comparison_is_literal(self): def check(test, msg): self.check_syntax_warning(test, msg) - check('x is 1', '"is" with int literal') - check('x is "thing"', '"is" with str literal') - check('1 is x', '"is" with int literal') - check('x is y is 1', '"is" with int literal') - check('x is not 1', '"is not" with int literal') - check('x is not (1, 2)', '"is not" with tuple literal') - check('(1, 2) is not x', '"is not" with tuple literal') - - check('None is 1', '"is" with int literal') - check('1 is None', '"is" with int literal') + check('x is 1', '"is" with \'int\' literal') + check('x is "thing"', '"is" with \'str\' literal') + check('1 is x', '"is" with \'int\' literal') + check('x is y is 1', '"is" with \'int\' literal') + check('x is not 1', '"is not" with \'int\' literal') + check('x is not (1, 2)', '"is not" with \'tuple\' literal') + check('(1, 2) is not x', '"is not" with \'tuple\' literal') + + check('None is 1', '"is" with \'int\' literal') + check('1 is None', '"is" with \'int\' literal') with warnings.catch_warnings(): warnings.simplefilter('error', SyntaxWarning) diff --git a/Python/compile.c b/Python/compile.c index c53a64f73fc74a..9d5d70c52b2afb 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2285,8 +2285,8 @@ check_compare(struct compiler *c, expr_ty e) if (op == Is || op == IsNot) { if (!right || !left) { const char *msg = (op == Is) - ? "\"is\" with %.200s literal. Did you mean \"==\"?" - : "\"is not\" with %.200s literal. Did you mean \"!=\"?"; + ? "\"is\" with '%.200s' literal. Did you mean \"==\"?" + : "\"is not\" with '%.200s' literal. Did you mean \"!=\"?"; expr_ty literal = !left ? e->v.Compare.left : right_expr; return compiler_warn( c, LOC(e), msg, Py_TYPE(literal->v.Constant.value)->tp_name From 8f16b1a6fa9fa7b7f307d8eb6cbac49cb8dc653f Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 20 Apr 2023 11:18:04 -0600 Subject: [PATCH 7/9] simplify warning code (tested with and without -W error) --- Lib/test/test_grammar.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index 6617b793bce2f4..d27dcba11352dd 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -236,12 +236,9 @@ def check(test, error=False): check(f"[{num}for x in ()]") check(f"{num}spam", error=True) + with self.assertWarnsRegex(SyntaxWarning, r'invalid \w+ literal'): + compile(f"{num}is x", "", "eval") with warnings.catch_warnings(): - warnings.filterwarnings('ignore', '"is" with int literal', - SyntaxWarning) - with self.assertWarnsRegex(SyntaxWarning, - r'invalid \w+ literal'): - compile(f"{num}is x", "", "eval") warnings.simplefilter('error', SyntaxWarning) with self.assertRaisesRegex(SyntaxError, r'invalid \w+ literal'): From b7b4f1a6b8e363b5b3d6c90957820255b9df8630 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Mon, 24 Apr 2023 14:13:43 -0600 Subject: [PATCH 8/9] fix bug irit mentions, add test --- Lib/test/test_grammar.py | 3 +++ Python/compile.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index d27dcba11352dd..ee105a3de17f8a 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -1478,6 +1478,9 @@ def check(test, msg): check('None is 1', '"is" with \'int\' literal') check('1 is None', '"is" with \'int\' literal') + check('x == 3 is y', '"is" with \'int\' literal') + check('x == "thing" is y', '"is" with \'str\' literal') + with warnings.catch_warnings(): warnings.simplefilter('error', SyntaxWarning) compile('x is None', '', 'exec') diff --git a/Python/compile.c b/Python/compile.c index 9d5d70c52b2afb..ac7818f142f289 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2277,6 +2277,7 @@ check_compare(struct compiler *c, expr_ty e) { Py_ssize_t i, n; bool left = check_is_arg(e->v.Compare.left); + expr_ty left_expr = 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); @@ -2287,13 +2288,14 @@ check_compare(struct compiler *c, expr_ty e) const char *msg = (op == Is) ? "\"is\" with '%.200s' literal. Did you mean \"==\"?" : "\"is not\" with '%.200s' literal. Did you mean \"!=\"?"; - expr_ty literal = !left ? e->v.Compare.left : right_expr; + expr_ty literal = !left ? left_expr : right_expr; return compiler_warn( c, LOC(e), msg, Py_TYPE(literal->v.Constant.value)->tp_name ); } } left = right; + left_expr = right_expr; } return SUCCESS; } From 20d0554986b15caab29797fef6df8f55f6b50abb Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Mon, 24 Apr 2023 14:16:49 -0600 Subject: [PATCH 9/9] use infer_type --- Python/compile.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index ac7818f142f289..ebaad6033edbf9 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2269,6 +2269,8 @@ check_is_arg(expr_ty e) || value == Py_Ellipsis); } +static PyTypeObject * infer_type(expr_ty e); + /* Check operands of identity checks ("is" and "is not"). Emit a warning if any operand is a constant except named singletons. */ @@ -2290,7 +2292,7 @@ check_compare(struct compiler *c, expr_ty e) : "\"is not\" with '%.200s' literal. Did you mean \"!=\"?"; expr_ty literal = !left ? left_expr : right_expr; return compiler_warn( - c, LOC(e), msg, Py_TYPE(literal->v.Constant.value)->tp_name + c, LOC(e), msg, infer_type(literal)->tp_name ); } }