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

#906) For optional properties on interfaces, emit default implementations (for .NET, this required upgrading the target to `netcoreapp3.0`) so that adding a new optional property on an interface does not break existing code that implements the interface. In it's current form, the getters always return `null`, while the setters throw an exception unless overridden. Fixes #543
aws · Oct 29, 2019 · 37ddfd5 · 37ddfd5
1 parent 670d00e
commit 37ddfd5
Show file tree

Hide file tree

Showing 50 changed files with 323 additions and 83 deletions.
diff --git a/...tnet-analyzers/src/Amazon.JSII.Analyzers.UnitTests/Amazon.JSII.Analyzers.UnitTests.csproj b/...tnet-analyzers/src/Amazon.JSII.Analyzers.UnitTests/Amazon.JSII.Analyzers.UnitTests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <IsPackable>false</IsPackable>
     <AssemblyName>Amazon.JSII.Analyzers.UnitTests</AssemblyName>
   </PropertyGroup>

diff --git a/packages/jsii-dotnet-analyzers/src/Amazon.JSII.Analyzers/Amazon.JSII.Analyzers.csproj b/packages/jsii-dotnet-analyzers/src/Amazon.JSII.Analyzers/Amazon.JSII.Analyzers.csproj
@@ -4,7 +4,7 @@
     <PropertyGroup>
         <PackageId>Amazon.JSII.Analyzers</PackageId>
         <Title>.NET Roslyn Analyzers for JSII</Title>
-        <TargetFramework>netcoreapp2.1</TargetFramework>
+        <TargetFramework>netcoreapp3.0</TargetFramework>
         <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
         <PackageIcon>icon.png</PackageIcon>
     </PropertyGroup>

diff --git a/...tnet-jsonmodel/src/Amazon.JSII.JsonModel.UnitTests/Amazon.JSII.JsonModel.UnitTests.csproj b/...tnet-jsonmodel/src/Amazon.JSII.JsonModel.UnitTests/Amazon.JSII.JsonModel.UnitTests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <IsPackable>false</IsPackable>
     <AssemblyName>Amazon.JSII.JsonModel.UnitTests</AssemblyName>
     <RootNamespace>Amazon.JSII.JsonModel.UnitTests</RootNamespace>

diff --git a/packages/jsii-dotnet-jsonmodel/src/Amazon.JSII.JsonModel/Amazon.JSII.JsonModel.csproj b/packages/jsii-dotnet-jsonmodel/src/Amazon.JSII.JsonModel/Amazon.JSII.JsonModel.csproj
@@ -2,7 +2,7 @@
   <Import Project="../NuGet.Metadata.props" />
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <PackageId>Amazon.JSII.JsonModel</PackageId>
     <Title>.NET JsonModel for JSII</Title>
     <PackageIcon>icon.png</PackageIcon>

diff --git a/...est/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj b/...est/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
-        <TargetFramework>netcoreapp2.1</TargetFramework>
+        <TargetFramework>netcoreapp3.0</TargetFramework>
 
         <IsPackable>false</IsPackable>
     </PropertyGroup>

diff --git a/...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
@@ -928,7 +928,7 @@ public void OptionalAndVariadicArgumentsTest()
         [Fact(DisplayName = Prefix + nameof(JsiiAgent))]
         public void JsiiAgent()
         {
-            Assert.Equal("DotNet/" + Environment.Version.ToString() + "/.NETCoreApp,Version=v2.1/1.0.0.0", JsiiAgent_.JsiiAgent);
+            Assert.Equal("DotNet/" + Environment.Version.ToString() + "/.NETCoreApp,Version=v3.0/1.0.0.0", JsiiAgent_.JsiiAgent);
         }
 
         [Fact(DisplayName = Prefix + nameof(ReceiveInstanceOfPrivateClass))]

diff --git a/...sii-dotnet-runtime/src/Amazon.JSII.Runtime.UnitTests/Amazon.JSII.Runtime.UnitTests.csproj b/...sii-dotnet-runtime/src/Amazon.JSII.Runtime.UnitTests/Amazon.JSII.Runtime.UnitTests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
 
     <IsPackable>false</IsPackable>
 

diff --git a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Amazon.JSII.Runtime.csproj b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Amazon.JSII.Runtime.csproj
@@ -2,7 +2,7 @@
   <Import Project="../NuGet.Metadata.props" />
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <PackageId>Amazon.JSII.Runtime</PackageId>
     <Title>.NET Runtime for JSII</Title>
     <PackageIcon>icon.png</PackageIcon>

diff --git a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/JsiiOptionalAttribute.cs b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/JsiiOptionalAttribute.cs
@@ -4,9 +4,15 @@ namespace Amazon.JSII.Runtime.Deputy
 {
     /// <summary>
     /// Flags a property as optional.
-    /// This is used by the jsii-dotnet-analyzers package to emit errors
-    /// on required properties that are missing.
+    /// 
+    /// This is used by the jsii-dotnet-analyzers package to emit errors on required properties that are missing.
     /// </summary>
+    /// <remarks>
+    /// Annotated properties have a setter with a default implementation that throws
+    /// <c>System.NotSupportedException</c> when invoked. In a similar way that they have to in TypeScript,
+    /// implementors need to actively opt into supporting the functionality by providing a custom implementation
+    /// for the member.
+    /// </remarks>
     [AttributeUsage(AttributeTargets.Property)]
     public class JsiiOptionalAttribute : Attribute
     {

diff --git a/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/Optional.java b/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/Optional.java
@@ -0,0 +1,15 @@
+package software.amazon.jsii;
+
+import java.lang.annotation.*;
+
+/**
+ * Denotes an optional member from the TypeScript model for the API.
+ *
+ * Annotated methods have a default implementation that throws {@link UnsupportedOperationException} when invoked.
+ * In a similar way that they have to in TypeScript, implementors need to actively opt into supporting the
+ * functionality by providing a custom implementation for the member.
+ */
+@Documented
+@Target({ElementType.METHOD})
+@Retention(RetentionPolicy.SOURCE)
+public @interface Optional {}
diff --git a/packages/jsii-pacmak/lib/targets/dotnet.ts b/packages/jsii-pacmak/lib/targets/dotnet.ts
@@ -55,7 +55,7 @@ export default class Dotnet extends Target {
     await this.copyFiles(
       path.join(sourceDir, packageId, 'bin', 'Release'),
       outDir);
-    await fs.remove(path.join(outDir, 'netcoreapp2.1'));
+    await fs.remove(path.join(outDir, 'netcoreapp3.0'));
   }
 
   private async generateNuGetConfigForLocalDeps(sourceDirectory: string, currentOutputDirectory: string): Promise<void> {

diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
@@ -179,14 +179,30 @@ export class DotNetGenerator extends Generator {
     const propType = this.typeresolver.toDotNetType(prop.type);
     const propName = this.nameutils.convertPropertyName(prop.name);
 
+    if (prop.optional) {
+      this.code.line('[Amazon.JSII.Runtime.Deputy.JsiiOptional]');
+    }
+
     // Specifying that a type is nullable is only required for primitive value types
     const isOptionalPrimitive = this.isOptionalPrimitive(prop) ? '?' : '';
     this.code.openBlock(`${propType}${isOptionalPrimitive} ${propName}`);
 
-    this.code.line('get;');
-    if (!prop.immutable) {
-      this.code.line('set;');
+    if (prop.optional) {
+      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();
+      }
+    } else {
+      this.code.line('get;');
+      if (!prop.immutable) {
+        this.code.line('set;');
+      }
     }
+
     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('TargetFramework', '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,26 @@ 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.line('@software.amazon.jsii.Optional');
+          this.code.openBlock(`default void set${propName}(final ${type} value)`);
+          this.code.line(`throw new UnsupportedOperationException("'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
@@ -30,7 +30,11 @@ function assert-generator() {
     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
+        # When IDEs automaticallu compile the java target, the diff-test may fail later on with a cryptic error message
+        # due to the existence of .settings / .projects / ... directories that aren't part of the expected outcome. This
+        # check is here to avoid the confusion.
+        echo "An IDE plugin seems to have eagerly tried to compile ${original_expected}/java. Please remove:"
+        echo "    rm -rf ${original_expected}/java/target"
         exit 1
     fi
 

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>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <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>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <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
@@ -37,9 +37,13 @@ string Astring
         /// </remarks>
         [JsiiProperty(name: "firstOptional", typeJson: "{\"collection\":{\"elementtype\":{\"primitive\":\"string\"},\"kind\":\"array\"}}", isOptional: true)]
         [System.Obsolete()]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         string[] FirstOptional
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
     }
 }
diff --git a/...bPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IStructWithOnlyOptionals.cs b/...bPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/IStructWithOnlyOptionals.cs
@@ -16,29 +16,41 @@ public interface IStructWithOnlyOptionals
         /// </remarks>
         [JsiiProperty(name: "optional1", typeJson: "{\"primitive\":\"string\"}", isOptional: true)]
         [System.Obsolete()]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         string Optional1
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
 
         /// <remarks>
         /// stability: Deprecated
         /// </remarks>
         [JsiiProperty(name: "optional2", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
         [System.Obsolete()]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         double? Optional2
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
 
         /// <remarks>
         /// stability: Deprecated
         /// </remarks>
         [JsiiProperty(name: "optional3", typeJson: "{\"primitive\":\"boolean\"}", isOptional: true)]
         [System.Obsolete()]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         bool? Optional3
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
     }
 }
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>
+    <TargetFramework>netcoreapp3.0</TargetFramework>
     <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
@@ -13,18 +13,26 @@ public interface ICalculatorProps
         /// stability: Experimental
         /// </remarks>
         [JsiiProperty(name: "initialValue", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         double? InitialValue
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
 
         /// <remarks>
         /// stability: Experimental
         /// </remarks>
         [JsiiProperty(name: "maximumValue", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         double? MaximumValue
         {
-            get;
+            get
+            {
+                return null;
+            }
         }
     }
 }
diff --git a/...I.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDeprecatedInterface.cs b/...I.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IDeprecatedInterface.cs
@@ -14,10 +14,17 @@ public interface IDeprecatedInterface
         /// </remarks>
         [JsiiProperty(name: "mutableProperty", typeJson: "{\"primitive\":\"number\"}", isOptional: true)]
         [System.Obsolete("could be better")]
+        [Amazon.JSII.Runtime.Deputy.JsiiOptional]
         double? MutableProperty
         {
-            get;
-            set;
+            get
+            {
+                return null;
+            }
+            set
+            {
+                throw new System.NotSupportedException("'set' for 'MutableProperty' is not implemented");
+            }
         }
         /// <remarks>
         /// stability: Deprecated