]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-107944: Improve error message for getargs with bad keyword arguments (#114792)
authorShantanu <12621235+hauntsaninja@users.noreply.github.com>
Thu, 8 Feb 2024 09:04:41 +0000 (01:04 -0800)
committerGitHub <noreply@github.com>
Thu, 8 Feb 2024 09:04:41 +0000 (01:04 -0800)
Doc/whatsnew/3.13.rst
Lib/test/test_call.py
Lib/test/test_capi/test_getargs.py
Lib/test/test_exceptions.py
Misc/NEWS.d/next/Core and Builtins/2024-01-31-09-10-10.gh-issue-107944.XWm1B-.rst [new file with mode: 0644]
Python/getargs.c

index 2ac5afa8ce601cb87313fcfbcb13a9875578c585..50a2a69c75ac70bfb5f63e01bba3996435b960f2 100644 (file)
@@ -101,6 +101,17 @@ Improved Error Messages
   variables. See also :ref:`using-on-controlling-color`.
   (Contributed by Pablo Galindo Salgado in :gh:`112730`.)
 
+* When an incorrect keyword argument is passed to a function, the error message
+  now potentially suggests the correct keyword argument.
+  (Contributed by Pablo Galindo Salgado and Shantanu Jain in :gh:`107944`.)
+
+    >>> "better error messages!".split(max_split=1)
+    Traceback (most recent call last):
+      File "<stdin>", line 1, in <module>
+        "better error messages!".split(max_split=1)
+        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
+    TypeError: split() got an unexpected keyword argument 'max_split'. Did you mean 'maxsplit'?
+
 Other Language Changes
 ======================
 
index 3c8fc35e3c116d0904d07862aa4d6e4c4fb78f2e..2a6a5d287b04ee97af04d3754d47f8b42b09d3c2 100644 (file)
@@ -155,7 +155,7 @@ class CFunctionCallsErrorMessages(unittest.TestCase):
                                min, 0, default=1, key=2, foo=3)
 
     def test_varargs17_kw(self):
-        msg = r"'foo' is an invalid keyword argument for print\(\)$"
+        msg = r"print\(\) got an unexpected keyword argument 'foo'$"
         self.assertRaisesRegex(TypeError, msg,
                                print, 0, sep=1, end=2, file=3, flush=4, foo=5)
 
@@ -928,7 +928,7 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
         self.assertIn(f"Did you mean '{message}'?", str(cm.exception))
 
     @contextlib.contextmanager
-    def check_suggestion_not_pressent(self):
+    def check_suggestion_not_present(self):
         with self.assertRaises(TypeError) as cm:
             yield
         self.assertNotIn("Did you mean", str(cm.exception))
@@ -946,7 +946,7 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
 
         for keyword, suggestion in cases:
             with self.subTest(keyword):
-                ctx = self.check_suggestion_includes(suggestion) if suggestion else self.check_suggestion_not_pressent()
+                ctx = self.check_suggestion_includes(suggestion) if suggestion else self.check_suggestion_not_present()
                 with ctx:
                     foo(**{keyword:None})
 
@@ -987,6 +987,32 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
                 with self.check_suggestion_includes(suggestion):
                     func(bluch=None)
 
+    def test_unexpected_keyword_suggestion_via_getargs(self):
+        with self.check_suggestion_includes("maxsplit"):
+            "foo".split(maxsplt=1)
+
+        self.assertRaisesRegex(
+            TypeError, r"split\(\) got an unexpected keyword argument 'blech'$",
+            "foo".split, blech=1
+        )
+        with self.check_suggestion_not_present():
+            "foo".split(blech=1)
+        with self.check_suggestion_not_present():
+            "foo".split(more_noise=1, maxsplt=1)
+
+        # Also test the vgetargskeywords path
+        with self.check_suggestion_includes("name"):
+            ImportError(namez="oops")
+
+        self.assertRaisesRegex(
+            TypeError, r"ImportError\(\) got an unexpected keyword argument 'blech'$",
+            ImportError, blech=1
+        )
+        with self.check_suggestion_not_present():
+            ImportError(blech=1)
+        with self.check_suggestion_not_present():
+            ImportError(blech=1, namez="oops")
+
 @cpython_only
 class TestRecursion(unittest.TestCase):
 
index 9b6aef27625ad0e5cb03f10d1d1f988ba5f6bcc2..12039803ba543ebb0688130c838a963a67961f1a 100644 (file)
@@ -667,7 +667,7 @@ class Keywords_TestCase(unittest.TestCase):
         try:
             getargs_keywords((1,2),3,arg5=10,arg666=666)
         except TypeError as err:
-            self.assertEqual(str(err), "'arg666' is an invalid keyword argument for this function")
+            self.assertEqual(str(err), "this function got an unexpected keyword argument 'arg666'")
         else:
             self.fail('TypeError should have been raised')
 
@@ -675,7 +675,7 @@ class Keywords_TestCase(unittest.TestCase):
         try:
             getargs_keywords((1,2), 3, (4,(5,6)), (7,8,9), **{'\uDC80': 10})
         except TypeError as err:
-            self.assertEqual(str(err), "'\udc80' is an invalid keyword argument for this function")
+            self.assertEqual(str(err), "this function got an unexpected keyword argument '\udc80'")
         else:
             self.fail('TypeError should have been raised')
 
@@ -742,12 +742,12 @@ class KeywordOnly_TestCase(unittest.TestCase):
     def test_invalid_keyword(self):
         # extraneous keyword arg
         with self.assertRaisesRegex(TypeError,
-            "'monster' is an invalid keyword argument for this function"):
+            "this function got an unexpected keyword argument 'monster'"):
             getargs_keyword_only(1, 2, monster=666)
 
     def test_surrogate_keyword(self):
         with self.assertRaisesRegex(TypeError,
-            "'\udc80' is an invalid keyword argument for this function"):
+            "this function got an unexpected keyword argument '\udc80'"):
             getargs_keyword_only(1, 2, **{'\uDC80': 10})
 
     def test_weird_str_subclass(self):
@@ -761,7 +761,7 @@ class KeywordOnly_TestCase(unittest.TestCase):
             "invalid keyword argument for this function"):
             getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
         with self.assertRaisesRegex(TypeError,
-            "invalid keyword argument for this function"):
+            "this function got an unexpected keyword argument"):
             getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
 
     def test_weird_str_subclass2(self):
@@ -774,7 +774,7 @@ class KeywordOnly_TestCase(unittest.TestCase):
             "invalid keyword argument for this function"):
             getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
         with self.assertRaisesRegex(TypeError,
-            "invalid keyword argument for this function"):
+            "this function got an unexpected keyword argument"):
             getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
 
 
@@ -807,7 +807,7 @@ class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
 
     def test_empty_keyword(self):
         with self.assertRaisesRegex(TypeError,
-            "'' is an invalid keyword argument for this function"):
+            "this function got an unexpected keyword argument ''"):
             self.getargs(1, 2, **{'': 666})
 
 
@@ -1204,7 +1204,7 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
                 "function missing required argument 'a'"):
             parse((), {}, 'O', ['a'])
         with self.assertRaisesRegex(TypeError,
-                "'b' is an invalid keyword argument"):
+                "this function got an unexpected keyword argument 'b'"):
             parse((), {'b': 1}, '|O', ['a'])
         with self.assertRaisesRegex(TypeError,
                 fr"argument for function given by name \('a'\) "
@@ -1278,10 +1278,10 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
                         fr"and position \(1\)"):
                     parse((1,), {name: 2}, 'O|O', [name, 'b'])
                 with self.assertRaisesRegex(TypeError,
-                        f"'{name}' is an invalid keyword argument"):
+                        f"this function got an unexpected keyword argument '{name}'"):
                     parse((), {name: 1}, '|O', ['b'])
                 with self.assertRaisesRegex(TypeError,
-                        "'b' is an invalid keyword argument"):
+                        "this function got an unexpected keyword argument 'b'"):
                     parse((), {'b': 1}, '|O', [name])
 
                 invalid = name.encode() + (name.encode()[:-1] or b'\x80')
@@ -1301,17 +1301,17 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
                 for name2 in ('b', 'ë', 'ĉ', 'Ɐ', '𐀁'):
                     with self.subTest(name2=name2):
                         with self.assertRaisesRegex(TypeError,
-                                f"'{name2}' is an invalid keyword argument"):
+                                f"this function got an unexpected keyword argument '{name2}'"):
                             parse((), {name2: 1}, '|O', [name])
 
                 name2 = name.encode().decode('latin1')
                 if name2 != name:
                     with self.assertRaisesRegex(TypeError,
-                            f"'{name2}' is an invalid keyword argument"):
+                            f"this function got an unexpected keyword argument '{name2}'"):
                         parse((), {name2: 1}, '|O', [name])
                     name3 = name + '3'
                     with self.assertRaisesRegex(TypeError,
-                            f"'{name2}' is an invalid keyword argument"):
+                            f"this function got an unexpected keyword argument '{name2}'"):
                         parse((), {name2: 1, name3: 2}, '|OO', [name, name3])
 
     def test_nested_tuple(self):
index c57488e44aecc6463d2faaa602b3b57500545feb..c7e76414ff07154264c31b6a9603a9567fd3ee19 100644 (file)
@@ -1917,7 +1917,7 @@ class ImportErrorTests(unittest.TestCase):
         self.assertEqual(exc.name, 'somename')
         self.assertEqual(exc.path, 'somepath')
 
-        msg = "'invalid' is an invalid keyword argument for ImportError"
+        msg = r"ImportError\(\) got an unexpected keyword argument 'invalid'"
         with self.assertRaisesRegex(TypeError, msg):
             ImportError('test', invalid='keyword')
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-31-09-10-10.gh-issue-107944.XWm1B-.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-31-09-10-10.gh-issue-107944.XWm1B-.rst
new file mode 100644 (file)
index 0000000..8e3fb78
--- /dev/null
@@ -0,0 +1 @@
+Improve error message for function calls with bad keyword arguments via getargs
index 0c4ce282f48764868ff62fa283a53edb9f344a03..08e97ee3e627b5a5abd2addad7390b0388521928 100644 (file)
@@ -8,6 +8,7 @@
 #include "pycore_modsupport.h"    // export _PyArg_NoKeywords()
 #include "pycore_pylifecycle.h"   // _PyArg_Fini
 #include "pycore_tuple.h"         // _PyTuple_ITEMS()
+#include "pycore_pyerrors.h"      // _Py_CalculateSuggestions()
 
 /* Export Stable ABIs (abi only) */
 PyAPI_FUNC(int) _PyArg_Parse_SizeT(PyObject *, const char *, ...);
@@ -1424,12 +1425,31 @@ error_unexpected_keyword_arg(PyObject *kwargs, PyObject *kwnames, PyObject *kwtu
         int match = PySequence_Contains(kwtuple, keyword);
         if (match <= 0) {
             if (!match) {
-                PyErr_Format(PyExc_TypeError,
-                             "'%S' is an invalid keyword "
-                             "argument for %.200s%s",
-                             keyword,
-                             (fname == NULL) ? "this function" : fname,
-                             (fname == NULL) ? "" : "()");
+                PyObject *kwlist = PySequence_List(kwtuple);
+                if (!kwlist) {
+                    return;
+                }
+                PyObject *suggestion_keyword = _Py_CalculateSuggestions(kwlist, keyword);
+                Py_DECREF(kwlist);
+
+                if (suggestion_keyword) {
+                    PyErr_Format(PyExc_TypeError,
+                                "%.200s%s got an unexpected keyword argument '%S'."
+                                " Did you mean '%S'?",
+                                (fname == NULL) ? "this function" : fname,
+                                (fname == NULL) ? "" : "()",
+                                keyword,
+                                suggestion_keyword);
+                    Py_DECREF(suggestion_keyword);
+                }
+                else {
+                    PyErr_Format(PyExc_TypeError,
+                                "%.200s%s got an unexpected keyword argument '%S'",
+                                (fname == NULL) ? "this function" : fname,
+                                (fname == NULL) ? "" : "()",
+                                keyword);
+                }
+
             }
             return;
         }
@@ -1457,6 +1477,9 @@ PyArg_ValidateKeywordArguments(PyObject *kwargs)
     return 1;
 }
 
+static PyObject *
+new_kwtuple(const char * const *keywords, int total, int pos);
+
 #define IS_END_OF_FORMAT(c) (c == '\0' || c == ';' || c == ':')
 
 static int
@@ -1722,12 +1745,35 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
                 }
             }
             if (!match) {
-                PyErr_Format(PyExc_TypeError,
-                             "'%U' is an invalid keyword "
-                             "argument for %.200s%s",
-                             key,
-                             (fname == NULL) ? "this function" : fname,
-                             (fname == NULL) ? "" : "()");
+                PyObject *_pykwtuple = new_kwtuple(kwlist, len, pos);
+                if (!_pykwtuple) {
+                    return cleanreturn(0, &freelist);
+                }
+                PyObject *pykwlist = PySequence_List(_pykwtuple);
+                Py_DECREF(_pykwtuple);
+                if (!pykwlist) {
+                    return cleanreturn(0, &freelist);
+                }
+                PyObject *suggestion_keyword = _Py_CalculateSuggestions(pykwlist, key);
+                Py_DECREF(pykwlist);
+
+                if (suggestion_keyword) {
+                    PyErr_Format(PyExc_TypeError,
+                                "%.200s%s got an unexpected keyword argument '%S'."
+                                " Did you mean '%S'?",
+                                (fname == NULL) ? "this function" : fname,
+                                (fname == NULL) ? "" : "()",
+                                key,
+                                suggestion_keyword);
+                    Py_DECREF(suggestion_keyword);
+                }
+                else {
+                    PyErr_Format(PyExc_TypeError,
+                                "%.200s%s got an unexpected keyword argument '%S'",
+                                (fname == NULL) ? "this function" : fname,
+                                (fname == NULL) ? "" : "()",
+                                key);
+                }
                 return cleanreturn(0, &freelist);
             }
         }