bpo-20028: Improve error message of csv.Dialect when initializing (GH-28705)

3 files changed, 70 insertions, 9 deletions

diff --git a/Lib/test/test_csv.py b/Lib/test/test_csv.py
index 09e72a7..6e5dfc6 100644
--- a/Lib/test/test_csv.py
+++ b/Lib/test/test_csv.py
@@ -897,7 +897,7 @@ class TestDialectValidity(unittest.TestCase):
         with self.assertRaises(csv.Error) as cm:
             mydialect()
         self.assertEqual(str(cm.exception),
-                         '"quotechar" must be string, not int')
+                         '"quotechar" must be string or None, not int')
 
     def test_delimiter(self):
         class mydialect(csv.Dialect):
@@ -934,6 +934,35 @@ class TestDialectValidity(unittest.TestCase):
         self.assertEqual(str(cm.exception),
                          '"delimiter" must be string, not int')
 
+        mydialect.delimiter = None
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"delimiter" must be string, not NoneType')
+
+    def test_escapechar(self):
+        class mydialect(csv.Dialect):
+            delimiter = ";"
+            escapechar = '\\'
+            doublequote = False
+            skipinitialspace = True
+            lineterminator = '\r\n'
+            quoting = csv.QUOTE_NONE
+        d = mydialect()
+        self.assertEqual(d.escapechar, "\\")
+
+        mydialect.escapechar = "**"
+        with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'):
+            mydialect()
+
+        mydialect.escapechar = b"*"
+        with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not bytes'):
+            mydialect()
+
+        mydialect.escapechar = 4
+        with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not int'):
+            mydialect()
+
     def test_lineterminator(self):
         class mydialect(csv.Dialect):
             delimiter = ";"
diff --git a/Misc/NEWS.d/next/Library/2021-10-03-21-14-37.bpo-20028.zBA4RK.rst b/Misc/NEWS.d/next/Library/2021-10-03-21-14-37.bpo-20028.zBA4RK.rst
new file mode 100644
index 0000000..e756121
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-10-03-21-14-37.bpo-20028.zBA4RK.rst
@@ -0,0 +1,2 @@
+Improve error message of :class:`csv.Dialect` when initializing.
+Patch by Vajrasky Kok and Dong-hee Na.
diff --git a/Modules/_csv.c b/Modules/_csv.c
index 3729d2b..cfdfbce 100644
--- a/Modules/_csv.c
+++ b/Modules/_csv.c
@@ -229,21 +229,21 @@ _set_int(const char *name, int *target, PyObject *src, int dflt)
 }
 
 static int
-_set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
+_set_char_or_none(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
 {
-    if (src == NULL)
+    if (src == NULL) {
         *target = dflt;
+    }
     else {
         *target = '\0';
         if (src != Py_None) {
-            Py_ssize_t len;
             if (!PyUnicode_Check(src)) {
                 PyErr_Format(PyExc_TypeError,
-                    "\"%s\" must be string, not %.200s", name,
+                    "\"%s\" must be string or None, not %.200s", name,
                     Py_TYPE(src)->tp_name);
                 return -1;
             }
-            len = PyUnicode_GetLength(src);
+            Py_ssize_t len = PyUnicode_GetLength(src);
             if (len > 1) {
                 PyErr_Format(PyExc_TypeError,
                     "\"%s\" must be a 1-character string",
@@ -251,8 +251,38 @@ _set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
                 return -1;
             }
             /* PyUnicode_READY() is called in PyUnicode_GetLength() */
-            if (len > 0)
+            else {
                 *target = PyUnicode_READ_CHAR(src, 0);
+            }
+        }
+    }
+    return 0;
+}
+
+static int
+_set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
+{
+    if (src == NULL) {
+        *target = dflt;
+    }
+    else {
+        *target = '\0';
+        if (!PyUnicode_Check(src)) {
+            PyErr_Format(PyExc_TypeError,
+                         "\"%s\" must be string, not %.200s", name,
+                         Py_TYPE(src)->tp_name);
+                return -1;
+        }
+        Py_ssize_t len = PyUnicode_GetLength(src);
+        if (len > 1) {
+            PyErr_Format(PyExc_TypeError,
+                         "\"%s\" must be a 1-character string",
+                         name);
+            return -1;
+        }
+        /* PyUnicode_READY() is called in PyUnicode_GetLength() */
+        else {
+            *target = PyUnicode_READ_CHAR(src, 0);
         }
     }
     return 0;
@@ -445,9 +475,9 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
         goto err
     DIASET(_set_char, "delimiter", &self->delimiter, delimiter, ',');
     DIASET(_set_bool, "doublequote", &self->doublequote, doublequote, true);
-    DIASET(_set_char, "escapechar", &self->escapechar, escapechar, 0);
+    DIASET(_set_char_or_none, "escapechar", &self->escapechar, escapechar, 0);
     DIASET(_set_str, "lineterminator", &self->lineterminator, lineterminator, "\r\n");
-    DIASET(_set_char, "quotechar", &self->quotechar, quotechar, '"');
+    DIASET(_set_char_or_none, "quotechar", &self->quotechar, quotechar, '"');
     DIASET(_set_int, "quoting", &self->quoting, quoting, QUOTE_MINIMAL);
     DIASET(_set_bool, "skipinitialspace", &self->skipinitialspace, skipinitialspace, false);
     DIASET(_set_bool, "strict", &self->strict, strict, false);