bpo-44024: Improve the TypeError message for non-string second arguments passed to the built-in functions getattr and hasattr

[geryogam]

Problem

Actual behaviour:

>>> getattr('foobar', 123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: getattr(): attribute name must be string
>>> hasattr('foobar', 123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: hasattr(): attribute name must be string

Expected behaviour:

>>> getattr('foobar', 123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'int'
>>> hasattr('foobar', 123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'int'

Motivation:

>>> setattr('foobar', 123, 'baz')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'int'
>>> delattr('foobar', 123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: attribute name must be string, not 'int'

Solution

In the function builtin_getattr defined in Python/bltinmodule.c, we remove the following lines:

    if (!PyUnicode_Check(name)) {
        PyErr_SetString(PyExc_TypeError,
                        "getattr(): attribute name must be string");
        return NULL;
    }

because the expected TypeError message is already implemented in the subsequent call to the functions _PyObject_LookupAttr and PyObject_GetAttr defined in Objects/object.c:

PyObject *
PyObject_GetAttr(PyObject *v, PyObject *name)
{
    PyTypeObject *tp = Py_TYPE(v);
    if (!PyUnicode_Check(name)) {
        PyErr_Format(PyExc_TypeError,
                     "attribute name must be string, not '%.200s'",
                     Py_TYPE(name)->tp_name);
        return NULL;
    }

[…]

int
_PyObject_LookupAttr(PyObject *v, PyObject *name, PyObject **result)
{
    PyTypeObject *tp = Py_TYPE(v);

    if (!PyUnicode_Check(name)) {
        PyErr_Format(PyExc_TypeError,
                     "attribute name must be string, not '%.200s'",
                     Py_TYPE(name)->tp_name);
        *result = NULL;
        return -1;
    }

[…]

Likewise, in the function builtin_hasattr_impl defined in Python/bltinmodule.c, we remove the following lines:

    if (!PyUnicode_Check(name)) {
        PyErr_SetString(PyExc_TypeError,
                        "hasattr(): attribute name must be string");
        return NULL;
    }

because the expected TypeError message is already implemented in the subsequent call to the function _PyObject_LookupAttr defined in Objects/object.c.

https://bugs.python.org/issue44024

[JelleZijlstra]

Could you add a test for the error message and a news entry for the change?

[geryogam]

Thanks for the review @JelleZijlstra!

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[merwok]

Looks good, thanks! Could you pull and merge upstream in your branch?

[serhiy-storchaka]

The change LGTM, but tests are added in wrong place.

[serhiy-storchaka]

LGTM.