gh-77566: Make setdefault methods require explicit default argument for dbm backends

[furkanonder]

• Issue: setdefault() with a single argument doesn't work for dbm.gnu and dmb.dumb objects #77566

[furkanonder]

By the way, if we want to use the same error messages for all DBM backends, we can follow these changes;

diff --git a/Lib/dbm/dumb.py b/Lib/dbm/dumb.py
index 1bc239a84f..dd98c51954 100644
--- a/Lib/dbm/dumb.py
+++ b/Lib/dbm/dumb.py
@@ -287,6 +287,21 @@ def __enter__(self):
     def __exit__(self, *args):
         self.close()

+    def setdefault(self, key, default):
+        if isinstance(key, str):
+            key = key.encode('utf-8')
+        try:
+            if key in self._index:
+                return self[key]
+            else:
+                self[key] = default
+                return default
+        except TypeError:
+            if self._index is None:
+                raise error('DBM object has already been closed') from None
+            else:
+                raise
+
     def reorganize(self):
         if self._readonly:
             raise error('The database is opened for reading only')
diff --git a/Lib/dbm/sqlite3.py b/Lib/dbm/sqlite3.py
index b296a1bcd1..691f4354b3 100644
--- a/Lib/dbm/sqlite3.py
+++ b/Lib/dbm/sqlite3.py
@@ -126,6 +126,12 @@ def __exit__(self, *args):
     def reorganize(self):
         self._execute(REORGANIZE)

+    def setdefault(self, key, default):
+        if key in self:
+            return self[key]
+        else:
+            self[key] = default
+            return default

 def open(filename, /, flag="r", mode=0o666):
     """Open a dbm.sqlite3 database and return the dbm object.
diff --git a/Modules/clinic/_dbmmodule.c.h b/Modules/clinic/_dbmmodule.c.h
index 091ce9edc4..1fdca2d0d6 100644
--- a/Modules/clinic/_dbmmodule.c.h
+++ b/Modules/clinic/_dbmmodule.c.h
@@ -108,7 +108,7 @@ exit:
 }

 PyDoc_STRVAR(_dbm_dbm_setdefault__doc__,
-"setdefault($self, key, default=b\'\', /)\n"
+"setdefault($self, key, default, /)\n"
 "--\n"
 "\n"
 "Return the value for key if present, otherwise default.\n"
@@ -132,21 +132,26 @@ _dbm_dbm_setdefault(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py
     #  define KWTUPLE NULL
     #endif

-    static const char * const _keywords[] = {"", "", NULL};
-    static _PyArg_Parser _parser = {
-        .keywords = _keywords,
-        .format = "s#|O:setdefault",
-        .kwtuple = KWTUPLE,
-    };
-    #undef KWTUPLE
     const char *key;
     Py_ssize_t key_length;
-    PyObject *default_value = NULL;
-
-    if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser,
-        &key, &key_length, &default_value)) {
+    PyObject *default_value;
+
+    if (nargs != 2 || (kwnames && PyTuple_GET_SIZE(kwnames))) {
+        if (nargs < 2) {
+            PyErr_Format(PyExc_TypeError,
+                         "_dbm.dbm.setdefault() missing %d required positional argument%s: 'default'",
+                         2 - nargs, (2 - nargs == 1 ? "" : "s"));
+        } else {
+            PyErr_Format(PyExc_TypeError,
+                         "_dbm.dbm.setdefault() takes exactly 2 positional arguments (%d given)",
+                         nargs);
+        }
+        goto exit;
+    }
+    if (!PyArg_Parse(args[0], "s#", &key, &key_length)) {
         goto exit;
     }
+    default_value = args[1];
     Py_BEGIN_CRITICAL_SECTION(self);
     return_value = _dbm_dbm_setdefault_impl((dbmobject *)self, cls, key, key_length, default_value);
     Py_END_CRITICAL_SECTION();
@@ -246,4 +251,4 @@ skip_optional:
 exit:
     return return_value;
 }
-/*[clinic end generated code: output=279511ea7cda38dd input=a9049054013a1b77]*/
+/*[clinic end generated code: output=4d9975add7da9d1b input=a9049054013a1b77]*/
diff --git a/Modules/clinic/_gdbmmodule.c.h b/Modules/clinic/_gdbmmodule.c.h
index 6fd6aa3da5..8a1583686f 100644
--- a/Modules/clinic/_gdbmmodule.c.h
+++ b/Modules/clinic/_gdbmmodule.c.h
@@ -43,7 +43,7 @@ exit:
 }

 PyDoc_STRVAR(_gdbm_gdbm_setdefault__doc__,
-"setdefault($self, key, default=None, /)\n"
+"setdefault($self, key, default, /)\n"
 "--\n"
 "\n"
 "Get value for key, or set it to default and return default if not present.");
@@ -60,17 +60,17 @@ _gdbm_gdbm_setdefault(PyObject *self, PyObject *const *args, Py_ssize_t nargs)
 {
     PyObject *return_value = NULL;
     PyObject *key;
-    PyObject *default_value = Py_None;
+    PyObject *default_value;

-    if (!_PyArg_CheckPositional("setdefault", nargs, 1, 2)) {
+    if (nargs != 2) {
+        PyErr_Format(PyExc_TypeError,
+                     "_gdbm.gdbm.setdefault() missing %d required positional argument%s: 'default'",
+                     nargs < 2 ? 2 - nargs : nargs - 2,
+                     nargs < 2 ? (2 - nargs == 1 ? "" : "s") : "s");
         goto exit;
     }
     key = args[0];
-    if (nargs < 2) {
-        goto skip_optional;
-    }
     default_value = args[1];
-skip_optional:
     Py_BEGIN_CRITICAL_SECTION(self);
     return_value = _gdbm_gdbm_setdefault_impl((gdbmobject *)self, key, default_value);
     Py_END_CRITICAL_SECTION();
@@ -389,4 +389,4 @@ skip_optional:
 exit:
     return return_value;
 }
-/*[clinic end generated code: output=8bca34ce9d4493dd input=a9049054013a1b77]*/
+/*[clinic end generated code: output=0847daeab077bbe0 input=a9049054013a1b77]*/

[serhiy-storchaka]

Thank you for your PR. Please add more tests for setdefault().

• with non-existing key (this case is currently tested in the form self.assertEqual(f.setdefault(b'xxx', b'foo'), b'foo'))
• with existing key
• with wrong number of arguments (with non-existing and existing key)
• with wrong type of default (with non-existing and existing key)
• with read-only DB (with non-existing and existing key)

[serhiy-storchaka]

This can be optimized to two _execute() if inline __getitem__() and __setitem__(). Could it be done with a single _execute()? cc @erlend-aasland