gh-103492: Clarify SyntaxWarning with literal comparison (#103493)

hauntsaninja · web-flow · commit ae25855045e8 · 2023-04-24T15:42:57.000-06:00
diff --git 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")
diff --git 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", "<testcase>", "eval")
             with warnings.catch_warnings():
-                warnings.filterwarnings('ignore', '"is" with a literal',
-                                        SyntaxWarning)
-                with self.assertWarnsRegex(SyntaxWarning,
-                            r'invalid \w+ literal'):
-                    compile(f"{num}is x", "<testcase>", "eval")
                 warnings.simplefilter('error', SyntaxWarning)
                 with self.assertRaisesRegex(SyntaxError,
                             r'invalid \w+ literal'):
@@ -1467,21 +1464,33 @@ 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')
+
+        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', '<testcase>', 'exec')
             compile('x is False', '<testcase>', 'exec')
             compile('x is True', '<testcase>', 'exec')
             compile('x is ...', '<testcase>', 'exec')
+            compile('None is x', '<testcase>', 'exec')
+            compile('False is x', '<testcase>', 'exec')
+            compile('True is x', '<testcase>', 'exec')
+            compile('... is x', '<testcase>', 'exec')
 
     def test_warn_missed_comma(self):
         def check(test):
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
@@ -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 are idiomatic.
diff --git 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.
  */
@@ -2277,19 +2279,25 @@ 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);
-        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 ? left_expr : right_expr;
+                return compiler_warn(
+                    c, LOC(e), msg, infer_type(literal)->tp_name
+                );
             }
         }
         left = right;
+        left_expr = right_expr;
     }
     return SUCCESS;
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +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 are idiomatic.