fix(java,dotnet): emit default implementations for optional properties

For optional properties on interfaces, emit default implementations when possible (for .NET, this requires `netcoreapp3.0` or later) so that adding a new optional property on an interface does not break existing core that implements the interface. In it's current form, the getters always return `null`, while the setters throw an exception (this may change in the future). Fixes #543
aws · Oct 24, 2019 · 9879e11 · 9879e11
1 parent e3f0f6c
commit 9879e11
Show file tree

Hide file tree

Showing 39 changed files with 336 additions and 42 deletions.
diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
@@ -183,10 +183,27 @@ export class DotNetGenerator extends Generator {
     const isOptionalPrimitive = this.isOptionalPrimitive(prop) ? '?' : '';
     this.code.openBlock(`${propType}${isOptionalPrimitive} ${propName}`);
 
+    if (prop.optional) {
+      // Conditional compilation - default interface implementation requires C#8.0 support, which imples netcoreapp3.0 or above!
+      this.code.line('#if NETCOREAPP3_0');
+      this.code.openBlock('get');
+      this.code.line('return null;');
+      this.code.closeBlock();
+      if (!prop.immutable) {
+        this.code.openBlock('set');
+        this.code.line(`throw new System.NotSupportedException("'set' for '${propName}' is not implemented");`);
+        this.code.closeBlock();
+      }
+      this.code.line('#else')
+    }
     this.code.line('get;');
     if (!prop.immutable) {
       this.code.line('set;');
     }
+    if (prop.optional) {
+      this.code.line('#endif');
+    }
+
     this.code.closeBlock();
     this.flagFirstMemberWritten(true);
   }

diff --git a/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts
@@ -50,7 +50,7 @@ export class FileGenerator {
     const rootNode = xmlbuilder.create('Project', { encoding: 'UTF-8', headless: true });
     rootNode.att('Sdk', 'Microsoft.NET.Sdk');
     const propertyGroup = rootNode.ele('PropertyGroup');
-    propertyGroup.ele('TargetFramework', 'netcoreapp2.1');
+    propertyGroup.ele('TargetFrameworks', 'netcoreapp2.1;netcoreapp3.0');
     propertyGroup.ele('GeneratePackageOnBuild', 'true');
     propertyGroup.ele('GenerateDocumentationFile', 'true');
     propertyGroup.ele('IncludeSymbols', 'true');

diff --git a/packages/jsii-pacmak/lib/targets/java.ts b/packages/jsii-pacmak/lib/targets/java.ts
@@ -538,13 +538,25 @@ class JavaGenerator extends Generator {
     this.code.line();
     this.addJavaDocs(prop);
     this.emitStabilityAnnotations(prop);
-    this.code.line(`${getterType} get${propName}();`);
+    if (prop.optional) {
+      this.code.openBlock(`default ${getterType} get${propName}()`);
+      this.code.line('return null;');
+      this.code.closeBlock();
+    } else {
+      this.code.line(`${getterType} get${propName}();`);
+    }
 
     if (!prop.immutable) {
       for (const type of setterTypes) {
         this.code.line();
         this.addJavaDocs(prop);
-        this.code.line(`void set${propName}(final ${type} value);`);
+        if (prop.optional) {
+          this.code.openBlock(`default void set${propName}(final ${type} value)`);
+          this.code.line(`throw new RuntimeException("'void " + getClass().getCanonicalName() + "#set${propName}(${type})' is not implemented!");`);
+          this.code.closeBlock();
+        } else {
+          this.code.line(`void set${propName}(final ${type} value);`);
+        }
       }
     }
   }

diff --git a/packages/jsii-pacmak/test/diff-test.sh b/packages/jsii-pacmak/test/diff-test.sh
@@ -29,11 +29,6 @@ function assert-generator() {
         rsync -a ${original_expected}/ ${expected}/
     fi
 
-    if [[ -d ${original_expected}/java/target ]]; then
-        echo "An IDE plugin seems to have eagerly tried to compile ${original_expected}/java. Please remove." >&2
-        exit 1
-    fi
-
     # put the real expected tarball instead of the placeholder
     for expected_tarball_placeholder in $(find ${expected} -name "*.tgz" || true); do
         rm -f ${expected_tarball_placeholder}

diff --git a/...culatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj b/...culatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj
@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFrameworks>netcoreapp2.1;netcoreapp3.0</TargetFrameworks>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>

diff --git a/...alculatorPackageId.LibPackageId/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId.csproj b/...alculatorPackageId.LibPackageId/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId.csproj
@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFrameworks>netcoreapp2.1;netcoreapp3.0</TargetFrameworks>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>

diff --git a/...ckageId.LibPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IMyFirstStruct.cs b/...ckageId.LibPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IMyFirstStruct.cs
@@ -39,7 +39,14 @@ string Astring
         [System.Obsolete()]
         string[] FirstOptional
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }
diff --git a/...bPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IStructWithOnlyOptionals.cs b/...bPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IStructWithOnlyOptionals.cs
@@ -18,7 +18,14 @@ public interface IStructWithOnlyOptionals
         [System.Obsolete()]
         string Optional1
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -28,7 +35,14 @@ string Optional1
         [System.Obsolete()]
         double? Optional2
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -38,7 +52,14 @@ string Optional1
         [System.Obsolete()]
         bool? Optional3
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }
diff --git a/...-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/MyFirstStruct.java b/...-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/MyFirstStruct.java
@@ -26,7 +26,9 @@ public interface MyFirstStruct extends software.amazon.jsii.JsiiSerializable {
      */
     @Deprecated
     @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
-    java.util.List<java.lang.String> getFirstOptional();
+    default java.util.List<java.lang.String> getFirstOptional() {
+        return null;
+    }
 
     /**
      * @return a {@link Builder} of {@link MyFirstStruct}

diff --git a/...java/src/main/java/software/amazon/jsii/tests/calculator/lib/StructWithOnlyOptionals.java b/...java/src/main/java/software/amazon/jsii/tests/calculator/lib/StructWithOnlyOptionals.java
@@ -13,19 +13,25 @@ public interface StructWithOnlyOptionals extends software.amazon.jsii.JsiiSerial
      */
     @Deprecated
     @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
-    java.lang.String getOptional1();
+    default java.lang.String getOptional1() {
+        return null;
+    }
 
     /**
      */
     @Deprecated
     @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
-    java.lang.Number getOptional2();
+    default java.lang.Number getOptional2() {
+        return null;
+    }
 
     /**
      */
     @Deprecated
     @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
-    java.lang.Boolean getOptional3();
+    default java.lang.Boolean getOptional3() {
+        return null;
+    }
 
     /**
      * @return a {@link Builder} of {@link StructWithOnlyOptionals}

diff --git a/...dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj b/...dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj
@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFrameworks>netcoreapp2.1;netcoreapp3.0</TargetFrameworks>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>

diff --git a/....JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/ICalculatorProps.cs b/....JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/ICalculatorProps.cs
@@ -15,7 +15,14 @@ public interface ICalculatorProps
         [JsiiProperty(name: "initialValue", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
         double? InitialValue
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -24,7 +31,14 @@ public interface ICalculatorProps
         [JsiiProperty(name: "maximumValue", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
         double? MaximumValue
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }
diff --git a/...I.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDeprecatedInterface.cs b/...I.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDeprecatedInterface.cs
@@ -16,8 +16,19 @@ public interface IDeprecatedInterface
         [System.Obsolete("could be better")]
         double? MutableProperty
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            set
+            {
+                throw new System.NotSupportedException("'set' for 'MutableProperty' is not implemented");
+            }
+            #else
             get;
             set;
+            #endif
         }
         /// <remarks>
         /// stability: Deprecated

diff --git a/...on.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs b/...on.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDerivedStruct.cs
@@ -44,7 +44,14 @@ Amazon.JSII.Tests.CalculatorNamespace.DoubleTrouble NonPrimitive
         [JsiiProperty(name: "anotherOptional", typeJson: "{\"collection\":{\"elementtype\":{\"fqn\":\"@scope/jsii-calc-lib.Value\"},\"kind\":\"map\"}}", isOptional: true)]
         System.Collections.Generic.IDictionary<string, Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.Value_> AnotherOptional
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -53,7 +60,14 @@ Amazon.JSII.Tests.CalculatorNamespace.DoubleTrouble NonPrimitive
         [JsiiProperty(name: "optionalAny", typeJson: "{\"primitive\":\"any\"}", isOptional: true)]
         object OptionalAny
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -62,7 +76,14 @@ object OptionalAny
         [JsiiProperty(name: "optionalArray", typeJson: "{\"collection\":{\"elementtype\":{\"primitive\":\"string\"},\"kind\":\"array\"}}", isOptional: true)]
         string[] OptionalArray
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }
diff --git a/...ulatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IEraseUndefinedHashValuesOptions.cs b/...ulatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IEraseUndefinedHashValuesOptions.cs
@@ -14,7 +14,14 @@ public interface IEraseUndefinedHashValuesOptions
         [JsiiProperty(name: "option1", typeJson: "{\"primitive\":\"string\"}", isOptional: true)]
         string Option1
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
 
         /// <remarks>
@@ -23,7 +30,14 @@ string Option1
         [JsiiProperty(name: "option2", typeJson: "{\"primitive\":\"string\"}", isOptional: true)]
         string Option2
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }
diff --git a/...Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IExperimentalInterface.cs b/...Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IExperimentalInterface.cs
@@ -14,8 +14,19 @@ public interface IExperimentalInterface
         [JsiiProperty(name: "mutableProperty", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
         double? MutableProperty
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            set
+            {
+                throw new System.NotSupportedException("'set' for 'MutableProperty' is not implemented");
+            }
+            #else
             get;
             set;
+            #endif
         }
         /// <remarks>
         /// stability: Experimental

diff --git a/...t/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IGreetee.cs b/...t/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IGreetee.cs
@@ -18,7 +18,14 @@ public interface IGreetee
         [JsiiProperty(name: "name", typeJson: "{\"primitive\":\"string\"}", isOptional: true)]
         string Name
         {
+            #if NETCOREAPP3_0
+            get
+            {
+                return null;
+            }
+            #else
             get;
+            #endif
         }
     }
 }