[gjs/ewlsh/optional-arguments: 10/10] gi: Allow undefined for optional arguments

[Evan Welsh <ewlsh src gnome org>]

commit d7cb2f1e3f85ff77a335ed67b98e188ffe833e35
Author: Evan Welsh <contact evanwelsh com>
Date:   Sat Nov 27 22:10:38 2021 -0800

    gi: Allow undefined for optional arguments

 gi/arg-cache.cpp                     | 54 ++++++++++++++++++++++++++++++++++--
 gi/arg-cache.h                       | 10 +++++--
 gi/arg.h                             |  1 +
 gi/function.cpp                      | 30 +++++++++++++++-----
 installed-tests/js/testExceptions.js |  2 +-
 installed-tests/js/testRegress.js    |  2 +-
 6 files changed, 85 insertions(+), 14 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 9fc02613..b4f76196 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -139,6 +139,12 @@ static bool report_invalid_null(JSContext* cx, const char* arg_name) {
     return false;
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool report_invalid_undefined(JSContext* cx, const char* arg_name) {
+    gjs_throw(cx, "Argument %s is required", arg_name);
+    return false;
+}
+
 // Marshallers:
 //
 // Each argument, irrespective of the direction, is processed in three phases:
@@ -272,7 +278,11 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
     GjsCallbackTrampoline* trampoline;
     ffi_closure* closure;
 
-    if (value.isNull() && (self->flags & GjsArgumentFlags::MAY_BE_NULL)) {
+    if (value.isUndefined() && (self->flags & GjsArgumentFlags::IS_OPTIONAL)) {
+        closure = nullptr;
+        trampoline = nullptr;
+    } else if (value.isNull() &&
+               (self->flags & GjsArgumentFlags::MAY_BE_NULL)) {
         closure = nullptr;
         trampoline = nullptr;
     } else {
@@ -443,6 +453,10 @@ static bool gjs_marshal_gtype_in_in(JSContext* cx, GjsArgumentCache* self,
                                     JS::HandleValue value) {
     if (value.isNull())
         return report_invalid_null(cx, self->arg_name);
+
+    if (value.isUndefined())
+        return report_invalid_undefined(cx, self->arg_name);
+
     if (!value.isObject())
         return report_typeof_mismatch(cx, self->arg_name, value,
                                       ExpectedType::OBJECT);
@@ -460,6 +474,13 @@ bool GjsArgumentCache::handle_nullable(JSContext* cx, GIArgument* arg) {
     return true;
 }
 
+bool GjsArgumentCache::handle_optional(JSContext* cx, GIArgument* arg) {
+    if (!(flags & GjsArgumentFlags::IS_OPTIONAL))
+        return report_invalid_undefined(cx, arg_name);
+    gjs_arg_unset<void*>(arg);
+    return true;
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_string_in_in(JSContext* cx, GjsArgumentCache* self,
                                      GjsFunctionCallState*, GIArgument* arg,
@@ -467,6 +488,9 @@ static bool gjs_marshal_string_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     if (!value.isString())
         return report_typeof_mismatch(cx, self->arg_name, value,
                                       ExpectedType::STRING);
@@ -586,6 +610,9 @@ static bool gjs_marshal_boxed_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     GType gtype = self->contents.object.gtype;
 
     if (!value.isObject())
@@ -612,6 +639,9 @@ static bool gjs_marshal_union_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
@@ -631,6 +661,9 @@ static bool gjs_marshal_gclosure_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     if (!(JS_TypeOfValue(cx, value) == JSTYPE_FUNCTION))
         return report_typeof_mismatch(cx, self->arg_name, value,
                                       ExpectedType::FUNCTION);
@@ -651,6 +684,9 @@ static bool gjs_marshal_gbytes_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     if (!value.isObject())
         return report_gtype_mismatch(cx, self->arg_name, value, G_TYPE_BYTES);
 
@@ -674,6 +710,9 @@ static bool gjs_marshal_interface_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
@@ -703,6 +742,9 @@ static bool gjs_marshal_object_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
@@ -722,6 +764,9 @@ static bool gjs_marshal_fundamental_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isUndefined())
+        return self->handle_optional(cx, arg);
+
     GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
@@ -1622,7 +1667,8 @@ void gjs_arg_cache_build_instance(GjsArgumentCache* self,
 void gjs_arg_cache_build_arg(GjsArgumentCache* self,
                              GjsArgumentCache* arguments, uint8_t gi_index,
                              GIDirection direction, GIArgInfo* arg,
-                             GICallableInfo* callable, bool* inc_counter_out) {
+                             GICallableInfo* callable, bool* inc_counter_out,
+                             bool* is_optional) {
     g_assert(inc_counter_out && "forgot out parameter");
 
     self->set_arg_pos(gi_index);
@@ -1631,8 +1677,10 @@ void gjs_arg_cache_build_arg(GjsArgumentCache* self,
     self->transfer = g_arg_info_get_ownership_transfer(arg);
 
     GjsArgumentFlags flags = GjsArgumentFlags::NONE;
-    if (g_arg_info_may_be_null(arg))
+    if (g_arg_info_may_be_null(arg)) {
         flags |= GjsArgumentFlags::MAY_BE_NULL;
+        *is_optional = true;
+    }
     if (g_arg_info_is_caller_allocates(arg))
         flags |= GjsArgumentFlags::CALLER_ALLOCATES;
 
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 98c620b1..5b865551 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -56,7 +56,7 @@ struct GjsArgumentCache {
 
     uint8_t arg_pos;
     GITransfer transfer : 2;
-    GjsArgumentFlags flags : 5;
+    GjsArgumentFlags flags : 6;
 
     union {
         // for explicit array only
@@ -101,6 +101,9 @@ struct GjsArgumentCache {
     GJS_JSAPI_RETURN_CONVENTION
     bool handle_nullable(JSContext* cx, GIArgument* arg);
 
+    GJS_JSAPI_RETURN_CONVENTION
+    bool handle_optional(JSContext* cx, GIArgument* arg);
+
     // Introspected functions can have up to 253 arguments. 255 is a placeholder
     // for the return value and 254 for the instance parameter. The callback
     // closure or destroy notify parameter may have a value of 255 to indicate
@@ -140,6 +143,8 @@ struct GjsArgumentCache {
         flags = static_cast<GjsArgumentFlags>(GjsArgumentFlags::NONE | GjsArgumentFlags::SKIP_OUT);
     }
 
+    void set_optional() { flags = flags | GjsArgumentFlags::IS_OPTIONAL; }
+
     void set_return_value() {
         arg_pos = RETURN_VALUE;
         arg_name = "return value";
@@ -175,7 +180,8 @@ static_assert(sizeof(GjsArgumentCache) <= 112,
 void gjs_arg_cache_build_arg(GjsArgumentCache* self,
                              GjsArgumentCache* arguments, uint8_t gi_index,
                              GIDirection direction, GIArgInfo* arg,
-                             GICallableInfo* callable, bool* inc_counter_out);
+                             GICallableInfo* callable, bool* inc_counter_out,
+                             bool* is_optional);
 
 void gjs_arg_cache_build_return(GjsArgumentCache* self,
                                 GjsArgumentCache* arguments,
diff --git a/gi/arg.h b/gi/arg.h
index c5c81760..cb4015b6 100644
--- a/gi/arg.h
+++ b/gi/arg.h
@@ -38,6 +38,7 @@ enum class GjsArgumentFlags : uint8_t {
     SKIP_ALL = SKIP_IN | SKIP_OUT,
     FILENAME = 1 << 4,  //  Sharing the bit with UNSIGNED, used only for strings
     UNSIGNED = 1 << 4,  //  Sharing the bit with FILENAME, used only for enums
+    IS_OPTIONAL = 1 << 5,
 };
 
 [[nodiscard]] char* gjs_argument_display_name(const char* arg_name,
diff --git a/gi/function.cpp b/gi/function.cpp
index acbe3a7b..a3a4768c 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -68,6 +68,7 @@ class Function : public CWrapper<Function> {
     GjsArgumentCache* m_arguments;
 
     uint8_t m_js_in_argc;
+    uint8_t m_js_in_optional_argc;
     uint8_t m_js_out_argc;
     GIFunctionInvoker m_invoker;
 
@@ -75,6 +76,7 @@ class Function : public CWrapper<Function> {
         : m_info(info, GjsAutoTakeOwnership()),
           m_arguments(nullptr),
           m_js_in_argc(0),
+          m_js_in_optional_argc(0),
           m_js_out_argc(0),
           m_invoker({}) {
         GJS_INC_COUNTER(function);
@@ -816,16 +818,18 @@ bool Function::invoke(JSContext* context, const JS::CallArgs& args,
     // state.gi_argc is the number of arguments the GICallableInfo describes
     // (which does not include "this" or GError**). m_js_in_argc is the number
     // of arguments we expect the JS function to take (which does not include
-    // PARAM_SKIPPED args).
-    // args.length() is the number of arguments that were actually passed.
+    // PARAM_SKIPPED args). m_js_in_optional_argc is the minimum number of
+    // arguments we expect the JS function to take (not including PARAM_SKIPPED
+    // and ALLOW_NONE args) args.length() is the number of arguments that were
+    // actually passed.
     if (args.length() > m_js_in_argc) {
         if (!JS::WarnUTF8(context,
                           "Too many arguments to %s: expected %u, got %u",
                           format_name().c_str(), m_js_in_argc, args.length()))
             return false;
-    } else if (args.length() < m_js_in_argc) {
-        args.reportMoreArgsNeeded(context, format_name().c_str(), m_js_in_argc,
-                                  args.length());
+    } else if (args.length() < m_js_in_optional_argc) {
+        args.reportMoreArgsNeeded(context, format_name().c_str(),
+                                  m_js_in_optional_argc, args.length());
         return false;
     }
 
@@ -1262,6 +1266,8 @@ bool Function::init(JSContext* context, GType gtype /* = G_TYPE_NONE */) {
     if (inc_counter)
         m_js_out_argc++;
 
+    int in_argc = m_js_in_argc;
+    int in_optional_argc = m_js_in_argc;
     for (i = 0; i < n_args; i++) {
         GIDirection direction;
         GIArgInfo arg_info;
@@ -1272,8 +1278,9 @@ bool Function::init(JSContext* context, GType gtype /* = G_TYPE_NONE */) {
         g_callable_info_load_arg(m_info, i, &arg_info);
         direction = g_arg_info_get_direction(&arg_info);
 
+        bool is_optional = false;
         gjs_arg_cache_build_arg(&m_arguments[i], m_arguments, i, direction,
-                                &arg_info, m_info, &inc_counter);
+                                &arg_info, m_info, &inc_counter, &is_optional);
 
         if (inc_counter) {
             switch (direction) {
@@ -1281,7 +1288,7 @@ bool Function::init(JSContext* context, GType gtype /* = G_TYPE_NONE */) {
                     m_js_out_argc++;
                     [[fallthrough]];
                 case GI_DIRECTION_IN:
-                    m_js_in_argc++;
+                    in_argc++;
                     break;
                 case GI_DIRECTION_OUT:
                     m_js_out_argc++;
@@ -1289,9 +1296,18 @@ bool Function::init(JSContext* context, GType gtype /* = G_TYPE_NONE */) {
                 default:
                     g_assert_not_reached();
             }
+
+            if (!is_optional)
+                in_optional_argc = in_argc;
         }
     }
 
+    m_js_in_argc = in_argc;
+    m_js_in_optional_argc = in_optional_argc;
+
+    for (i = m_js_in_optional_argc; i < m_js_in_argc; i++)
+        m_arguments[i].set_optional();
+
     return true;
 }
 
diff --git a/installed-tests/js/testExceptions.js b/installed-tests/js/testExceptions.js
index fbc32bc2..823b9bb4 100644
--- a/installed-tests/js/testExceptions.js
+++ b/installed-tests/js/testExceptions.js
@@ -176,7 +176,7 @@ describe('Exception from function with too few arguments', function () {
 
     it('contains the full method name', function () {
         let file = Gio.File.new_for_path('foo');
-        expect(() => file.read()).toThrowError(/Gio\.File\.read/);
+        expect(() => file.get_child()).toThrowError(/Gio\.File\.get_child/);
     });
 });
 
diff --git a/installed-tests/js/testRegress.js b/installed-tests/js/testRegress.js
index 094d4150..02765f99 100644
--- a/installed-tests/js/testRegress.js
+++ b/installed-tests/js/testRegress.js
@@ -1424,7 +1424,7 @@ describe('Life, the Universe and Everything', function () {
 
     it('null / undefined callback', function () {
         expect(Regress.test_callback(null)).toEqual(0);
-        expect(() => Regress.test_callback(undefined)).toThrow();
+        expect(Regress.test_callback(undefined)).toEqual(0);
     });
 
     it('callback called more than once', function () {