bpo-38530: Offer suggestions on AttributeError (#16856)

When printing AttributeError, PyErr_Display will offer suggestions of similar attribute names in the object that the exception was raised from: >>> collections.namedtoplo Traceback (most recent call last): File "<stdin>", line 1, in <module> AttributeError: module 'collections' has no attribute 'namedtoplo'. Did you mean: namedtuple?
2021-04-14 02:36:07 +01:00 · 2021-04-14 02:36:07 +01:00 · 37494b441a
parent 3bc694d5f3
commit 37494b441a
12 changed files with 472 additions and 17 deletions
--- a/Doc/library/exceptions.rst
+++ b/Doc/library/exceptions.rst
@ -149,6 +149,13 @@ The following exceptions are the exceptions that are usually raised.
   assignment fails.  (When an object does not support attribute references or
   attribute assignments at all, :exc:`TypeError` is raised.)

+   The :attr:`name` and :attr:`obj` attributes can be set using keyword-only
+   arguments to the constructor. When set they represent the name of the attribute
+   that was attempted to be accessed and the object that was accessed for said
+   attribute, respectively.
+
+   .. versionchanged:: 3.10
+      Added the :attr:`name` and :attr:`obj` attributes.

 .. exception:: EOFError

--- a/Doc/whatsnew/3.10.rst
+++ b/Doc/whatsnew/3.10.rst
@ -125,8 +125,11 @@ Check :pep:`617` for more details.
 in :issue:`12782` and :issue:`40334`.)


-Better error messages in the parser
-----------------------------------
+Better error messages
+---------------------
+
+SyntaxErrors
+~~~~~~~~~~~~

 When parsing code that contains unclosed parentheses or brackets the interpreter
 now includes the location of the unclosed bracket of parentheses instead of displaying
@ -167,6 +170,23 @@ These improvements are inspired by previous work in the PyPy interpreter.
 (Contributed by Pablo Galindo in :issue:`42864` and Batuhan Taskaya in
 :issue:`40176`.)

+
+AttributeErrors
+~~~~~~~~~~~~~~~
+
+When printing :exc:`AttributeError`, :c:func:`PyErr_Display` will offer
+suggestions of simmilar attribute names in the object that the exception was
+raised from:
+
+.. code-block:: python
+
+    >>> collections.namedtoplo
+    Traceback (most recent call last):
+      File "<stdin>", line 1, in <module>
+    AttributeError: module 'collections' has no attribute 'namedtoplo'. Did you mean: namedtuple?
+
+(Contributed by Pablo Galindo in :issue:`38530`.)
+
 PEP 626: Precise line numbers for debugging and other tools
 -----------------------------------------------------------

--- a/Include/cpython/pyerrors.h
+++ b/Include/cpython/pyerrors.h
@ -62,6 +62,12 @@ typedef struct {
    PyObject *value;
 } PyStopIterationObject;

+typedef struct {
+    PyException_HEAD
+    PyObject *obj;
+    PyObject *name;
+} PyAttributeErrorObject;
+
 /* Compatibility typedefs */
 typedef PyOSErrorObject PyEnvironmentErrorObject;
 #ifdef MS_WINDOWS
--- a/Include/internal/pycore_pyerrors.h
+++ b/Include/internal/pycore_pyerrors.h
@ -86,6 +86,8 @@ PyAPI_FUNC(int) _PyErr_CheckSignalsTstate(PyThreadState *tstate);

 PyAPI_FUNC(void) _Py_DumpExtensionModules(int fd, PyInterpreterState *interp);

+extern PyObject* _Py_Offer_Suggestions(PyObject* exception);
+
 #ifdef __cplusplus
 }
 #endif
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@ -1414,6 +1414,165 @@ class ExceptionTests(unittest.TestCase):
            gc_collect()


+class AttributeErrorTests(unittest.TestCase):
+    def test_attributes(self):
+        # Setting 'attr' should not be a problem.
+        exc = AttributeError('Ouch!')
+        self.assertIsNone(exc.name)
+        self.assertIsNone(exc.obj)
+
+        sentinel = object()
+        exc = AttributeError('Ouch', name='carry', obj=sentinel)
+        self.assertEqual(exc.name, 'carry')
+        self.assertIs(exc.obj, sentinel)
+
+    def test_getattr_has_name_and_obj(self):
+        class A:
+            blech = None
+
+        obj = A()
+        try:
+            obj.bluch
+        except AttributeError as exc:
+            self.assertEqual("bluch", exc.name)
+            self.assertEqual(obj, exc.obj)
+
+    def test_getattr_has_name_and_obj_for_method(self):
+        class A:
+            def blech(self):
+                return
+
+        obj = A()
+        try:
+            obj.bluch()
+        except AttributeError as exc:
+            self.assertEqual("bluch", exc.name)
+            self.assertEqual(obj, exc.obj)
+
+    def test_getattr_suggestions(self):
+        class Substitution:
+            noise = more_noise = a = bc = None
+            blech = None
+
+        class Elimination:
+            noise = more_noise = a = bc = None
+            blch = None
+
+        class Addition:
+            noise = more_noise = a = bc = None
+            bluchin = None
+
+        class SubstitutionOverElimination:
+            blach = None
+            bluc = None
+
+        class SubstitutionOverAddition:
+            blach = None
+            bluchi = None
+
+        class EliminationOverAddition:
+            blucha = None
+            bluc = None
+
+        for cls, suggestion in [(Substitution, "blech?"),
+                                (Elimination, "blch?"),
+                                (Addition, "bluchin?"),
+                                (EliminationOverAddition, "bluc?"),
+                                (SubstitutionOverElimination, "blach?"),
+                                (SubstitutionOverAddition, "blach?")]:
+            try:
+                cls().bluch
+            except AttributeError as exc:
+                with support.captured_stderr() as err:
+                    sys.__excepthook__(*sys.exc_info())
+
+            self.assertIn(suggestion, err.getvalue())
+
+    def test_getattr_suggestions_do_not_trigger_for_long_attributes(self):
+        class A:
+            blech = None
+
+        try:
+            A().somethingverywrong
+        except AttributeError as exc:
+            with support.captured_stderr() as err:
+                sys.__excepthook__(*sys.exc_info())
+
+        self.assertNotIn("blech", err.getvalue())
+
+    def test_getattr_suggestions_do_not_trigger_for_big_dicts(self):
+        class A:
+            blech = None
+        # A class with a very big __dict__ will not be consider
+        # for suggestions.
+        for index in range(101):
+            setattr(A, f"index_{index}", None)
+
+        try:
+            A().bluch
+        except AttributeError as exc:
+            with support.captured_stderr() as err:
+                sys.__excepthook__(*sys.exc_info())
+
+        self.assertNotIn("blech", err.getvalue())
+
+    def test_getattr_suggestions_no_args(self):
+        class A:
+            blech = None
+            def __getattr__(self, attr):
+                raise AttributeError()
+
+        try:
+            A().bluch
+        except AttributeError as exc:
+            with support.captured_stderr() as err:
+                sys.__excepthook__(*sys.exc_info())
+
+        self.assertIn("blech", err.getvalue())
+
+        class A:
+            blech = None
+            def __getattr__(self, attr):
+                raise AttributeError
+
+        try:
+            A().bluch
+        except AttributeError as exc:
+            with support.captured_stderr() as err:
+                sys.__excepthook__(*sys.exc_info())
+
+        self.assertIn("blech", err.getvalue())
+
+    def test_getattr_suggestions_invalid_args(self):
+        class NonStringifyClass:
+            __str__ = None
+            __repr__ = None
+
+        class A:
+            blech = None
+            def __getattr__(self, attr):
+                raise AttributeError(NonStringifyClass())
+
+        class B:
+            blech = None
+            def __getattr__(self, attr):
+                raise AttributeError("Error", 23)
+
+        class C:
+            blech = None
+            def __getattr__(self, attr):
+                raise AttributeError(23)
+
+        for cls in [A, B, C]:
+            try:
+                cls().bluch
+            except AttributeError as exc:
+                with support.captured_stderr() as err:
+                    sys.__excepthook__(*sys.exc_info())
+
+            self.assertIn("blech", err.getvalue())
+
+
 class ImportErrorTests(unittest.TestCase):

    def test_attributes(self):
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@ -387,6 +387,7 @@ PYTHON_OBJS=	\
 		Python/dtoa.o \
 		Python/formatter_unicode.o \
 		Python/fileutils.o \
+		Python/suggestions.o \
 		Python/$(DYNLOADFILE) \
 		$(LIBOBJS) \
 		$(MACHDEP_OBJS) \
--- a/Builtins/2019-10-27-20-20-07.bpo-38530.ZyoDNn.rst
+++ b/Builtins/2019-10-27-20-20-07.bpo-38530.ZyoDNn.rst
@ -0,0 +1,3 @@
+When printing :exc:`AttributeError`, :c:func:`PyErr_Display` will offer
+suggestions of simmilar attribute names in the object that the exception was
+raised from. Patch by Pablo Galindo
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@ -1338,9 +1338,76 @@ SimpleExtendsException(PyExc_NameError, UnboundLocalError,
 /*
 *    AttributeError extends Exception
 */
-SimpleExtendsException(PyExc_Exception, AttributeError,
-                       "Attribute not found.");

+static int
+AttributeError_init(PyAttributeErrorObject *self, PyObject *args, PyObject *kwds)
+{
+    static char *kwlist[] = {"name", "obj", NULL};
+    PyObject *name = NULL;
+    PyObject *obj = NULL;
+
+    if (BaseException_init((PyBaseExceptionObject *)self, args, NULL) == -1) {
+        return -1;
+    }
+
+    PyObject *empty_tuple = PyTuple_New(0);
+    if (!empty_tuple) {
+        return -1;
+    }
+    if (!PyArg_ParseTupleAndKeywords(empty_tuple, kwds, "|$OO:AttributeError", kwlist,
+                                     &name, &obj)) {
+        Py_DECREF(empty_tuple);
+        return -1;
+    }
+    Py_DECREF(empty_tuple);
+
+    Py_XINCREF(name);
+    Py_XSETREF(self->name, name);
+
+    Py_XINCREF(obj);
+    Py_XSETREF(self->obj, obj);
+
+    return 0;
+}
+
+static int
+AttributeError_clear(PyAttributeErrorObject *self)
+{
+    Py_CLEAR(self->obj);
+    Py_CLEAR(self->name);
+    return BaseException_clear((PyBaseExceptionObject *)self);
+}
+
+static void
+AttributeError_dealloc(PyAttributeErrorObject *self)
+{
+    _PyObject_GC_UNTRACK(self);
+    AttributeError_clear(self);
+    Py_TYPE(self)->tp_free((PyObject *)self);
+}
+
+static int
+AttributeError_traverse(PyAttributeErrorObject *self, visitproc visit, void *arg)
+{
+    Py_VISIT(self->obj);
+    Py_VISIT(self->name);
+    return BaseException_traverse((PyBaseExceptionObject *)self, visit, arg);
+}
+
+static PyMemberDef AttributeError_members[] = {
+    {"name", T_OBJECT, offsetof(PyAttributeErrorObject, name), 0, PyDoc_STR("attribute name")},
+    {"obj", T_OBJECT, offsetof(PyAttributeErrorObject, obj), 0, PyDoc_STR("object")},
+    {NULL}  /* Sentinel */
+};
+
+static PyMethodDef AttributeError_methods[] = {
+    {NULL}  /* Sentinel */
+};
+
+ComplexExtendsException(PyExc_Exception, AttributeError,
+                        AttributeError, 0,
+                        AttributeError_methods, AttributeError_members,
+                        0, BaseException_str, "Attribute not found.");

 /*
 *    SyntaxError extends Exception
--- a/Objects/object.c
+++ b/Objects/object.c
@ -884,29 +884,60 @@ _PyObject_SetAttrId(PyObject *v, _Py_Identifier *name, PyObject *w)
    return result;
 }

+static inline int
+set_attribute_error_context(PyObject* v, PyObject* name)
+{
+    assert(PyErr_Occurred());
+    _Py_IDENTIFIER(name);
+    _Py_IDENTIFIER(obj);
+    // Intercept AttributeError exceptions and augment them to offer
+    // suggestions later.
+    if (PyErr_ExceptionMatches(PyExc_AttributeError)){
+        PyObject *type, *value, *traceback;
+        PyErr_Fetch(&type, &value, &traceback);
+        PyErr_NormalizeException(&type, &value, &traceback);
+        if (PyErr_GivenExceptionMatches(value, PyExc_AttributeError) &&
+            (_PyObject_SetAttrId(value, &PyId_name, name) ||
+             _PyObject_SetAttrId(value, &PyId_obj, v))) {
+            return 1;
+        }
+        PyErr_Restore(type, value, traceback);
+    }
+    return 0;
+}
+
 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;
    }
-    if (tp->tp_getattro != NULL)
-        return (*tp->tp_getattro)(v, name);
-    if (tp->tp_getattr != NULL) {
-        const char *name_str = PyUnicode_AsUTF8(name);
-        if (name_str == NULL)
-            return NULL;
-        return (*tp->tp_getattr)(v, (char *)name_str);
+
+    PyObject* result = NULL;
+    if (tp->tp_getattro != NULL) {
+        result = (*tp->tp_getattro)(v, name);
    }
-    PyErr_Format(PyExc_AttributeError,
-                 "'%.50s' object has no attribute '%U'",
-                 tp->tp_name, name);
-    return NULL;
+    else if (tp->tp_getattr != NULL) {
+        const char *name_str = PyUnicode_AsUTF8(name);
+        if (name_str == NULL) {
+            return NULL;
+        }
+        result = (*tp->tp_getattr)(v, (char *)name_str);
+    }
+    else {
+        PyErr_Format(PyExc_AttributeError,
+                    "'%.50s' object has no attribute '%U'",
+                    tp->tp_name, name);
+    }
+
+    if (result == NULL) {
+        set_attribute_error_context(v, name);
+    }
+    return result;
 }

 int
@ -1165,6 +1196,8 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
    PyErr_Format(PyExc_AttributeError,
                 "'%.50s' object has no attribute '%U'",
                 tp->tp_name, name);
+
+    set_attribute_error_context(obj, name);
    return 0;
 }

--- a/PCbuild/pythoncore.vcxproj
+++ b/PCbuild/pythoncore.vcxproj
@ -485,6 +485,7 @@
    <ClCompile Include="..\Python\dtoa.c" />
    <ClCompile Include="..\Python\Python-ast.c" />
    <ClCompile Include="..\Python\pythonrun.c" />
+    <ClCompile Include="..\Python\suggestions.c" />
    <ClCompile Include="..\Python\structmember.c" />
    <ClCompile Include="..\Python\symtable.c" />
    <ClCompile Include="..\Python\sysmodule.c" />
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@ -15,7 +15,7 @@
 #include "pycore_interp.h"        // PyInterpreterState.importlib
 #include "pycore_object.h"        // _PyDebug_PrintTotalRefs()
 #include "pycore_parser.h"        // _PyParser_ASTFromString()
-#include "pycore_pyerrors.h"      // _PyErr_Fetch
+#include "pycore_pyerrors.h"      // _PyErr_Fetch, _Py_Offer_Suggestions
 #include "pycore_pylifecycle.h"   // _Py_UnhandledKeyboardInterrupt
 #include "pycore_pystate.h"       // _PyInterpreterState_GET()
 #include "pycore_sysmodule.h"     // _PySys_Audit()
@ -953,6 +953,16 @@ print_exception(PyObject *f, PyObject *value)
    if (err < 0) {
        PyErr_Clear();
    }
+    PyObject* suggestions = _Py_Offer_Suggestions(value);
+    if (suggestions) {
+        // Add a trailer ". Did you mean: (...)?"
+        err = PyFile_WriteString(". Did you mean: ", f);
+        if (err == 0) {
+            err = PyFile_WriteObject(suggestions, f, Py_PRINT_RAW);
+            err += PyFile_WriteString("?", f);
+        }
+        Py_DECREF(suggestions);
+    }
    err += PyFile_WriteString("\n", f);
    Py_XDECREF(tb);
    Py_DECREF(value);
--- a/Python/suggestions.c
+++ b/Python/suggestions.c
@ -0,0 +1,146 @@
+#include "Python.h"
+
+#include "pycore_pyerrors.h"
+
+#define MAX_DISTANCE 3
+#define MAX_CANDIDATE_ITEMS 100
+#define MAX_STRING_SIZE 20
+
+/* Calculate the Levenshtein distance between string1 and string2 */
+static size_t
+levenshtein_distance(const char *a, const char *b) {
+    if (a == NULL || b == NULL) {
+        return 0;
+    }
+
+    const size_t a_size = strlen(a);
+    const size_t b_size = strlen(b);
+
+    if (a_size > MAX_STRING_SIZE || b_size > MAX_STRING_SIZE) {
+        return 0;
+    }
+
+    // Both strings are the same (by identity)
+    if (a == b) {
+        return 0;
+    }
+
+    // The first string is empty
+    if (a_size == 0) {
+        return b_size;
+    }
+
+    // The second string is empty
+    if (b_size == 0) {
+        return a_size;
+    }
+
+    size_t *buffer = PyMem_Calloc(a_size, sizeof(size_t));
+    if (buffer == NULL) {
+        return 0;
+    }
+
+    // Initialize the buffer row
+    size_t index = 0;
+    while (index < a_size) {
+        buffer[index] = index + 1;
+        index++;
+    }
+
+    size_t b_index = 0;
+    size_t result = 0;
+    while (b_index < b_size) {
+        char code = b[b_index];
+        size_t distance = result = b_index++;
+        index = SIZE_MAX;
+        while (++index < a_size) {
+            size_t b_distance = code == a[index] ? distance : distance + 1;
+            distance = buffer[index];
+            if (distance > result) {
+                if (b_distance > result) {
+                    result = result + 1;
+                } else {
+                    result = b_distance;
+                }
+            } else {
+                if (b_distance > distance) {
+                    result = distance + 1;
+                } else {
+                    result = b_distance;
+                }
+            }
+            buffer[index] = result;
+        }
+    }
+    PyMem_Free(buffer);
+    return result;
+}
+
+static inline PyObject *
+calculate_suggestions(PyObject *dir,
+                      PyObject *name) {
+    assert(!PyErr_Occurred());
+    assert(PyList_CheckExact(dir));
+
+    Py_ssize_t dir_size = PyList_GET_SIZE(dir);
+    if (dir_size >= MAX_CANDIDATE_ITEMS) {
+        return NULL;
+    }
+
+    Py_ssize_t suggestion_distance = PyUnicode_GetLength(name);
+    PyObject *suggestion = NULL;
+    for (int i = 0; i < dir_size; ++i) {
+        PyObject *item = PyList_GET_ITEM(dir, i);
+        const char *name_str = PyUnicode_AsUTF8(name);
+        if (name_str == NULL) {
+            PyErr_Clear();
+            continue;
+        }
+        Py_ssize_t current_distance = levenshtein_distance(PyUnicode_AsUTF8(name), PyUnicode_AsUTF8(item));
+        if (current_distance == 0 || current_distance > MAX_DISTANCE) {
+            continue;
+        }
+        if (!suggestion || current_distance < suggestion_distance) {
+            suggestion = item;
+            suggestion_distance = current_distance;
+        }
+    }
+    if (!suggestion) {
+        return NULL;
+    }
+    Py_INCREF(suggestion);
+    return suggestion;
+}
+
+static PyObject *
+offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) {
+    PyObject *name = exc->name; // borrowed reference
+    PyObject *obj = exc->obj; // borrowed reference
+
+    // Abort if we don't have an attribute name or we have an invalid one
+    if (name == NULL || obj == NULL || !PyUnicode_CheckExact(name)) {
+        return NULL;
+    }
+
+    PyObject *dir = PyObject_Dir(obj);
+    if (dir == NULL) {
+        return NULL;
+    }
+
+    PyObject *suggestions = calculate_suggestions(dir, name);
+    Py_DECREF(dir);
+    return suggestions;
+}
+
+// Offer suggestions for a given exception. Returns a python string object containing the
+// suggestions. This function does not raise exceptions and returns NULL if no suggestion was found.
+PyObject *_Py_Offer_Suggestions(PyObject *exception) {
+    PyObject *result = NULL;
+    assert(!PyErr_Occurred()); // Check that we are not going to clean any existing exception
+    if (PyErr_GivenExceptionMatches(exception, PyExc_AttributeError)) {
+        result = offer_suggestions_for_attribute_error((PyAttributeErrorObject *) exception);
+    }
+    assert(!PyErr_Occurred());
+    return result;
+}
+