From 471cbf3ae0bcad6615f3e82bf1d5e7ee483e18a4 Mon Sep 17 00:00:00 2001
From: Brendan Dahl <brendan.dahl@gmail.com>
Date: Thu, 18 Apr 2024 21:20:22 +0000
Subject: [PATCH] [tsgen] Use optional params for trailing std::optional
 arguments.

This replaces `_0: Foo | undefined` with `_0?: Foo` for any optional
arguments at the end of function, which allows for more idiomatic TS.

Fixes #21776
---
 src/embind/embind_gen.js              | 20 +++++++++++++++++---
 test/other/embind_tsgen.cpp           |  5 +++++
 test/other/embind_tsgen.d.ts          |  3 ++-
 test/other/embind_tsgen_ignore_1.d.ts |  3 ++-
 test/other/embind_tsgen_ignore_2.d.ts |  3 ++-
 test/other/embind_tsgen_ignore_3.d.ts |  3 ++-
 6 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/embind/embind_gen.js b/src/embind/embind_gen.js
index 503bc4bac5c3..876f431de61a 100644
--- a/src/embind/embind_gen.js
+++ b/src/embind/embind_gen.js
@@ -55,11 +55,25 @@ var LibraryEmbind = {
 
     printSignature(nameMap, out) {
       out.push('(');
-
       const argOut = [];
-      for (const arg of this.argumentTypes) {
-        argOut.push(`${arg.name}: ${nameMap(arg.type)}`);
+      // Work backwards on the arguments, so optional types can be replaced
+      // with TS optional params until we see the first non-optional argument.
+      let seenNonOptional = false;
+      for (let i = this.argumentTypes.length - 1; i >= 0; i--) {
+        const arg = this.argumentTypes[i];
+        let argType;
+        let argName;
+        if (arg.type instanceof OptionalType && !seenNonOptional) {
+          argType = nameMap(arg.type.type);
+          argName = arg.name + '?';
+        } else {
+          seenNonOptional = true;
+          argType = nameMap(arg.type);
+          argName = arg.name;
+        }
+        argOut.unshift(`${argName}: ${argType}`);
       }
+
       out.push(argOut.join(', '));
       out.push(`): ${nameMap(this.returnType, true)}`);
     }
diff --git a/test/other/embind_tsgen.cpp b/test/other/embind_tsgen.cpp
index d85024c62c28..122c5785354d 100644
--- a/test/other/embind_tsgen.cpp
+++ b/test/other/embind_tsgen.cpp
@@ -102,6 +102,10 @@ std::optional<int> optional_test(std::optional<Foo> arg) {
   return {};
 }
 
+std::optional<int> optional_and_nonoptional_test(std::optional<Foo> arg1, int arg2) {
+  return {};
+}
+
 class BaseClass {
  public:
   virtual ~BaseClass() = default;
@@ -174,6 +178,7 @@ EMSCRIPTEN_BINDINGS(Test) {
   register_optional<int>();
   register_optional<Foo>();
   function("optional_test", &optional_test);
+  function("optional_and_nonoptional_test", &optional_and_nonoptional_test);
 
   function("string_test", &string_test);
   function("wstring_test", &wstring_test);
diff --git a/test/other/embind_tsgen.d.ts b/test/other/embind_tsgen.d.ts
index bbf7999e7f58..7cb0001be057 100644
--- a/test/other/embind_tsgen.d.ts
+++ b/test/other/embind_tsgen.d.ts
@@ -114,8 +114,9 @@ interface EmbindModule {
   DerivedClass: {};
   a_bool: boolean;
   an_int: number;
-  optional_test(_0: Foo | undefined): number | undefined;
+  optional_test(_0?: Foo): number | undefined;
   global_fn(_0: number, _1: number): number;
+  optional_and_nonoptional_test(_0: Foo | undefined, _1: number): number | undefined;
   smart_ptr_function(_0: ClassWithSmartPtrConstructor): number;
   smart_ptr_function_with_params(foo: ClassWithSmartPtrConstructor): number;
   function_with_callback_param(_0: (message: string) => void): number;
diff --git a/test/other/embind_tsgen_ignore_1.d.ts b/test/other/embind_tsgen_ignore_1.d.ts
index befb5985963e..54e2e6e60c35 100644
--- a/test/other/embind_tsgen_ignore_1.d.ts
+++ b/test/other/embind_tsgen_ignore_1.d.ts
@@ -123,8 +123,9 @@ interface EmbindModule {
   DerivedClass: {};
   a_bool: boolean;
   an_int: number;
-  optional_test(_0: Foo | undefined): number | undefined;
+  optional_test(_0?: Foo): number | undefined;
   global_fn(_0: number, _1: number): number;
+  optional_and_nonoptional_test(_0: Foo | undefined, _1: number): number | undefined;
   smart_ptr_function(_0: ClassWithSmartPtrConstructor): number;
   smart_ptr_function_with_params(foo: ClassWithSmartPtrConstructor): number;
   function_with_callback_param(_0: (message: string) => void): number;
diff --git a/test/other/embind_tsgen_ignore_2.d.ts b/test/other/embind_tsgen_ignore_2.d.ts
index b2d988d5fef2..2ed8cab9ffbe 100644
--- a/test/other/embind_tsgen_ignore_2.d.ts
+++ b/test/other/embind_tsgen_ignore_2.d.ts
@@ -101,8 +101,9 @@ interface EmbindModule {
   DerivedClass: {};
   a_bool: boolean;
   an_int: number;
-  optional_test(_0: Foo | undefined): number | undefined;
+  optional_test(_0?: Foo): number | undefined;
   global_fn(_0: number, _1: number): number;
+  optional_and_nonoptional_test(_0: Foo | undefined, _1: number): number | undefined;
   smart_ptr_function(_0: ClassWithSmartPtrConstructor): number;
   smart_ptr_function_with_params(foo: ClassWithSmartPtrConstructor): number;
   function_with_callback_param(_0: (message: string) => void): number;
diff --git a/test/other/embind_tsgen_ignore_3.d.ts b/test/other/embind_tsgen_ignore_3.d.ts
index bbf7999e7f58..7cb0001be057 100644
--- a/test/other/embind_tsgen_ignore_3.d.ts
+++ b/test/other/embind_tsgen_ignore_3.d.ts
@@ -114,8 +114,9 @@ interface EmbindModule {
   DerivedClass: {};
   a_bool: boolean;
   an_int: number;
-  optional_test(_0: Foo | undefined): number | undefined;
+  optional_test(_0?: Foo): number | undefined;
   global_fn(_0: number, _1: number): number;
+  optional_and_nonoptional_test(_0: Foo | undefined, _1: number): number | undefined;
   smart_ptr_function(_0: ClassWithSmartPtrConstructor): number;
   smart_ptr_function_with_params(foo: ClassWithSmartPtrConstructor): number;
   function_with_callback_param(_0: (message: string) => void): number;