From 76d2adad90bb9f2695e21dab8b27de64f9d01439 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 13 Oct 2022 23:22:17 +0100 Subject: [PATCH 1/9] gh-98254: Include stdlib module names in error messages for NameErrors --- Lib/test/test_traceback.py | 7 +++ Lib/traceback.py | 8 ++++ Python/pythonrun.c | 7 --- Python/suggestions.c | 95 +++++++++++++++++++++++++++++--------- 4 files changed, 87 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index 4864b5c10b01bf..5fd28739d58c43 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -3184,6 +3184,13 @@ def func(): actual = self.get_suggestion(func) self.assertNotIn("something", actual) + + def test_name_error_for_stdlib_modules(self): + def func(): + stream = io.StringIO() + + actual = self.get_suggestion(func) + self.assertIn("forget to import 'io'", actual) class PurePythonSuggestionFormattingTests( diff --git a/Lib/traceback.py b/Lib/traceback.py index c46ddaf51a00d4..ac2b6db37e6bf2 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -712,6 +712,14 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, suggestion = _compute_suggestion_error(exc_value, exc_traceback) if suggestion: self._str += f". Did you mean: '{suggestion}'?" + if issubclass(exc_type, NameError): + mod_names = frozenset(mod for mod in sys.stdlib_module_names if not mod.startswith("_")) + wrong_name = getattr(exc_value, "name", None) + if wrong_name is not None and wrong_name in mod_names: + if suggestion: + self._str += f" Or did you forget to import '{wrong_name}'" + else: + self._str += f". Did you forget to import '{wrong_name}'" if lookup_lines: self._load_lines() self.__suppress_context__ = \ diff --git a/Python/pythonrun.c b/Python/pythonrun.c index acb1330b85fb75..a0005b32fcf146 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -1107,16 +1107,9 @@ print_exception_suggestions(struct exception_print_context *ctx, PyObject *f = ctx->file; PyObject *suggestions = _Py_Offer_Suggestions(value); if (suggestions) { - // Add a trailer ". Did you mean: (...)?" - if (PyFile_WriteString(". Did you mean: '", f) < 0) { - goto error; - } if (PyFile_WriteObject(suggestions, f, Py_PRINT_RAW) < 0) { goto error; } - if (PyFile_WriteString("'?", f) < 0) { - goto error; - } Py_DECREF(suggestions); } else if (PyErr_Occurred()) { diff --git a/Python/suggestions.c b/Python/suggestions.c index c336ec8ffffc9c..86e2e5ded76191 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -3,6 +3,7 @@ #include "pycore_pyerrors.h" #include "pycore_code.h" // _PyCode_GetVarnames() +#include "stdlib_module_names.h" // _Py_stdlib_module_names #define MAX_CANDIDATE_ITEMS 750 #define MAX_STRING_SIZE 40 @@ -175,7 +176,7 @@ calculate_suggestions(PyObject *dir, } static PyObject * -offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) +get_suggestions_for_attribute_error(PyAttributeErrorObject *exc) { PyObject *name = exc->name; // borrowed reference PyObject *obj = exc->obj; // borrowed reference @@ -195,35 +196,23 @@ offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) return suggestions; } - static PyObject * -offer_suggestions_for_name_error(PyNameErrorObject *exc) +offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) { - PyObject *name = exc->name; // borrowed reference - PyTracebackObject *traceback = (PyTracebackObject *) exc->traceback; // borrowed reference - // Abort if we don't have a variable name or we have an invalid one - // or if we don't have a traceback to work with - if (name == NULL || !PyUnicode_CheckExact(name) || - traceback == NULL || !Py_IS_TYPE(traceback, &PyTraceBack_Type) - ) { + PyObject* suggestion = get_suggestions_for_attribute_error(exc); + if (suggestion == NULL) { return NULL; } + // Add a trailer ". Did you mean: (...)?" + return PyUnicode_FromFormat(". Did you mean %R?", suggestion); +} - // Move to the traceback of the exception - while (1) { - PyTracebackObject *next = traceback->tb_next; - if (next == NULL || !Py_IS_TYPE(next, &PyTraceBack_Type)) { - break; - } - else { - traceback = next; - } - } - - PyFrameObject *frame = traceback->tb_frame; - assert(frame != NULL); +static PyObject * +get_suggestions_for_name_error(PyObject* name, PyFrameObject* frame) +{ PyCodeObject *code = PyFrame_GetCode(frame); assert(code != NULL && code->co_localsplusnames != NULL); + PyObject *varnames = _PyCode_GetVarnames(code); if (varnames == NULL) { return NULL; @@ -261,6 +250,66 @@ offer_suggestions_for_name_error(PyNameErrorObject *exc) return suggestions; } +static bool +is_name_stdlib_module(PyObject* name) +{ + const char* the_name = PyUnicode_AsUTF8(name); + Py_ssize_t len = Py_ARRAY_LENGTH(_Py_stdlib_module_names); + for (Py_ssize_t i = 0; i < len; i++) { + if (strcmp(the_name, _Py_stdlib_module_names[i]) == 0) { + return 1; + } + } + return 0; +} + +static PyObject * +offer_suggestions_for_name_error(PyNameErrorObject *exc) +{ + PyObject *name = exc->name; // borrowed reference + PyTracebackObject *traceback = (PyTracebackObject *) exc->traceback; // borrowed reference + // Abort if we don't have a variable name or we have an invalid one + // or if we don't have a traceback to work with + if (name == NULL || !PyUnicode_CheckExact(name) || + traceback == NULL || !Py_IS_TYPE(traceback, &PyTraceBack_Type) + ) { + return NULL; + } + + // Move to the traceback of the exception + while (1) { + PyTracebackObject *next = traceback->tb_next; + if (next == NULL || !Py_IS_TYPE(next, &PyTraceBack_Type)) { + break; + } + else { + traceback = next; + } + } + + PyFrameObject *frame = traceback->tb_frame; + assert(frame != NULL); + + PyObject* suggestion = get_suggestions_for_name_error(name, frame); + bool is_stdlib_module = is_name_stdlib_module(name); + + if (suggestion == NULL && !is_stdlib_module) { + return NULL; + } + + // Add a trailer ". Did you mean: (...)?" + PyObject* result = NULL; + if (!is_stdlib_module) { + result = PyUnicode_FromFormat(". Did you mean %R?", suggestion); + } else if (suggestion == NULL) { + result = PyUnicode_FromFormat(". Did you forget to import %R?", name); + } else { + result = PyUnicode_FromFormat(". Did you mean %R? Or did you forget to import %R?", suggestion, name); + } + Py_XDECREF(suggestion); + return result; +} + // Offer suggestions for a given exception. Returns a python string object containing the // suggestions. This function returns NULL if no suggestion was found or if an exception happened, // users must call PyErr_Occurred() to disambiguate. From 21fef723db4a26362b8d11dc9ebbffc7ed6a2b29 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 13 Oct 2022 23:25:06 +0100 Subject: [PATCH 2/9] Add news entry --- .../2022-10-13-23-23-01.gh-issue-98254.bC8IKt.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-13-23-23-01.gh-issue-98254.bC8IKt.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-10-13-23-23-01.gh-issue-98254.bC8IKt.rst b/Misc/NEWS.d/next/Core and Builtins/2022-10-13-23-23-01.gh-issue-98254.bC8IKt.rst new file mode 100644 index 00000000000000..af5d93ff24e96a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-13-23-23-01.gh-issue-98254.bC8IKt.rst @@ -0,0 +1,3 @@ +Modules from the standard library are now potentially suggested as part of the +error messages displayed by the interpreter when an :exc:`NameError` is raised +to the top level. Patch by Pablo Galindo From 88555fa504fec28a12d0cdc698e30fe0c946d3e8 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 13 Oct 2022 23:25:57 +0100 Subject: [PATCH 3/9] Consider all names --- Lib/traceback.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/traceback.py b/Lib/traceback.py index ac2b6db37e6bf2..bb7856a5142e03 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -713,9 +713,8 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, if suggestion: self._str += f". Did you mean: '{suggestion}'?" if issubclass(exc_type, NameError): - mod_names = frozenset(mod for mod in sys.stdlib_module_names if not mod.startswith("_")) wrong_name = getattr(exc_value, "name", None) - if wrong_name is not None and wrong_name in mod_names: + if wrong_name is not None and wrong_name in sys.stdlib_module_names: if suggestion: self._str += f" Or did you forget to import '{wrong_name}'" else: From 46ce50f419c7ac47fcbca8c98ede8ba759e98164 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 13 Oct 2022 23:27:00 +0100 Subject: [PATCH 4/9] Add more tests --- Lib/test/test_traceback.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index 5fd28739d58c43..5cebb74cf71327 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -3192,6 +3192,14 @@ def func(): actual = self.get_suggestion(func) self.assertIn("forget to import 'io'", actual) + def test_name_error_for_private_stdlib_modules(self): + def func(): + stream = _io.StringIO() + + actual = self.get_suggestion(func) + self.assertIn("forget to import '_io'", actual) + + class PurePythonSuggestionFormattingTests( PurePythonExceptionFormattingMixin, From 19e8469cb8755a4a4b96e641eca69489be390485 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Thu, 13 Oct 2022 23:31:47 +0100 Subject: [PATCH 5/9] Fix refleak --- Python/suggestions.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/suggestions.c b/Python/suggestions.c index 86e2e5ded76191..317344c56a5495 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -204,7 +204,9 @@ offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) return NULL; } // Add a trailer ". Did you mean: (...)?" - return PyUnicode_FromFormat(". Did you mean %R?", suggestion); + PyObject* result = PyUnicode_FromFormat(". Did you mean %R?", suggestion); + Py_DECREF(suggestion); + return result; } static PyObject * From 5103f8cfeeed17bc3a2ea2ff1b9699a63a51e668 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 14 Oct 2022 00:10:21 +0100 Subject: [PATCH 6/9] Fix test --- Python/suggestions.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/suggestions.c b/Python/suggestions.c index 317344c56a5495..4925bae5a359fa 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -302,11 +302,11 @@ offer_suggestions_for_name_error(PyNameErrorObject *exc) // Add a trailer ". Did you mean: (...)?" PyObject* result = NULL; if (!is_stdlib_module) { - result = PyUnicode_FromFormat(". Did you mean %R?", suggestion); + result = PyUnicode_FromFormat(". Did you mean: %R?", suggestion); } else if (suggestion == NULL) { result = PyUnicode_FromFormat(". Did you forget to import %R?", name); } else { - result = PyUnicode_FromFormat(". Did you mean %R? Or did you forget to import %R?", suggestion, name); + result = PyUnicode_FromFormat(". Did you mean: %R? Or did you forget to import %R?", suggestion, name); } Py_XDECREF(suggestion); return result; From a30504e229a7608fa693843fbb66ffc90adc8e8c Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 14 Oct 2022 01:06:35 +0100 Subject: [PATCH 7/9] Fix idle test --- Lib/idlelib/idle_test/test_run.py | 3 ++- Python/suggestions.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/idlelib/idle_test/test_run.py b/Lib/idlelib/idle_test/test_run.py index ec4637c5ca617a..a38e43dcb9d1c4 100644 --- a/Lib/idlelib/idle_test/test_run.py +++ b/Lib/idlelib/idle_test/test_run.py @@ -39,7 +39,8 @@ def __eq__(self, other): data = (('1/0', ZeroDivisionError, "division by zero\n"), ('abc', NameError, "name 'abc' is not defined. " - "Did you mean: 'abs'?\n"), + "Did you mean: 'abs'? " + "Or did you forget to import 'abc'?\n"), ('int.reel', AttributeError, "type object 'int' has no attribute 'reel'. " "Did you mean: 'real'?\n"), diff --git a/Python/suggestions.c b/Python/suggestions.c index 4925bae5a359fa..9213458a07a0ff 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -204,7 +204,7 @@ offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) return NULL; } // Add a trailer ". Did you mean: (...)?" - PyObject* result = PyUnicode_FromFormat(". Did you mean %R?", suggestion); + PyObject* result = PyUnicode_FromFormat(". Did you mean: %R?", suggestion); Py_DECREF(suggestion); return result; } From ca2c471e5d9aa434fd9dff03b7c2356594a3123c Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 14 Oct 2022 01:34:52 +0100 Subject: [PATCH 8/9] Fix whitespace --- Lib/test/test_traceback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index 5cebb74cf71327..2d17e060065075 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -3184,7 +3184,7 @@ def func(): actual = self.get_suggestion(func) self.assertNotIn("something", actual) - + def test_name_error_for_stdlib_modules(self): def func(): stream = io.StringIO() From 06cda71630ed4bd357cb05bc8618b06d92a9d14a Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Sat, 15 Oct 2022 20:20:16 +0100 Subject: [PATCH 9/9] Update Python/suggestions.c Co-authored-by: Jelle Zijlstra --- Python/suggestions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/suggestions.c b/Python/suggestions.c index 9213458a07a0ff..89b86f78bc7ac8 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -204,7 +204,7 @@ offer_suggestions_for_attribute_error(PyAttributeErrorObject *exc) return NULL; } // Add a trailer ". Did you mean: (...)?" - PyObject* result = PyUnicode_FromFormat(". Did you mean: %R?", suggestion); + PyObject* result = PyUnicode_FromFormat(". Did you mean: %R?", suggestion); Py_DECREF(suggestion); return result; }