Merge branch 'master' into rmuller/hack-around-node-13.2+-issue

aws · Dec 19, 2019 · 5ab5034 · 5ab5034
2 parents dfb4a95 + 674181e
commit 5ab5034
Show file tree

Hide file tree

Showing 24 changed files with 617 additions and 52 deletions.
diff --git a/...es/@jsii/dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/...es/@jsii/dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
@@ -1398,5 +1398,30 @@ public void LiftedKwargWithSameNameAsPositionalArg()
             Assert.Equal(bell, amb.Scope);
             Assert.Equal("Driiiing!", amb.Props.Scope);
         }
+
+        [Fact(DisplayName = Prefix + nameof(AbstractMembersAreCorrectlyHandled))]
+        public void AbstractMembersAreCorrectlyHandled()
+        {
+            var abstractSuite = new AbstractSuiteImpl();
+            Assert.Equal("Wrapped<String<Oomf!>>", abstractSuite.WorkItAll("Oomf!"));
+        }
+
+        private sealed class AbstractSuiteImpl : AbstractSuite
+        {
+            private string _property;
+
+            public AbstractSuiteImpl() {}
+
+            protected override string SomeMethod(string str)
+            {
+                return $"Wrapped<{str}>";
+            }
+
+            protected override string Property
+            {
+                get => _property;
+                set => _property = $"String<{value}>";
+            }
+        }
     }
 }
diff --git a/packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs b/packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs
@@ -155,7 +155,7 @@ private static CallbackResult InvokeGetter(GetRequest request, IReferenceMap ref
 
             var attribute = propertyInfo.GetAttribute<JsiiPropertyAttribute>();
 
-            var methodInfo = propertyInfo.GetGetMethod();
+            var methodInfo = propertyInfo.GetMethod;
             if (methodInfo == null)
             {
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} getter, but this property does not have a getter");
@@ -175,7 +175,7 @@ private static void InvokeSetter(SetRequest request, IReferenceMap referenceMap)
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} setter, but this property does not exist");
             }
 
-            var methodInfo = propertyInfo.GetSetMethod();
+            var methodInfo = propertyInfo.SetMethod;
             if (methodInfo == null)
             {
                 throw new InvalidOperationException($"Received callback for {deputy.GetType().Name}.{request.Property} setter, but this property does not have a setter");

diff --git a/.../java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java b/.../java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java
@@ -1667,4 +1667,28 @@ public void liftedKwargWithSameNameAsPositionalArg() {
         assertEquals(bell, amb.getScope());
         assertEquals(StructParameterType.builder().scope("Driiiing!").build(), amb.getProps());
     }
+
+    @Test
+    public void abstractMembersAreCorrectlyHandled() {
+        final AbstractSuite abstractSuite = new AbstractSuite() {
+            private String property;
+
+            @Override
+            protected String someMethod(String str) {
+                return String.format("Wrapped<%s>", str);
+            }
+
+            @Override
+            protected String getProperty() {
+                return this.property;
+            }
+
+            @Override
+            protected void setProperty(String value) {
+                this.property = String.format("String<%s>", value);
+            }
+        };
+
+        assertEquals("Wrapped<String<Oomf!>>", abstractSuite.workItAll("Oomf!"));
+    }
 }
diff --git a/packages/@jsii/python-runtime/tests/test_compliance.py b/packages/@jsii/python-runtime/tests/test_compliance.py
@@ -10,6 +10,7 @@
 
 from jsii_calc import (
     AbstractClassReturner,
+    AbstractSuite,
     Add,
     AllTypes,
     AllTypesEnum,
@@ -1130,3 +1131,19 @@ def test_lifted_kwarg_with_same_name_as_positional_arg():
 
     assert amb.scope == bell
     assert amb.props == StructParameterType(scope='Driiiing!')
+
+def test_abstract_members_are_correctly_handled():
+    class AbstractSuiteImpl(AbstractSuite):
+        @property
+        def _property(self):
+            return self.property
+
+        @_property.setter
+        def _property(self, value):
+            self.property = "String<%s>" % value
+
+        def _some_method(self, str):
+            return "Wrapped<%s>" % str
+
+    abstract_suite = AbstractSuiteImpl()
+    assert "Wrapped<String<Oomf!>>" == abstract_suite.work_it_all("Oomf!")
diff --git a/packages/@jsii/runtime/package.json b/packages/@jsii/runtime/package.json
@@ -49,7 +49,7 @@
     "source-map-loader": "^0.2.4",
     "typescript": "~3.7.3",
     "wasm-loader": "^1.3.0",
-    "webpack": "^4.41.3",
+    "webpack": "^4.41.4",
     "webpack-cli": "^3.3.10"
   },
   "jest": {

diff --git a/packages/jsii-calc/lib/calculator.ts b/packages/jsii-calc/lib/calculator.ts
@@ -394,3 +394,20 @@ export interface SmellyStruct {
     readonly property: string;
     readonly yetAnoterOne: boolean;
 }
+
+/**
+ * Ensures abstract members implementations correctly register overrides in various languages.
+ */
+export abstract class AbstractSuite {
+    protected abstract property: string;
+    protected abstract someMethod(str: string): string;
+
+    /**
+     * Sets `seed` to `this.property`, then calls `someMethod` with `this.property` and returns the result.
+     * @param seed a `string`.
+     */
+    public workItAll(seed: string) {
+        this.property = seed;
+        return this.someMethod(this.property);
+    }
+}
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -401,6 +401,93 @@
         }
       ]
     },
+    "jsii-calc.AbstractSuite": {
+      "abstract": true,
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "experimental",
+        "summary": "Ensures abstract members implementations correctly register overrides in various languages."
+      },
+      "fqn": "jsii-calc.AbstractSuite",
+      "initializer": {},
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/calculator.ts",
+        "line": 401
+      },
+      "methods": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/calculator.ts",
+            "line": 403
+          },
+          "name": "someMethod",
+          "parameters": [
+            {
+              "name": "str",
+              "type": {
+                "primitive": "string"
+              }
+            }
+          ],
+          "protected": true,
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        },
+        {
+          "docs": {
+            "stability": "experimental",
+            "summary": "Sets `seed` to `this.property`, then calls `someMethod` with `this.property` and returns the result."
+          },
+          "locationInModule": {
+            "filename": "lib/calculator.ts",
+            "line": 409
+          },
+          "name": "workItAll",
+          "parameters": [
+            {
+              "docs": {
+                "summary": "a `string`."
+              },
+              "name": "seed",
+              "type": {
+                "primitive": "string"
+              }
+            }
+          ],
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        }
+      ],
+      "name": "AbstractSuite",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/calculator.ts",
+            "line": 402
+          },
+          "name": "property",
+          "protected": true,
+          "type": {
+            "primitive": "string"
+          }
+        }
+      ]
+    },
     "jsii-calc.Add": {
       "assembly": "jsii-calc",
       "base": "jsii-calc.BinaryOperation",
@@ -12002,5 +12089,5 @@
     }
   },
   "version": "0.20.11",
-  "fingerprint": "eenqZx1+dNMvfZCY02AIrinXeK98iQC4XLLj0yPIXuM="
+  "fingerprint": "2idHJnE1Vv8YAFFrhqRzB2SwAvTXdSFkD7jKYcbabX4="
 }
diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
@@ -628,8 +628,8 @@ export class DotNetGenerator extends Generator {
   }
 
   /**
-     * Emits a property
-     */
+   * Emits a property
+   */
   private emitProperty(cls: spec.Type, prop: spec.Property, datatype = false, proxy = false): void {
 
     this.emitNewLineIfNecessary();
@@ -646,26 +646,31 @@ export class DotNetGenerator extends Generator {
     this.dotnetRuntimeGenerator.emitAttributesForProperty(prop, datatype);
 
     let isOverrideKeyWord = '';
-
     let isVirtualKeyWord = '';
+    let isAbstractKeyword = '';
+
     // If the prop parent is a class
     if (cls.kind === spec.TypeKind.Class) {
       const implementedInBase = this.isMemberDefinedOnAncestor(cls as spec.ClassType, prop);
       if (implementedInBase || datatype || proxy) {
         // Override if the property is in a datatype or proxy class or declared in a parent class
         isOverrideKeyWord = 'override ';
-      } else if (!prop.static && (prop.abstract || !implementedInBase)) {
-        // Virtual if the prop is not static, and is abstract or not implemented in base member, this way we can later override it.
+      } else if (prop.abstract) {
+        // Abstract members get decorated as such
+        isAbstractKeyword = 'abstract ';
+      } else if (!prop.static && !implementedInBase) {
+        // Virtual if the prop is not static, and is not implemented in base member, this way we can later override it.
         isVirtualKeyWord = 'virtual ';
       }
     }
+
     const propTypeFQN = this.typeresolver.toDotNetType(prop.type);
     const isOptionalPrimitive = this.isOptionalPrimitive(prop) ? '?' : '';
-    const statement = `${access} ${isVirtualKeyWord}${isOverrideKeyWord}${staticKeyWord}${propTypeFQN}${isOptionalPrimitive} ${propName}`;
+    const statement = `${access} ${isAbstractKeyword}${isVirtualKeyWord}${isOverrideKeyWord}${staticKeyWord}${propTypeFQN}${isOptionalPrimitive} ${propName}`;
     this.code.openBlock(statement);
 
     // Emit getters
-    if (datatype || prop.const) {
+    if (datatype || prop.const || prop.abstract) {
       this.code.line('get;');
     } else {
       if (prop.static) {
@@ -676,7 +681,7 @@ export class DotNetGenerator extends Generator {
     }
 
     // Emit setters
-    if (datatype) {
+    if (datatype || (!prop.immutable && prop.abstract)) {
       this.code.line('set;');
     } else {
       if (!prop.immutable) {

diff --git a/packages/jsii-pacmak/lib/targets/java.ts b/packages/jsii-pacmak/lib/targets/java.ts
@@ -894,6 +894,7 @@ class JavaGenerator extends Generator {
     const propName = this.code.toPascalCase(JavaGenerator.safeJavaPropertyName(prop.name));
     const access = this.renderAccessLevel(prop);
     const statc = prop.static ? 'static ' : '';
+    const abstract = prop.abstract ? 'abstract ' : '';
     const javaClass = this.toJavaType(cls);
 
     // for unions we only generate overloads for setters, not getters.
@@ -902,18 +903,23 @@ class JavaGenerator extends Generator {
       this.addJavaDocs(prop);
       if (overrides) { this.code.line('@Override'); }
       this.emitStabilityAnnotations(prop);
-      this.code.openBlock(`${access} ${statc}${getterType} get${propName}()`);
-      let statement;
-      if (prop.static) {
-        statement = `software.amazon.jsii.JsiiObject.jsiiStaticGet(${javaClass}.class, `;
+      const signature = `${access} ${abstract}${statc}${getterType} get${propName}()`;
+      if (prop.abstract) {
+        this.code.line(`${signature};`);
       } else {
-        statement = 'this.jsiiGet(';
-      }
+        this.code.openBlock(signature);
+        let statement;
+        if (prop.static) {
+          statement = `software.amazon.jsii.JsiiObject.jsiiStaticGet(${javaClass}.class, `;
+        } else {
+          statement = 'this.jsiiGet(';
+        }
 
-      statement += `"${prop.name}", ${propClass}.class)`;
+        statement += `"${prop.name}", ${propClass}.class)`;
 
-      this.code.line(`return ${this.wrapCollection(statement, prop.type, prop.optional)};`);
-      this.code.closeBlock();
+        this.code.line(`return ${this.wrapCollection(statement, prop.type, prop.optional)};`);
+        this.code.closeBlock();
+      }
     }
 
     if (!prop.immutable) {
@@ -922,18 +928,23 @@ class JavaGenerator extends Generator {
         this.addJavaDocs(prop);
         if (overrides) { this.code.line('@Override'); }
         this.emitStabilityAnnotations(prop);
-        this.code.openBlock(`${access} ${statc}void set${propName}(final ${type} value)`);
-        let statement = '';
-
-        if (prop.static) {
-          statement += `software.amazon.jsii.JsiiObject.jsiiStaticSet(${javaClass}.class, `;
+        const signature = `${access} ${abstract}${statc}void set${propName}(final ${type} value)`;
+        if (prop.abstract) {
+          this.code.line(`${signature};`);
         } else {
-          statement += 'this.jsiiSet(';
+          this.code.openBlock(signature);
+          let statement = '';
+
+          if (prop.static) {
+            statement += `software.amazon.jsii.JsiiObject.jsiiStaticSet(${javaClass}.class, `;
+          } else {
+            statement += 'this.jsiiSet(';
+          }
+          const value = prop.optional ? 'value' : `java.util.Objects.requireNonNull(value, "${prop.name} is required")`;
+          statement += `"${prop.name}", ${value});`;
+          this.code.line(statement);
+          this.code.closeBlock();
         }
-        const value = prop.optional ? 'value' : `java.util.Objects.requireNonNull(value, "${prop.name} is required")`;
-        statement += `"${prop.name}", ${value});`;
-        this.code.line(statement);
-        this.code.closeBlock();
       }
     }
   }

diff --git a/...ulatorPackageId.LibPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/Value_.cs b/...ulatorPackageId.LibPackageId/Amazon/JSII/Tests/CalculatorNamespace/LibNamespace/Value_.cs
@@ -44,9 +44,9 @@ public override string ToString()
         /// </remarks>
         [JsiiProperty(name: "value", typeJson: "{\"primitive\":\"number\"}")]
         [System.Obsolete()]
-        public virtual double Value
+        public abstract double Value
         {
-            get => GetInstanceProperty<double>();
+            get;
         }
     }
 }
diff --git a/...ted.jsii-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/Value.java b/...ted.jsii-calc-lib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/Value.java
@@ -36,9 +36,7 @@ public java.lang.String toString() {
      */
     @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
     @Deprecated
-    public java.lang.Number getValue() {
-        return this.jsiiGet("value", java.lang.Number.class);
-    }
+    public abstract java.lang.Number getValue();
 
     /**
      * A proxy class which represents a concrete javascript instance of this type.