gh-132813: Improve error messages for incorrect types and values of csv.Dialog attributes (GH-133241)

Make them similar to PyArg_Parse error messages, mention None as
a possible value, show a wrong type and the string length.
This commit is contained in:
Serhiy Storchaka 2025-06-02 23:35:41 +03:00 committed by GitHub
parent e814f43f2c
commit df98a47a61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 69 additions and 54 deletions

View File

@ -1122,19 +1122,22 @@ class TestDialectValidity(unittest.TestCase):
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"quotechar" must be a 1-character string') '"quotechar" must be a unicode character or None, '
'not a string of length 0')
mydialect.quotechar = "''" mydialect.quotechar = "''"
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"quotechar" must be a 1-character string') '"quotechar" must be a unicode character or None, '
'not a string of length 2')
mydialect.quotechar = 4 mydialect.quotechar = 4
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"quotechar" must be string or None, not int') '"quotechar" must be a unicode character or None, '
'not int')
def test_delimiter(self): def test_delimiter(self):
class mydialect(csv.Dialect): class mydialect(csv.Dialect):
@ -1151,31 +1154,32 @@ class TestDialectValidity(unittest.TestCase):
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"delimiter" must be a 1-character string') '"delimiter" must be a unicode character, '
'not a string of length 3')
mydialect.delimiter = "" mydialect.delimiter = ""
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"delimiter" must be a 1-character string') '"delimiter" must be a unicode character, not a string of length 0')
mydialect.delimiter = b"," mydialect.delimiter = b","
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"delimiter" must be string, not bytes') '"delimiter" must be a unicode character, not bytes')
mydialect.delimiter = 4 mydialect.delimiter = 4
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"delimiter" must be string, not int') '"delimiter" must be a unicode character, not int')
mydialect.delimiter = None mydialect.delimiter = None
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"delimiter" must be string, not NoneType') '"delimiter" must be a unicode character, not NoneType')
def test_escapechar(self): def test_escapechar(self):
class mydialect(csv.Dialect): class mydialect(csv.Dialect):
@ -1189,20 +1193,32 @@ class TestDialectValidity(unittest.TestCase):
self.assertEqual(d.escapechar, "\\") self.assertEqual(d.escapechar, "\\")
mydialect.escapechar = "" mydialect.escapechar = ""
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'): with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception),
'"escapechar" must be a unicode character or None, '
'not a string of length 0')
mydialect.escapechar = "**" mydialect.escapechar = "**"
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'): with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception),
'"escapechar" must be a unicode character or None, '
'not a string of length 2')
mydialect.escapechar = b"*" mydialect.escapechar = b"*"
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not bytes'): with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception),
'"escapechar" must be a unicode character or None, '
'not bytes')
mydialect.escapechar = 4 mydialect.escapechar = 4
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not int'): with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception),
'"escapechar" must be a unicode character or None, '
'not int')
def test_lineterminator(self): def test_lineterminator(self):
class mydialect(csv.Dialect): class mydialect(csv.Dialect):
@ -1223,7 +1239,13 @@ class TestDialectValidity(unittest.TestCase):
with self.assertRaises(csv.Error) as cm: with self.assertRaises(csv.Error) as cm:
mydialect() mydialect()
self.assertEqual(str(cm.exception), self.assertEqual(str(cm.exception),
'"lineterminator" must be a string') '"lineterminator" must be a string, not int')
mydialect.lineterminator = None
with self.assertRaises(csv.Error) as cm:
mydialect()
self.assertEqual(str(cm.exception),
'"lineterminator" must be a string, not NoneType')
def test_invalid_chars(self): def test_invalid_chars(self):
def create_invalid(field_name, value, **kwargs): def create_invalid(field_name, value, **kwargs):

View File

@ -0,0 +1,2 @@
Improve error messages for incorrect types and values of :class:`csv.Dialect`
attributes.

View File

@ -237,7 +237,7 @@ _set_int(const char *name, int *target, PyObject *src, int dflt)
int value; int value;
if (!PyLong_CheckExact(src)) { if (!PyLong_CheckExact(src)) {
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"\"%s\" must be an integer", name); "\"%s\" must be an integer, not %T", name, src);
return -1; return -1;
} }
value = PyLong_AsInt(src); value = PyLong_AsInt(src);
@ -255,27 +255,29 @@ _set_char_or_none(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt
if (src == NULL) { if (src == NULL) {
*target = dflt; *target = dflt;
} }
else { else if (src == Py_None) {
*target = NOT_SET; *target = NOT_SET;
if (src != Py_None) { }
if (!PyUnicode_Check(src)) { else {
PyErr_Format(PyExc_TypeError, // similar to PyArg_Parse("C?")
"\"%s\" must be string or None, not %.200s", name, if (!PyUnicode_Check(src)) {
Py_TYPE(src)->tp_name); PyErr_Format(PyExc_TypeError,
return -1; "\"%s\" must be a unicode character or None, not %T",
} name, src);
Py_ssize_t len = PyUnicode_GetLength(src); return -1;
if (len < 0) {
return -1;
}
if (len != 1) {
PyErr_Format(PyExc_TypeError,
"\"%s\" must be a 1-character string",
name);
return -1;
}
*target = PyUnicode_READ_CHAR(src, 0);
} }
Py_ssize_t len = PyUnicode_GetLength(src);
if (len < 0) {
return -1;
}
if (len != 1) {
PyErr_Format(PyExc_TypeError,
"\"%s\" must be a unicode character or None, "
"not a string of length %zd",
name, len);
return -1;
}
*target = PyUnicode_READ_CHAR(src, 0);
} }
return 0; return 0;
} }
@ -287,11 +289,12 @@ _set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
*target = dflt; *target = dflt;
} }
else { else {
// similar to PyArg_Parse("C")
if (!PyUnicode_Check(src)) { if (!PyUnicode_Check(src)) {
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"\"%s\" must be string, not %.200s", name, "\"%s\" must be a unicode character, not %T",
Py_TYPE(src)->tp_name); name, src);
return -1; return -1;
} }
Py_ssize_t len = PyUnicode_GetLength(src); Py_ssize_t len = PyUnicode_GetLength(src);
if (len < 0) { if (len < 0) {
@ -299,8 +302,9 @@ _set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
} }
if (len != 1) { if (len != 1) {
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"\"%s\" must be a 1-character string", "\"%s\" must be a unicode character, "
name); "not a string of length %zd",
name, len);
return -1; return -1;
} }
*target = PyUnicode_READ_CHAR(src, 0); *target = PyUnicode_READ_CHAR(src, 0);
@ -314,16 +318,12 @@ _set_str(const char *name, PyObject **target, PyObject *src, const char *dflt)
if (src == NULL) if (src == NULL)
*target = PyUnicode_DecodeASCII(dflt, strlen(dflt), NULL); *target = PyUnicode_DecodeASCII(dflt, strlen(dflt), NULL);
else { else {
if (src == Py_None) if (!PyUnicode_Check(src)) {
*target = NULL;
else if (!PyUnicode_Check(src)) {
PyErr_Format(PyExc_TypeError, PyErr_Format(PyExc_TypeError,
"\"%s\" must be a string", name); "\"%s\" must be a string, not %T", name, src);
return -1; return -1;
} }
else { Py_XSETREF(*target, Py_NewRef(src));
Py_XSETREF(*target, Py_NewRef(src));
}
} }
return 0; return 0;
} }
@ -533,11 +533,6 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
/* validate options */ /* validate options */
if (dialect_check_quoting(self->quoting)) if (dialect_check_quoting(self->quoting))
goto err; goto err;
if (self->delimiter == NOT_SET) {
PyErr_SetString(PyExc_TypeError,
"\"delimiter\" must be a 1-character string");
goto err;
}
if (quotechar == Py_None && quoting == NULL) if (quotechar == Py_None && quoting == NULL)
self->quoting = QUOTE_NONE; self->quoting = QUOTE_NONE;
if (self->quoting != QUOTE_NONE && self->quotechar == NOT_SET) { if (self->quoting != QUOTE_NONE && self->quotechar == NOT_SET) {
@ -545,10 +540,6 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
"quotechar must be set if quoting enabled"); "quotechar must be set if quoting enabled");
goto err; goto err;
} }
if (self->lineterminator == NULL) {
PyErr_SetString(PyExc_TypeError, "lineterminator must be set");
goto err;
}
if (dialect_check_char("delimiter", self->delimiter, self, true) || if (dialect_check_char("delimiter", self->delimiter, self, true) ||
dialect_check_char("escapechar", self->escapechar, self, dialect_check_char("escapechar", self->escapechar, self,
!self->skipinitialspace) || !self->skipinitialspace) ||