From af0f75b24a976b19c735e20963cd60293529ccc3 Mon Sep 17 00:00:00 2001 From: Miguel Mendes Date: Sat, 8 May 2021 12:50:23 +0100 Subject: [PATCH 1/3] Improve str error messages --- Lib/test/string_tests.py | 6 ++++++ Objects/unicodeobject.c | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 840d7bb7550f71b..8cd5a3fbdf55c6c 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -1195,6 +1195,12 @@ def test_subscript(self): self.checkraises(TypeError, 'abc', '__getitem__', 'def') + for idx in ('def', object()): + with self.assertRaises(TypeError) as cm: + getattr('abc', '__getitem__')(idx) + self.assertEqual(str(cm.exception), + "string indices must be integers, not '{}'".format(type(idx).__name__)) + def test_slice(self): self.checkequal('abc', 'abc', '__getitem__', slice(0, 1000)) self.checkequal('abc', 'abc', '__getitem__', slice(0, 3)) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index bfd5c881215deec..29d5b8c3c2867d9 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -14564,7 +14564,8 @@ unicode_subscript(PyObject* self, PyObject* item) assert(_PyUnicode_CheckConsistency(result, 1)); return result; } else { - PyErr_SetString(PyExc_TypeError, "string indices must be integers"); + PyErr_Format(PyExc_TypeError, "string indices must be integers, not '%.200s'", + Py_TYPE(item)->tp_name); return NULL; } } From 0ccfa55176627e3be6634fdb95b11e9d8dd00ec8 Mon Sep 17 00:00:00 2001 From: Miguel Mendes Date: Tue, 11 May 2021 19:14:46 +0100 Subject: [PATCH 2/3] Improve tests --- Lib/test/string_tests.py | 12 ++++++------ Lib/test/test_userstring.py | 4 +++- .../2021-05-11-21-52-44.bpo-44110.VqbAsB.rst | 1 + 3 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-05-11-21-52-44.bpo-44110.VqbAsB.rst diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 8cd5a3fbdf55c6c..f286936196ba121 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -80,12 +80,14 @@ class subtype(self.__class__.type2test): self.assertIsNot(obj, realresult) # check that obj.method(*args) raises exc - def checkraises(self, exc, obj, methodname, *args): + def checkraises(self, exc, obj, methodname, *args, **kwargs): obj = self.fixtype(obj) args = self.fixtype(args) with self.assertRaises(exc) as cm: getattr(obj, methodname)(*args) self.assertNotEqual(str(cm.exception), '') + if 'expected_msg' in kwargs: + self.assertEqual(str(cm.exception), kwargs['expected_msg']) # call obj.method(*args) without any checks def checkcall(self, obj, methodname, *args): @@ -1195,11 +1197,9 @@ def test_subscript(self): self.checkraises(TypeError, 'abc', '__getitem__', 'def') - for idx in ('def', object()): - with self.assertRaises(TypeError) as cm: - getattr('abc', '__getitem__')(idx) - self.assertEqual(str(cm.exception), - "string indices must be integers, not '{}'".format(type(idx).__name__)) + for idx_type in ('def', object()): + expected_msg = "string indices must be integers, not '{}'".format(type(idx_type).__name__) + self.checkraises(TypeError, 'abc', '__getitem__', idx_type, expected_msg=expected_msg) def test_slice(self): self.checkequal('abc', 'abc', '__getitem__', slice(0, 1000)) diff --git a/Lib/test/test_userstring.py b/Lib/test/test_userstring.py index 4d1d8b6b6fe2d95..6a1abcce442c402 100644 --- a/Lib/test/test_userstring.py +++ b/Lib/test/test_userstring.py @@ -27,12 +27,14 @@ def checkequal(self, result, object, methodname, *args, **kwargs): realresult ) - def checkraises(self, exc, obj, methodname, *args): + def checkraises(self, exc, obj, methodname, *args, **kwargs): obj = self.fixtype(obj) # we don't fix the arguments, because UserString can't cope with it with self.assertRaises(exc) as cm: getattr(obj, methodname)(*args) self.assertNotEqual(str(cm.exception), '') + if 'expected_msg' in kwargs: + self.assertEqual(str(cm.exception), kwargs['expected_msg']) def checkcall(self, object, methodname, *args): object = self.fixtype(object) diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-05-11-21-52-44.bpo-44110.VqbAsB.rst b/Misc/NEWS.d/next/Core and Builtins/2021-05-11-21-52-44.bpo-44110.VqbAsB.rst new file mode 100644 index 000000000000000..3b82e425ab4a744 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-05-11-21-52-44.bpo-44110.VqbAsB.rst @@ -0,0 +1 @@ +Improve :func:`str.__getitem__` error message From e78a9b9bfcf1f321b73367d9186bfce57deccb5e Mon Sep 17 00:00:00 2001 From: Miguel Mendes Date: Wed, 12 May 2021 20:40:09 +0100 Subject: [PATCH 3/3] Simplify test helper --- Lib/test/string_tests.py | 6 +++--- Lib/test/test_userstring.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index f286936196ba121..089701c983a1467 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -80,14 +80,14 @@ class subtype(self.__class__.type2test): self.assertIsNot(obj, realresult) # check that obj.method(*args) raises exc - def checkraises(self, exc, obj, methodname, *args, **kwargs): + def checkraises(self, exc, obj, methodname, *args, expected_msg=None): obj = self.fixtype(obj) args = self.fixtype(args) with self.assertRaises(exc) as cm: getattr(obj, methodname)(*args) self.assertNotEqual(str(cm.exception), '') - if 'expected_msg' in kwargs: - self.assertEqual(str(cm.exception), kwargs['expected_msg']) + if expected_msg is not None: + self.assertEqual(str(cm.exception), expected_msg) # call obj.method(*args) without any checks def checkcall(self, obj, methodname, *args): diff --git a/Lib/test/test_userstring.py b/Lib/test/test_userstring.py index 6a1abcce442c402..51b4f6041e49bfd 100644 --- a/Lib/test/test_userstring.py +++ b/Lib/test/test_userstring.py @@ -27,14 +27,14 @@ def checkequal(self, result, object, methodname, *args, **kwargs): realresult ) - def checkraises(self, exc, obj, methodname, *args, **kwargs): + def checkraises(self, exc, obj, methodname, *args, expected_msg=None): obj = self.fixtype(obj) # we don't fix the arguments, because UserString can't cope with it with self.assertRaises(exc) as cm: getattr(obj, methodname)(*args) self.assertNotEqual(str(cm.exception), '') - if 'expected_msg' in kwargs: - self.assertEqual(str(cm.exception), kwargs['expected_msg']) + if expected_msg is not None: + self.assertEqual(str(cm.exception), expected_msg) def checkcall(self, object, methodname, *args): object = self.fixtype(object)