gh-99139: Improve NameError error suggestion for instances (#99140)

pablogsal · web-flow · commit 99e2e60cb25b · 2022-11-06T13:52:06.000Z
diff --git 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.<NAME>`` 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 "<stdin>", 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
@@ -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
diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py
@@ -3356,6 +3356,31 @@ def func():
 
         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
@@ -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
@@ -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.<NAME>`` instead of the closest match in the method
+scope. Patch by Pablo Galindo
diff --git 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
