From 253a7c05dff89f584a2bddcd75abd25ec57ab28f Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manishsmail@gmail.com>
Date: Tue, 3 Sep 2024 17:49:09 -0700
Subject: [PATCH] Fix Dart default ctor generation code around Options (#676)

---
 core/src/hir/types.rs                         |  8 ++++++--
 .../dart/lib/src/OptionInputStruct.g.dart     | 16 +++++++++++++--
 feature_tests/src/option.rs                   | 14 +++++++++++++
 tool/src/dart/mod.rs                          | 20 +++++++++++++++----
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/core/src/hir/types.rs b/core/src/hir/types.rs
index 7b9a962f7..71ed6a811 100644
--- a/core/src/hir/types.rs
+++ b/core/src/hir/types.rs
@@ -149,9 +149,13 @@ impl<P: TyPosition> Type<P> {
         }
     }
 
-    /// Whether this type is a `DiplomatOption`
+    /// Whether this type is a `DiplomatOption` or optional Opaque
     pub fn is_option(&self) -> bool {
-        matches!(self, Self::DiplomatOption(..))
+        match self {
+            Self::DiplomatOption(..) => true,
+            Self::Opaque(ref o) if o.is_optional() => true,
+            _ => false,
+        }
     }
 }
 
diff --git a/feature_tests/dart/lib/src/OptionInputStruct.g.dart b/feature_tests/dart/lib/src/OptionInputStruct.g.dart
index e76c9f912..7378bb798 100644
--- a/feature_tests/dart/lib/src/OptionInputStruct.g.dart
+++ b/feature_tests/dart/lib/src/OptionInputStruct.g.dart
@@ -13,8 +13,6 @@ final class OptionInputStruct {
   Rune? b;
   OptionEnum? c;
 
-  OptionInputStruct({required this.a, required this.b, required this.c});
-
   // This struct contains borrowed fields, so this takes in a list of
   // "edges" corresponding to where each lifetime's data may have been borrowed from
   // and passes it down to individual fields containing the borrow.
@@ -38,6 +36,15 @@ final class OptionInputStruct {
     return struct;
   }
 
+  factory OptionInputStruct({int? a, Rune? b, OptionEnum? c}) {
+    final result = _OptionInputStruct_default_ctor();
+    final dart = OptionInputStruct._fromFfi(result);
+    dart.a = a;
+    dart.b = b;
+    dart.c = c;
+    return dart;
+  }
+
   @override
   bool operator ==(Object other) =>
       other is OptionInputStruct &&
@@ -52,3 +59,8 @@ final class OptionInputStruct {
         c,
       ]);
 }
+
+@meta.ResourceIdentifier('OptionInputStruct_default_ctor')
+@ffi.Native<_OptionInputStructFfi Function()>(isLeaf: true, symbol: 'OptionInputStruct_default_ctor')
+// ignore: non_constant_identifier_names
+external _OptionInputStructFfi _OptionInputStruct_default_ctor();
diff --git a/feature_tests/src/option.rs b/feature_tests/src/option.rs
index b688017cb..9f9e7ea9a 100644
--- a/feature_tests/src/option.rs
+++ b/feature_tests/src/option.rs
@@ -45,6 +45,20 @@ pub mod ffi {
         c: DiplomatOption<OptionEnum>,
     }
 
+    impl OptionInputStruct {
+        // Specifically test the Dart default constructor generation code
+        // around Options
+        #[diplomat::attr(not(dart), disable)]
+        #[diplomat::attr(auto, constructor)]
+        pub fn default_ctor() -> Self {
+            Self {
+                a: None.into(),
+                b: None.into(),
+                c: None.into(),
+            }
+        }
+    }
+
     #[diplomat::attr(not(supports = option), disable)]
     #[derive(Debug)]
     pub enum OptionEnum {
diff --git a/tool/src/dart/mod.rs b/tool/src/dart/mod.rs
index df1acd220..16ef9ca92 100644
--- a/tool/src/dart/mod.rs
+++ b/tool/src/dart/mod.rs
@@ -338,7 +338,15 @@ impl<'a, 'cx> TyGenContext<'a, 'cx> {
                 // If there's an existing zero-arg constructor, we repurpose it with optional arguments for all fields
                 let args = fields
                     .iter()
-                    .map(|field| format!("{}? {}", field.dart_type_name, field.name))
+                    .map(|field| {
+                        let question_mark = if field.ty.is_option() {
+                            // If it's optional we don't need to double-wrap in `?`
+                            ""
+                        } else {
+                            "?"
+                        };
+                        format!("{}{question_mark} {}", field.dart_type_name, field.name)
+                    })
                     .collect::<Vec<_>>();
                 constructor.declaration =
                     format!("factory {type_name}({{{args}}})", args = args.join(", "));
@@ -347,9 +355,13 @@ impl<'a, 'cx> TyGenContext<'a, 'cx> {
                 writeln!(&mut r, "final dart = {type_name}._fromFfi(result);").unwrap();
                 for field in &fields {
                     let name = &field.name;
-                    writeln!(&mut r, "if ({name} != null) {{").unwrap();
-                    writeln!(&mut r, "  dart.{name} = {name};").unwrap();
-                    writeln!(&mut r, "}}").unwrap();
+                    if field.ty.is_option() {
+                        writeln!(&mut r, "dart.{name} = {name};").unwrap();
+                    } else {
+                        writeln!(&mut r, "if ({name} != null) {{").unwrap();
+                        writeln!(&mut r, "  dart.{name} = {name};").unwrap();
+                        writeln!(&mut r, "}}").unwrap();
+                    }
                 }
                 write!(&mut r, "return dart;").unwrap();
                 constructor.return_expression = Some(r.into());