Support Optionals as Optional Parameters in the Constructor

README.md

@@ -67,6 +67,8 @@ verify the build and binary from the command line.
  - DataView for copy free data passing
  - DateRef for copy free data passing with ownership
  - Generating string names for C++ enums
+ - Omit optional parameters from record constructors
+ - Records are default mutable across all platforms
  - Bug fixes
 
 ## Using new features
@@ -341,6 +343,38 @@ The C++ `Future` type has optional support for coroutines. If coroutines are
 availble (eg. compiling with C++20 or C++17 with -fcoroutines-ts), then you can
 use `co_await` on future objects.
 
+## Omitting Optionals from Record Constructors
+- By default, optionals will be omitted from the constructor in Djinni
+- The Djinni code generator will generate two constructors for a record with optionals: one with all values and one with optionals omitted. This will minimize code disruption
+- Optional behavior will be able to be switched via a usage flag for each platform. 
+- A new [deriving method](https://github.com/dropbox/djinni#derived-methods) specifier will be implemented so that individual records can still require all parameters
+
+### Deriving Record
+Any record can be made to have all parameters be required by specifying it as a `req` deriving record:
+```
+MyClass = record {
+  required: string;
+  optional: optional<string>;
+} deriving(req)
+```
+
+### Omitting Convenience Constructors
+Extra convenience constructors which require all parameters can be removed from optional ObjC records with a new compiler flag:
+```
+--objc-omit-full-convenience-constructor
+```
+
+## Reverting to Legacy Record Behavior
+Djinni records are now mutable and do not require optionals in reocrd constructors by default. In order to reverse this behavior and make Java and ObjC records immutable with full constructors only, the following generation flags can be used:
+
+```
+--cpp-legacy-records
+--java-legacy-records
+--objc-legacy-records
+```
+
+Note that for C++, the legacy flag will only remove the optional-omitting constructor. Records were already mutable within C++.
+
 ## FAQ
 
 Q. Do I need to use Bazel to build my project?

src/source/CppGenerator.scala

@@ -221,6 +221,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
       writeCppTypeParams(w, params)
       w.w("struct " + actualSelf + cppFinal).bracedSemi {
         generateHppConstants(w, r.consts)
+
         // Field definitions.
         for (f <- r.fields) {
           writeDoc(w, f.doc)
@@ -243,18 +244,48 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
           w.wl(s"friend bool operator>=(const $actualSelf& lhs, const $actualSelf& rhs);")
         }
 
-        // Constructor.
-        if(r.fields.nonEmpty) {
-          w.wl
-          if (r.fields.size == 1) {
-            w.wl("//NOLINTNEXTLINE(google-explicit-constructor)")
+        // Constructor generator
+        def writeConstructor(reqFields: Seq[Field])  {
+          // Only skip if there are no fields at all. If only optional
+          // fields exist, we should still create the trivial constructor
+          if(r.fields.nonEmpty) {
+            w.wl
+            if (reqFields.size == 1) {
+              w.wl("//NOLINTNEXTLINE(google-explicit-constructor)")
+            }
+            writeAlignedCall(w, actualSelf + "(", reqFields, ")", f => marshal.fieldType(f.ty) + " " + idCpp.local(f.ident) + "_")
+            w.wl
+
+            // Determimne if we are writing a constructor omitting optionals or one requiring
+            // all parameters
+            if (r.fields.size != reqFields.size) {
+              writeAlignedCall(w, ": " + actualSelf + "(", r.fields, ")", f => {
+                var param = "std::move(" + idCpp.local(f.ident) + "_)"
+                if (isOptional(f.ty.resolved))
+                  if (isInterface(f.ty.resolved.args.head)) 
+                    param = s"nullptr" 
+                  else 
+                    param = s"${spec.cppNulloptValue}"
+
+                param
+              })
+              w.wl
+            } else {
+              // required constructor
+              val init = (f: Field) => idCpp.field(f.ident) + "(std::move(" + idCpp.local(f.ident) + "_))"
+              w.wl(": " + init(r.fields.head))
+              r.fields.tail.map(f => ", " + init(f)).foreach(w.wl)
+            }
+            w.wl("{}")
           }
-          writeAlignedCall(w, actualSelf + "(", r.fields, ")", f => marshal.fieldType(f.ty) + " " + idCpp.local(f.ident) + "_")
-          w.wl
-          val init = (f: Field) => idCpp.field(f.ident) + "(std::move(" + idCpp.local(f.ident) + "_))"
-          w.wl(": " + init(r.fields.head))
-          r.fields.tail.map(f => ", " + init(f)).foreach(w.wl)
-          w.wl("{}")
+        }
+
+        // First write constructor requiring optionals, required for marshaling
+        writeConstructor(r.fields)
+
+        // Next write optional omitting constructor if necessary
+        if (!spec.cppLegacyRecords && !r.derivingTypes.contains(DerivingType.Req) && r.reqFields.size != r.fields.size) {
+          writeConstructor(r.reqFields)
         }
 
         if (r.ext.cpp) {

src/source/JavaGenerator.scala

@@ -275,27 +275,46 @@ class JavaGenerator(spec: Spec) extends Generator(spec) {
         generateJavaConstants(w, r.consts)
         // Field definitions.
         for (f <- r.fields) {
+          var fieldFinal = if (spec.javaLegacyRecords) "final " else if (r.derivingTypes.contains(DerivingType.Req) || !isOptional(f.ty.resolved)) "" else "/*optional*/ "
           w.wl
-          w.wl(s"/*package*/ final ${marshal.fieldType(f.ty)} ${idJava.field(f.ident)};")
+          w.wl(s"/*package*/ ${fieldFinal}${marshal.fieldType(f.ty)} ${idJava.field(f.ident)};")
         }
 
-        // Constructor.
-        w.wl
-        w.wl(s"public $self(").nestedN(2) {
-          val skipFirst = SkipFirst()
-          for (f <- r.fields) {
-            skipFirst { w.wl(",") }
-            marshal.nullityAnnotation(f.ty).map(annotation => w.w(annotation + " "))
-            w.w(marshal.paramType(f.ty) + " " + idJava.local(f.ident))
+        def writeConstructor(reqFields: Seq[Field]) {
+          w.wl
+          if (reqFields.isEmpty)
+            w.wl(s"public $self() {")
+          else
+            w.wl(s"public $self(").nestedN(2) {
+              val skipFirst = SkipFirst()
+              for (f <- reqFields) {
+                skipFirst { w.wl(",") }
+                marshal.nullityAnnotation(f.ty).map(annotation => w.w(annotation + " "))
+                w.w(marshal.paramType(f.ty) + " " + idJava.local(f.ident))
+              }
+              w.wl(") {")
+            }
+          w.nested {
+            // Optional constructor or full constructor
+            if (reqFields.size != r.fields.size) {
+              writeAlignedCall(w, "this(", r.fields, ");", f=> if (isOptional(f.ty.resolved)) "null" else idJava.local(f.ident))
+              w.wl
+            } else {
+              for (f <- reqFields) {
+                w.wl(s"this.${idJava.field(f.ident)} = ${idJava.local(f.ident)};")
+              }
+            }
           }
-          w.wl(") {")
+          w.wl("}")
         }
-        w.nested {
-          for (f <- r.fields) {
-            w.wl(s"this.${idJava.field(f.ident)} = ${idJava.local(f.ident)};")
-          }
+
+        // Constructor, requiring all parameters, used for marshaling
+        writeConstructor(r.fields)
+
+        // Constructor, not requiring optionals, generated if necessary
+        if (!spec.javaLegacyRecords && !r.derivingTypes.contains(DerivingType.Req) && r.fields.size != r.reqFields.size) {
+          writeConstructor(r.reqFields)
         }
-        w.wl("}")
 
         // Accessors
         for (f <- r.fields) {
@@ -305,6 +324,13 @@ class JavaGenerator(spec: Spec) extends Generator(spec) {
           w.w("public " + marshal.returnType(Some(f.ty)) + " " + idJava.method("get_" + f.ident.name) + "()").braced {
             w.wl("return " + idJava.field(f.ident) + ";")
           }
+
+          if (!spec.javaLegacyRecords) {
+            w.wl
+            w.w("public void " + idJava.method("set_" + f.ident.name) + "(" + marshal.paramType(f.ty) + " " + idJava.local(f.ident) + ")").braced {
+              w.wl(s"this.${idJava.field(f.ident)} = ${idJava.local(f.ident)};")
+            }
+          }
         }
 
         if (r.derivingTypes.contains(DerivingType.Eq)) {

src/source/Main.scala

@@ -35,11 +35,13 @@ object Main {
     var cppBaseLibIncludePrefix: String = ""
     var cppOptionalTemplate: String = "std::optional"
     var cppOptionalHeader: String = "<optional>"
+    var cppNulloptValue: String = "std::nullopt"
     var cppEnumHashWorkaround : Boolean = true
     var cppNnHeader: Option[String] = None
     var cppNnType: Option[String] = None
     var cppNnCheckExpression: Option[String] = None
     var cppUseWideStrings: Boolean = false
+    var cppLegacyRecords: Boolean = false
     var javaOutFolder: Option[File] = None
     var javaPackage: Option[String] = None
     var javaClassAccessModifier: JavaAccessModifier.Value = JavaAccessModifier.Public
@@ -49,6 +51,7 @@ object Main {
     var javaNonnullAnnotation: Option[String] = None
     var javaImplementAndroidOsParcelable : Boolean = false
     var javaUseFinalForRecord: Boolean = true
+    var javaLegacyRecords: Boolean = false
     var javaGenInterface: Boolean = false
     var jniOutFolder: Option[File] = None
     var jniHeaderOutFolderOptional: Option[File] = None
@@ -86,6 +89,8 @@ object Main {
     var objcppDisableExceptionTranslation: Boolean = false
     var objcFileIdentStyleOptional: Option[IdentConverter] = None
     var objcStrictProtocol: Boolean = true
+    var objcLegacyRecords: Boolean = false
+    var objcOmitFullConvenienceConstructor: Boolean = false
     var objcppNamespace: String = "djinni_generated"
     var objcBaseLibIncludePrefix: String = ""
     var wasmOutFolder: Option[File] = None
@@ -141,6 +146,8 @@ object Main {
         .text("all generated java classes will implement the interface android.os.Parcelable")
       opt[Boolean]("java-use-final-for-record").valueName("<use-final-for-record>").foreach(x => javaUseFinalForRecord = x)
         .text("Whether generated Java classes for records should be marked 'final' (default: true). ")
+      opt[Boolean]("java-legacy-records").valueName("<legacy-records>").foreach(x => javaLegacyRecords = x)
+        .text("Use legacy record behavior for Java code (default: false)")
       opt[Boolean]("java-gen-interface").valueName("<true/false>").foreach(x => javaGenInterface = x)
         .text("Generate Java interface instead of abstract class.")
       note("")
@@ -162,6 +169,8 @@ object Main {
         .text("The template to use for optional values (default: \"std::optional\")")
       opt[String]("cpp-optional-header").valueName("<header>").foreach(x => cppOptionalHeader = x)
         .text("The header to use for optional values (default: \"<optional>\")")
+      opt[String]("cpp-nullopt-value").valueName("<value>").foreach(x => cppNulloptValue = x)
+        .text("The value to use for nullopt defaults of optional values (default: \"std::nullopt\")")
       opt[Boolean]("cpp-enum-hash-workaround").valueName("<true/false>").foreach(x => cppEnumHashWorkaround = x)
         .text("Work around LWG-2148 by generating std::hash specializations for C++ enums (default: true)")
       opt[String]("cpp-nn-header").valueName("<header>").foreach(x => cppNnHeader = Some(x))
@@ -172,6 +181,8 @@ object Main {
         .text("The expression to use for building non-nullable pointers")
       opt[Boolean]( "cpp-use-wide-strings").valueName("<true/false>").foreach(x => cppUseWideStrings = x)
         .text("Use wide strings in C++ code (default: false)")
+      opt[Boolean]( "cpp-legacy-records").valueName("<true/false>").foreach(x => cppLegacyRecords = x)
+        .text("Use legacy record behavior for C++ code (default: false)")
       note("")
       opt[File]("jni-out").valueName("<out-folder>").foreach(x => jniOutFolder = Some(x))
         .text("The folder for the JNI C++ output files (Generator disabled if unspecified).")
@@ -209,7 +220,13 @@ object Main {
       opt[Boolean]("objc-strict-protocols")
         .valueName("<true/false>").foreach(x => objcStrictProtocol = x)
         .text("All generated @protocol will implement <NSObject> (default: true). ")
+      opt[Boolean]("objc-legacy-records")
+        .valueName("<true/false>").foreach(x => objcLegacyRecords = x)
+        .text("Use legacy record behavior for ObjC code (default: false)")
       note("")
+      opt[Boolean]("objc-omit-full-convenience-constructor")
+        .valueName("<omit-full-constructor>").foreach(x => objcOmitFullConvenienceConstructor = x)
+        .text("Skips generation of the convenience constructor requiring all record parameters, if possible (default: false)")
       opt[File]("objcpp-out").valueName("<out-folder>").foreach(x => objcppOutFolder = Some(x))
         .text("The output folder for private Objective-C++ files (Generator disabled if unspecified).")
       opt[String]("objcpp-ext").valueName("<ext>").foreach(objcppExt = _)
@@ -377,6 +394,7 @@ object Main {
       javaImplementAndroidOsParcelable,
       javaUseFinalForRecord,
       javaGenInterface,
+      javaLegacyRecords,
       cppOutFolder,
       cppHeaderOutFolder,
       cppIncludePrefix,
@@ -387,11 +405,13 @@ object Main {
       cppBaseLibIncludePrefix,
       cppOptionalTemplate,
       cppOptionalHeader,
+      cppNulloptValue,
       cppEnumHashWorkaround,
       cppNnHeader,
       cppNnType,
       cppNnCheckExpression,
       cppUseWideStrings,
+      cppLegacyRecords,
       jniOutFolder,
       jniHeaderOutFolder,
       jniIncludePrefix,
@@ -425,6 +445,8 @@ object Main {
       objcDisableClassCtor,
       objcClosedEnums,
       objcStrictProtocol,
+      objcLegacyRecords,
+      objcOmitFullConvenienceConstructor,
       wasmOutFolder,
       wasmIncludePrefix,
       wasmIncludeCppPrefix,