From 99e2e60cb25bfcf77ba1386d331cfa85864e6a65 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Sun, 6 Nov 2022 13:52:06 +0000 Subject: [PATCH] gh-99139: Improve NameError error suggestion for instances (#99140) --- Doc/whatsnew/3.12.rst | 21 ++++++++++++++++ Include/internal/pycore_global_strings.h | 1 + .../internal/pycore_runtime_init_generated.h | 7 ++++++ Lib/test/test_traceback.py | 25 +++++++++++++++++++ Lib/traceback.py | 10 ++++++++ ...2-11-05-18-36-27.gh-issue-99139.cI9vV1.rst | 5 ++++ Python/suggestions.c | 23 +++++++++++++++++ 7 files changed, 92 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-05-18-36-27.gh-issue-99139.cI9vV1.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index b6daa6d5c9d..fb28d6758d3 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -75,6 +75,27 @@ Important deprecations, removals or restrictions: Improved Error Messages ======================= +* Improve the error suggestion for :exc:`NameError` exceptions for instances. + Now if a :exc:`NameError` is raised in a method and the instance has an + attribute that's exactly equal to the name in the exception, the suggestion + will include ``self.`` instead of the closest match in the method + scope. Contributed by Pablo Galindo in :gh:`99139`. + + >>> class A: + ... def __init__(self): + ... self.blech = 1 + ... + ... def foo(self): + ... somethin = blech + + >>> A().foo() + Traceback (most recent call last): + File "", line 1 + somethin = blech + ^^^^^ + NameError: name 'blech' is not defined. Did you mean: 'self.blech'? + + * Improve the :exc:`SyntaxError` error message when the user types ``import x from y`` instead of ``from y import x``. Contributed by Pablo Galindo in :gh:`98931`. diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index e60bd4b94bb..8912895b0de 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -600,6 +600,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(seek) STRUCT_FOR_ID(seekable) STRUCT_FOR_ID(selectors) + STRUCT_FOR_ID(self) STRUCT_FOR_ID(send) STRUCT_FOR_ID(sep) STRUCT_FOR_ID(sequence) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 8ce103700d6..a1f1efdf43b 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -1109,6 +1109,7 @@ extern "C" { INIT_ID(seek), \ INIT_ID(seekable), \ INIT_ID(selectors), \ + INIT_ID(self), \ INIT_ID(send), \ INIT_ID(sep), \ INIT_ID(sequence), \ @@ -2567,6 +2568,8 @@ _PyUnicode_InitStaticStrings(void) { PyUnicode_InternInPlace(&string); string = &_Py_ID(selectors); PyUnicode_InternInPlace(&string); + string = &_Py_ID(self); + PyUnicode_InternInPlace(&string); string = &_Py_ID(send); PyUnicode_InternInPlace(&string); string = &_Py_ID(sep); @@ -7104,6 +7107,10 @@ _PyStaticObjects_CheckRefcnt(void) { _PyObject_Dump((PyObject *)&_Py_ID(selectors)); Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); }; + if (Py_REFCNT((PyObject *)&_Py_ID(self)) < _PyObject_IMMORTAL_REFCNT) { + _PyObject_Dump((PyObject *)&_Py_ID(self)); + Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); + }; if (Py_REFCNT((PyObject *)&_Py_ID(send)) < _PyObject_IMMORTAL_REFCNT) { _PyObject_Dump((PyObject *)&_Py_ID(send)); Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT"); diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index 149d0234fe8..430daf69d29 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -3356,6 +3356,31 @@ class SuggestionFormattingTestBase: actual = self.get_suggestion(func) self.assertNotIn("blech", actual) + + def test_name_error_with_instance(self): + class A: + def __init__(self): + self.blech = None + def foo(self): + blich = 1 + x = blech + + instance = A() + actual = self.get_suggestion(instance.foo) + self.assertIn("self.blech", actual) + + def test_unbound_local_error_with_instance(self): + class A: + def __init__(self): + self.blech = None + def foo(self): + blich = 1 + x = blech + blech = 1 + + instance = A() + actual = self.get_suggestion(instance.foo) + self.assertNotIn("self.blech", actual) def test_unbound_local_error_does_not_match(self): def func(): diff --git a/Lib/traceback.py b/Lib/traceback.py index cf5f355ff04..8d518728fa1 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -1037,6 +1037,16 @@ def _compute_suggestion_error(exc_value, tb, wrong_name): + list(frame.f_globals) + list(frame.f_builtins) ) + + # Check first if we are in a method and the instance + # has the wrong name as attribute + if 'self' in frame.f_locals: + self = frame.f_locals['self'] + if hasattr(self, wrong_name): + return f"self.{wrong_name}" + + # Compute closest match + if len(d) > _MAX_CANDIDATE_ITEMS: return None wrong_name_len = len(wrong_name) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-05-18-36-27.gh-issue-99139.cI9vV1.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-05-18-36-27.gh-issue-99139.cI9vV1.rst new file mode 100644 index 00000000000..62b78b9b453 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-05-18-36-27.gh-issue-99139.cI9vV1.rst @@ -0,0 +1,5 @@ +Improve the error suggestion for :exc:`NameError` exceptions for instances. +Now if a :exc:`NameError` is raised in a method and the instance has an +attribute that's exactly equal to the name in the exception, the suggestion +will include ``self.`` instead of the closest match in the method +scope. Patch by Pablo Galindo diff --git a/Python/suggestions.c b/Python/suggestions.c index 82376b6cd98..239245d73de 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -1,5 +1,6 @@ #include "Python.h" #include "pycore_frame.h" +#include "pycore_runtime_init.h" // _Py_ID() #include "pycore_pyerrors.h" #include "pycore_code.h" // _PyCode_GetVarnames() @@ -226,6 +227,24 @@ get_suggestions_for_name_error(PyObject* name, PyFrameObject* frame) return NULL; } + // Are we inside a method and the instance has an attribute called 'name'? + if (PySequence_Contains(dir, &_Py_ID(self)) > 0) { + PyObject* locals = PyFrame_GetLocals(frame); + if (!locals) { + goto error; + } + PyObject* self = PyDict_GetItem(locals, &_Py_ID(self)); /* borrowed */ + Py_DECREF(locals); + if (!self) { + goto error; + } + + if (PyObject_HasAttr(self, name)) { + Py_DECREF(dir); + return PyUnicode_FromFormat("self.%S", name); + } + } + PyObject *suggestions = calculate_suggestions(dir, name); Py_DECREF(dir); if (suggestions != NULL) { @@ -250,6 +269,10 @@ get_suggestions_for_name_error(PyObject* name, PyFrameObject* frame) Py_DECREF(dir); return suggestions; + +error: + Py_DECREF(dir); + return NULL; } static bool