[Python-checkins] bpo-44024: Improve the TypeError message in getattr and hasattr (GH-25863)

[serhiy-storchaka]

https://github.com/python/cpython/commit/16bf9bd157c7bf5f9c60414fa8e0fe5047c55a9b
commit: 16bf9bd157c7bf5f9c60414fa8e0fe5047c55a9b
branch: main
author: Géry Ogam <gery.ogam at gmail.com>
committer: serhiy-storchaka <storchaka at gmail.com>
date: 2022-01-18T22:46:26+02:00
summary:

bpo-44024: Improve the TypeError message in getattr and hasattr (GH-25863)

Use common error message for non-string attribute name in the builtin
functions getattr and hasattr.
The special check no longer needed since Python 3.0.

files:
A Misc/NEWS.d/next/Core and Builtins/2021-05-04-21-55-49.bpo-44024.M9m8Qd.rst
M Lib/test/test_builtin.py
M Python/bltinmodule.c

diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py
index 4b0b15f0a9361..c6e67cc2910cf 100644
--- a/Lib/test/test_builtin.py
+++ b/Lib/test/test_builtin.py
@@ -509,6 +509,9 @@ def test_delattr(self):
         sys.spam = 1
         delattr(sys, 'spam')
         self.assertRaises(TypeError, delattr)
+        self.assertRaises(TypeError, delattr, sys)
+        msg = r"^attribute name must be string, not 'int'$"
+        self.assertRaisesRegex(TypeError, msg, delattr, sys, 1)
 
     def test_dir(self):
         # dir(wrong number of arguments)
@@ -801,17 +804,21 @@ def test_filter_pickle(self):
 
     def test_getattr(self):
         self.assertTrue(getattr(sys, 'stdout') is sys.stdout)
-        self.assertRaises(TypeError, getattr, sys, 1)
-        self.assertRaises(TypeError, getattr, sys, 1, "foo")
         self.assertRaises(TypeError, getattr)
+        self.assertRaises(TypeError, getattr, sys)
+        msg = r"^attribute name must be string, not 'int'$"
+        self.assertRaisesRegex(TypeError, msg, getattr, sys, 1)
+        self.assertRaisesRegex(TypeError, msg, getattr, sys, 1, 'spam')
         self.assertRaises(AttributeError, getattr, sys, chr(sys.maxunicode))
         # unicode surrogates are not encodable to the default encoding (utf8)
         self.assertRaises(AttributeError, getattr, 1, "\uDAD1\uD51E")
 
     def test_hasattr(self):
         self.assertTrue(hasattr(sys, 'stdout'))
-        self.assertRaises(TypeError, hasattr, sys, 1)
         self.assertRaises(TypeError, hasattr)
+        self.assertRaises(TypeError, hasattr, sys)
+        msg = r"^attribute name must be string, not 'int'$"
+        self.assertRaisesRegex(TypeError, msg, hasattr, sys, 1)
         self.assertEqual(False, hasattr(sys, chr(sys.maxunicode)))
 
         # Check that hasattr propagates all exceptions outside of
@@ -1457,8 +1464,11 @@ def test_bug_27936(self):
     def test_setattr(self):
         setattr(sys, 'spam', 1)
         self.assertEqual(sys.spam, 1)
-        self.assertRaises(TypeError, setattr, sys, 1, 'spam')
         self.assertRaises(TypeError, setattr)
+        self.assertRaises(TypeError, setattr, sys)
+        self.assertRaises(TypeError, setattr, sys, 'spam')
+        msg = r"^attribute name must be string, not 'int'$"
+        self.assertRaisesRegex(TypeError, msg, setattr, sys, 1, 'spam')
 
     # test_str(): see test_unicode.py and test_bytes.py for str() tests.
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-05-04-21-55-49.bpo-44024.M9m8Qd.rst b/Misc/NEWS.d/next/Core and Builtins/2021-05-04-21-55-49.bpo-44024.M9m8Qd.rst
new file mode 100644
index 0000000000000..5037413353d28
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2021-05-04-21-55-49.bpo-44024.M9m8Qd.rst	
@@ -0,0 +1,2 @@
+Improve the exc:`TypeError` message for non-string second arguments passed to
+the built-in functions :func:`getattr` and :func:`hasattr`. Patch by Géry Ogam.
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 6763f9969707d..ef1b2bb9cf644 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -1091,11 +1091,6 @@ builtin_getattr(PyObject *self, PyObject *const *args, Py_ssize_t nargs)
 
     v = args[0];
     name = args[1];
-    if (!PyUnicode_Check(name)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "getattr(): attribute name must be string");
-        return NULL;
-    }
     if (nargs > 2) {
         if (_PyObject_LookupAttr(v, name, &result) == 0) {
             PyObject *dflt = args[2];
@@ -1156,11 +1151,6 @@ builtin_hasattr_impl(PyObject *module, PyObject *obj, PyObject *name)
 {
     PyObject *v;
 
-    if (!PyUnicode_Check(name)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "hasattr(): attribute name must be string");
-        return NULL;
-    }
     if (_PyObject_LookupAttr(obj, name, &v) < 0) {
         return NULL;
     }