Skip to content

bpo-38530: Offer suggestions on AttributeError #16856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Include/cpython/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ typedef struct {
PyObject *value;
} PyStopIterationObject;

typedef struct {
PyException_HEAD
PyObject *obj;
PyObject *name;
} PyAttributeErrorObject;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.python.org/dev/peps/pep-0473/ was rejected because it was too broad, but this PR adds a single exception, which sounds ok according to the resolution: https://mail.python.org/pipermail/python-dev/2019-March/156692.html

My main worry is the risk of creating "more" and "worse" exception cycles, but Pablo says that it sounds unlikely: https://bugs.python.org/issue38530#msg354975


/* Compatibility typedefs */
typedef PyOSErrorObject PyEnvironmentErrorObject;
#ifdef MS_WINDOWS
Expand Down
13 changes: 13 additions & 0 deletions Include/internal/pycore_suggestions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#include "Python.h"

#ifndef Py_INTERNAL_SUGGESTIONS_H
#define Py_INTERNAL_SUGGESTIONS_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to keep the header file, you should add extern "C". Third party C++ code is allowed to use it.


#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

int _Py_offer_suggestions_for_attribute_error(PyAttributeErrorObject* exception_value);


#endif /* !Py_INTERNAL_SUGGESTIONS_H */
1 change: 0 additions & 1 deletion Include/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ PyAPI_FUNC(PyObject *) PyErr_NewExceptionWithDoc(
const char *name, const char *doc, PyObject *base, PyObject *dict);
PyAPI_FUNC(void) PyErr_WriteUnraisable(PyObject *);


/* In signalmodule.c */
PyAPI_FUNC(int) PyErr_CheckSignals(void);
PyAPI_FUNC(void) PyErr_SetInterrupt(void);
Expand Down
159 changes: 159 additions & 0 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,165 @@ class TestException(MemoryError):
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.assertNotIn("blech", err.getvalue())


class ImportErrorTests(unittest.TestCase):

def test_attributes(self):
Expand Down
2 changes: 2 additions & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ PYTHON_OBJS= \
Python/dtoa.o \
Python/formatter_unicode.o \
Python/fileutils.o \
Python/suggestions.o \
Python/$(DYNLOADFILE) \
$(LIBOBJS) \
$(MACHDEP_OBJS) \
Expand Down Expand Up @@ -1161,6 +1162,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_list.h \
$(srcdir)/Include/internal/pycore_long.h \
$(srcdir)/Include/internal/pycore_object.h \
$(srcdir)/Include/internal/pycore_suggestions.h \
$(srcdir)/Include/internal/pycore_pathconfig.h \
$(srcdir)/Include/internal/pycore_pyarena.h \
$(srcdir)/Include/internal/pycore_pyerrors.h \
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
77 changes: 75 additions & 2 deletions Objects/exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1338,9 +1338,82 @@ 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 PyObject *
AttributeError_str(PyAttributeErrorObject *self)
{
return BaseException_str((PyBaseExceptionObject *)self);
}

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, AttributeError_str, "Attribute not found.");

/*
* SyntaxError extends Exception
Expand Down
47 changes: 40 additions & 7 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_symtable.h" // PySTEntry_Type
#include "pycore_unionobject.h" // _Py_UnionType
#include "pycore_suggestions.h"
#include "frameobject.h"
#include "interpreteridobject.h"

Expand Down Expand Up @@ -884,10 +885,31 @@ _PyObject_SetAttrId(PyObject *v, _Py_Identifier *name, PyObject *w)
return result;
}

static inline int
add_context_to_attribute_error_exception(PyObject* v, PyObject* name)
{
assert(PyErr_Occurred());
// 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_SetAttrString(value, "name", name) ||
PyObject_SetAttrString(value, "obj", v))) {
return 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really useful to report setattr() failed? the callers don't seem to care.

}
PyErr_Restore(type, value, traceback);
}
return 0;
}

PyObject *
PyObject_GetAttr(PyObject *v, PyObject *name)
{
PyTypeObject *tp = Py_TYPE(v);
PyObject* result = NULL;

if (!PyUnicode_Check(name)) {
PyErr_Format(PyExc_TypeError,
Expand All @@ -896,17 +918,23 @@ PyObject_GetAttr(PyObject *v, PyObject *name)
return NULL;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I like to only declare variables just before they are initialized (here) thanks to C99.

PyObject* result;

if (tp->tp_getattro != NULL)
return (*tp->tp_getattro)(v, name);
if (tp->tp_getattr != NULL) {
result = (*tp->tp_getattro)(v, name);
else 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);
result = (*tp->tp_getattr)(v, (char *)name_str);
} else {
PyErr_Format(PyExc_AttributeError,
"'%.50s' object has no attribute '%U'",
tp->tp_name, name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe only set result = NULL; in this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is code from before, I prefer to not change it in case we introduce some unwanted change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok. It was just a suggestion, you can ignore it. it's just a coding style preference ;-)

}
PyErr_Format(PyExc_AttributeError,
"'%.50s' object has no attribute '%U'",
tp->tp_name, name);
return NULL;

if (!result && add_context_to_attribute_error_exception(v, name)) {
return NULL;
}

return result;
}

int
Expand Down Expand Up @@ -1165,6 +1193,11 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
PyErr_Format(PyExc_AttributeError,
"'%.50s' object has no attribute '%U'",
tp->tp_name, name);

if (add_context_to_attribute_error_exception(obj, name)) {
return 0;
}

return 0;
}

Expand Down
Loading