bpo-15248: Emit a compiler warning when missed a comma before tuple or list. (GH-11757)

4 files changed, 239 insertions, 6 deletions

diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst
index 740c608..dbc3787 100644
--- a/Doc/whatsnew/3.8.rst
+++ b/Doc/whatsnew/3.8.rst
@@ -115,6 +115,16 @@ Other Language Changes
   a :exc:`SyntaxWarning` instead.
   (Contributed by Serhiy Storchaka in :issue:`32912`.)
 
+* The compiler now produces a :exc:`SyntaxWarning` in some cases when a comma
+  is missed before tuple or list.  For example::
+
+     data = [
+         (1, 2, 3) # oops, missing comma!
+         (4, 5, 6)
+     ]
+
+  (Contributed by Serhiy Storchaka in :issue:`15248`.)
+
 * Arithmetic operations between subclasses of :class:`datetime.date` or
   :class:`datetime.datetime` and :class:`datetime.timedelta` objects now return
   an instance of the subclass, rather than the base class. This also affects
diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py
index 74590eb..2ee38f0 100644
--- a/Lib/test/test_grammar.py
+++ b/Lib/test/test_grammar.py
@@ -1263,6 +1263,93 @@ class GrammarTests(unittest.TestCase):
             compile('x is True', '<testcase>', 'exec')
             compile('x is ...', '<testcase>', 'exec')
 
+    def test_warn_missed_comma(self):
+        def check(test):
+            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')
+
+        msg=r'is not callable; perhaps you missed a comma\?'
+        check('[(1, 2) (3, 4)]')
+        check('[(x, y) (3, 4)]')
+        check('[[1, 2] (3, 4)]')
+        check('[{1, 2} (3, 4)]')
+        check('[{1: 2} (3, 4)]')
+        check('[[i for i in range(5)] (3, 4)]')
+        check('[{i for i in range(5)} (3, 4)]')
+        check('[(i for i in range(5)) (3, 4)]')
+        check('[{i: i for i in range(5)} (3, 4)]')
+        check('[f"{x}" (3, 4)]')
+        check('[f"x={x}" (3, 4)]')
+        check('["abc" (3, 4)]')
+        check('[b"abc" (3, 4)]')
+        check('[123 (3, 4)]')
+        check('[12.3 (3, 4)]')
+        check('[12.3j (3, 4)]')
+        check('[None (3, 4)]')
+        check('[True (3, 4)]')
+        check('[... (3, 4)]')
+
+        msg=r'is not subscriptable; perhaps you missed a comma\?'
+        check('[{1, 2} [i, j]]')
+        check('[{i for i in range(5)} [i, j]]')
+        check('[(i for i in range(5)) [i, j]]')
+        check('[(lambda x, y: x) [i, j]]')
+        check('[123 [i, j]]')
+        check('[12.3 [i, j]]')
+        check('[12.3j [i, j]]')
+        check('[None [i, j]]')
+        check('[True [i, j]]')
+        check('[... [i, j]]')
+
+        msg=r'indices must be integers or slices, not tuple; perhaps you missed a comma\?'
+        check('[(1, 2) [i, j]]')
+        check('[(x, y) [i, j]]')
+        check('[[1, 2] [i, j]]')
+        check('[[i for i in range(5)] [i, j]]')
+        check('[f"{x}" [i, j]]')
+        check('[f"x={x}" [i, j]]')
+        check('["abc" [i, j]]')
+        check('[b"abc" [i, j]]')
+
+        msg=r'indices must be integers or slices, not tuple;'
+        check('[[1, 2] [3, 4]]')
+        msg=r'indices must be integers or slices, not list;'
+        check('[[1, 2] [[3, 4]]]')
+        check('[[1, 2] [[i for i in range(5)]]]')
+        msg=r'indices must be integers or slices, not set;'
+        check('[[1, 2] [{3, 4}]]')
+        check('[[1, 2] [{i for i in range(5)}]]')
+        msg=r'indices must be integers or slices, not dict;'
+        check('[[1, 2] [{3: 4}]]')
+        check('[[1, 2] [{i: i for i in range(5)}]]')
+        msg=r'indices must be integers or slices, not generator;'
+        check('[[1, 2] [(i for i in range(5))]]')
+        msg=r'indices must be integers or slices, not function;'
+        check('[[1, 2] [(lambda x, y: x)]]')
+        msg=r'indices must be integers or slices, not str;'
+        check('[[1, 2] [f"{x}"]]')
+        check('[[1, 2] [f"x={x}"]]')
+        check('[[1, 2] ["abc"]]')
+        msg=r'indices must be integers or slices, not'
+        check('[[1, 2] [b"abc"]]')
+        check('[[1, 2] [12.3]]')
+        check('[[1, 2] [12.3j]]')
+        check('[[1, 2] [None]]')
+        check('[[1, 2] [...]]')
+
+        with warnings.catch_warnings():
+            warnings.filterwarnings('error', category=SyntaxWarning)
+            compile('[(lambda x, y: x) (3, 4)]', '<testcase>', 'exec')
+            compile('[[1, 2] [i]]', '<testcase>', 'exec')
+            compile('[[1, 2] [0]]', '<testcase>', 'exec')
+            compile('[[1, 2] [True]]', '<testcase>', 'exec')
+            compile('[[1, 2] [1:2]]', '<testcase>', 'exec')
+            compile('[{(1, 2): 3} [i, j]]', '<testcase>', 'exec')
+
     def test_binary_mask_ops(self):
         x = 1 & 1
         x = 1 ^ 1
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-02-04-21-10-17.bpo-15248.2sXSZZ.rst b/Misc/NEWS.d/next/Core and Builtins/2019-02-04-21-10-17.bpo-15248.2sXSZZ.rst
new file mode 100644
index 0000000..e938aaa
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-02-04-21-10-17.bpo-15248.2sXSZZ.rst
@@ -0,0 +1,2 @@
+The compiler emits now syntax warnings in the case when a comma is likely
+missed before tuple or list.
diff --git a/Python/compile.c b/Python/compile.c
index eb2c302..7565d3c 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -175,7 +175,7 @@ static int compiler_addop(struct compiler *, int);
 static int compiler_addop_i(struct compiler *, int, Py_ssize_t);
 static int compiler_addop_j(struct compiler *, int, basicblock *, int);
 static int compiler_error(struct compiler *, const char *);
-static int compiler_warn(struct compiler *, const char *);
+static int compiler_warn(struct compiler *, const char *, ...);
 static int compiler_nameop(struct compiler *, identifier, expr_context_ty);
 
 static PyCodeObject *compiler_mod(struct compiler *, mod_ty);
@@ -3766,6 +3766,122 @@ compiler_compare(struct compiler *c, expr_ty e)
     return 1;
 }
 
+static PyTypeObject *
+infer_type(expr_ty e)
+{
+    switch (e->kind) {
+    case Tuple_kind:
+        return &PyTuple_Type;
+    case List_kind:
+    case ListComp_kind:
+        return &PyList_Type;
+    case Dict_kind:
+    case DictComp_kind:
+        return &PyDict_Type;
+    case Set_kind:
+    case SetComp_kind:
+        return &PySet_Type;
+    case GeneratorExp_kind:
+        return &PyGen_Type;
+    case Lambda_kind:
+        return &PyFunction_Type;
+    case JoinedStr_kind:
+    case FormattedValue_kind:
+        return &PyUnicode_Type;
+    case Constant_kind:
+        return e->v.Constant.value->ob_type;
+    default:
+        return NULL;
+    }
+}
+
+static int
+check_caller(struct compiler *c, expr_ty e)
+{
+    switch (e->kind) {
+    case Constant_kind:
+    case Tuple_kind:
+    case List_kind:
+    case ListComp_kind:
+    case Dict_kind:
+    case DictComp_kind:
+    case Set_kind:
+    case SetComp_kind:
+    case GeneratorExp_kind:
+    case JoinedStr_kind:
+    case FormattedValue_kind:
+        return compiler_warn(c, "'%.200s' object is not callable; "
+                                "perhaps you missed a comma?",
+                                infer_type(e)->tp_name);
+    default:
+        return 1;
+    }
+}
+
+static int
+check_subscripter(struct compiler *c, expr_ty e)
+{
+    PyObject *v;
+
+    switch (e->kind) {
+    case Constant_kind:
+        v = e->v.Constant.value;
+        if (!(v == Py_None || v == Py_Ellipsis ||
+              PyLong_Check(v) || PyFloat_Check(v) || PyComplex_Check(v) ||
+              PyAnySet_Check(v)))
+        {
+            return 1;
+        }
+        /* fall through */
+    case Set_kind:
+    case SetComp_kind:
+    case GeneratorExp_kind:
+    case Lambda_kind:
+        return compiler_warn(c, "'%.200s' object is not subscriptable; "
+                                "perhaps you missed a comma?",
+                                infer_type(e)->tp_name);
+    default:
+        return 1;
+    }
+}
+
+static int
+check_index(struct compiler *c, expr_ty e, slice_ty s)
+{
+    PyObject *v;
+
+    if (s->kind != Index_kind) {
+        return 1;
+    }
+    PyTypeObject *index_type = infer_type(s->v.Index.value);
+    if (index_type == NULL
+        || PyType_FastSubclass(index_type, Py_TPFLAGS_LONG_SUBCLASS)
+        || index_type == &PySlice_Type) {
+        return 1;
+    }
+
+    switch (e->kind) {
+    case Constant_kind:
+        v = e->v.Constant.value;
+        if (!(PyUnicode_Check(v) || PyBytes_Check(v) || PyTuple_Check(v))) {
+            return 1;
+        }
+        /* fall through */
+    case Tuple_kind:
+    case List_kind:
+    case ListComp_kind:
+    case JoinedStr_kind:
+    case FormattedValue_kind:
+        return compiler_warn(c, "%.200s indices must be integers or slices, "
+                                "not %.200s; "
+                                "perhaps you missed a comma?",
+                                infer_type(e)->tp_name,
+                                index_type->tp_name);
+    default:
+        return 1;
+    }
+}
+
 static int
 maybe_optimize_method_call(struct compiler *c, expr_ty e)
 {
@@ -3801,7 +3917,9 @@ compiler_call(struct compiler *c, expr_ty e)
 {
     if (maybe_optimize_method_call(c, e) > 0)
         return 1;
-
+    if (!check_caller(c, e->v.Call.func)) {
+        return 0;
+    }
     VISIT(c, expr, e->v.Call.func);
     return compiler_call_helper(c, 0,
                                 e->v.Call.args,
@@ -4694,6 +4812,12 @@ compiler_visit_expr1(struct compiler *c, expr_ty e)
             VISIT_SLICE(c, e->v.Subscript.slice, AugLoad);
             break;
         case Load:
+            if (!check_subscripter(c, e->v.Subscript.value)) {
+                return 0;
+            }
+            if (!check_index(c, e->v.Subscript.value, e->v.Subscript.slice)) {
+                return 0;
+            }
             VISIT(c, expr, e->v.Subscript.value);
             VISIT_SLICE(c, e->v.Subscript.slice, Load);
             break;
@@ -4987,20 +5111,30 @@ compiler_error(struct compiler *c, const char *errstr)
    and returns 0.
 */
 static int
-compiler_warn(struct compiler *c, const char *errstr)
+compiler_warn(struct compiler *c, const char *format, ...)
 {
-    PyObject *msg = PyUnicode_FromString(errstr);
+    va_list vargs;
+#ifdef HAVE_STDARG_PROTOTYPES
+    va_start(vargs, format);
+#else
+    va_start(vargs);
+#endif
+    PyObject *msg = PyUnicode_FromFormatV(format, vargs);
+    va_end(vargs);
     if (msg == NULL) {
         return 0;
     }
     if (PyErr_WarnExplicitObject(PyExc_SyntaxWarning, msg, c->c_filename,
                                  c->u->u_lineno, NULL, NULL) < 0)
     {
-        Py_DECREF(msg);
         if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) {
+            /* Replace the SyntaxWarning exception with a SyntaxError
+               to get a more accurate error report */
             PyErr_Clear();
-            return compiler_error(c, errstr);
+            assert(PyUnicode_AsUTF8(msg) != NULL);
+            compiler_error(c, PyUnicode_AsUTF8(msg));
         }
+        Py_DECREF(msg);
         return 0;
     }
     Py_DECREF(msg);