fix: "Malformed enum value" when using @Scoped packages (#139)

Fixes #138 The code that validated the format of jsii enum values was too rigid and didn't allow "/" in type names. When using a package from an npm scope, the enum value will look like this `@scope/foo.EnumType/MemberName`. Also, add a verification that the enum member exists in the enum and fail otherwise. Required fix in kernel, java and dotnet runtimes.
aws · Aug 6, 2018 · 4e70209 · 4e70209
1 parent 8416b6a
commit 4e70209
Show file tree

Hide file tree

Showing 20 changed files with 348 additions and 18 deletions.
diff --git a/packages/jsii-calc-lib/lib/index.ts b/packages/jsii-calc-lib/lib/index.ts
@@ -82,3 +82,12 @@ export interface StructWithOnlyOptionals {
     optional2?: number
     optional3?: boolean
 }
+
+/**
+ * Check that enums from @scoped packages can be references.
+ * See awslabs/jsii#138
+ */
+export enum EnumFromScopedModule {
+    Value1,
+    Value2
+}
diff --git a/packages/jsii-calc-lib/test/assembly.jsii b/packages/jsii-calc-lib/test/assembly.jsii
@@ -1,5 +1,5 @@
 {
-  "fingerprint": "GbVnxjsdaq598NCRdOJ8entjQRwThRmFgdeLNaQbIZY=",
+  "fingerprint": "yzHGjyI4/53o87fPNjR/Nz/4G58kg5jVoKtgwdqaEU8=",
   "author": {
     "name": "Amazon Web Services",
     "organization": true,
@@ -73,6 +73,24 @@
     }
   },
   "types": {
+    "@scope/jsii-calc-lib.EnumFromScopedModule": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "comment": "Check that enums from @scoped packages can be references.\nSee awslabs/jsii#138"
+      },
+      "fqn": "@scope/jsii-calc-lib.EnumFromScopedModule",
+      "kind": "enum",
+      "members": [
+        {
+          "name": "Value1"
+        },
+        {
+          "name": "Value2"
+        }
+      ],
+      "name": "EnumFromScopedModule",
+      "namespace": "@scope/jsii-calc-lib"
+    },
     "@scope/jsii-calc-lib.IFriendly": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {

diff --git a/packages/jsii-calc/lib/index.ts b/packages/jsii-calc/lib/index.ts
@@ -1,5 +1,5 @@
 // tslint:disable
-import { Operation, Value, Number, IFriendly, MyFirstStruct, StructWithOnlyOptionals } from '@scope/jsii-calc-lib'
+import { Operation, Value, Number, IFriendly, MyFirstStruct, StructWithOnlyOptionals, EnumFromScopedModule } from '@scope/jsii-calc-lib'
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
@@ -1101,4 +1101,19 @@ export class UseCalcBase {
 
 export interface ImplictBaseOfBase extends base.BaseProps {
     goo: Date;
+}
+
+/**
+ * See awslabs/jsii#138
+ */
+export class ReferenceEnumFromScopedPackage {
+    public foo?: EnumFromScopedModule = EnumFromScopedModule.Value2;
+
+    public loadFoo(): EnumFromScopedModule | undefined {
+        return this.foo;
+    }
+
+    public saveFoo(value: EnumFromScopedModule) {
+        this.foo = value;
+    }
 }
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -1,5 +1,5 @@
 {
-  "fingerprint": "ZR90w/OQ/vVxm0Cg1sicOKwbZxbGK43MXqj2ksUyCrk=",
+  "fingerprint": "ENUxKESaYuaH8SRuMSsikJscFgsj+E8Ey1WvHIsNsZg=",
   "author": {
     "name": "Amazon Web Services",
     "organization": true,
@@ -1919,6 +1919,48 @@
         }
       ]
     },
+    "jsii-calc.ReferenceEnumFromScopedPackage": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "comment": "See awslabs/jsii#138"
+      },
+      "fqn": "jsii-calc.ReferenceEnumFromScopedPackage",
+      "initializer": {
+        "initializer": true
+      },
+      "kind": "class",
+      "methods": [
+        {
+          "name": "loadFoo",
+          "returns": {
+            "fqn": "@scope/jsii-calc-lib.EnumFromScopedModule",
+            "optional": true
+          }
+        },
+        {
+          "name": "saveFoo",
+          "parameters": [
+            {
+              "name": "value",
+              "type": {
+                "fqn": "@scope/jsii-calc-lib.EnumFromScopedModule"
+              }
+            }
+          ]
+        }
+      ],
+      "name": "ReferenceEnumFromScopedPackage",
+      "namespace": "jsii-calc",
+      "properties": [
+        {
+          "name": "foo",
+          "type": {
+            "fqn": "@scope/jsii-calc-lib.EnumFromScopedModule",
+            "optional": true
+          }
+        }
+      ]
+    },
     "jsii-calc.ReturnsNumber": {
       "assembly": "jsii-calc",
       "fqn": "jsii-calc.ReturnsNumber",

diff --git a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/EnumValue.cs b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/EnumValue.cs
@@ -11,14 +11,14 @@ public EnumValue(string value)
         {
             Value = value ?? throw new ArgumentNullException(nameof(value));
 
-            string[] valueTokens = value.Split('/');
-            if (valueTokens.Length != 2)
+            int sep = value.LastIndexOf('/');
+            if (sep == -1)
             {
                 throw new ArgumentException($"Unexpected format for enum value: {value}", nameof(value));
             }
 
-            FullyQualifiedName = valueTokens[0];
-            MemberName = valueTokens[1];
+            FullyQualifiedName = value.Substring(0, sep);
+            MemberName = value.Substring(sep + 1);
         }
 
         public EnumValue(string fullyQualifiedName, string memberName) : this($"{fullyQualifiedName}/{memberName}")

diff --git a/packages/jsii-dotnet-runtime/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/packages/jsii-dotnet-runtime/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
@@ -266,6 +266,17 @@ public void GetAndSetEnumValues()
             Assert.Equal("<<[[{{(((1 * (0 + 9)) * (0 + 9)) * (0 + 9))}}]]>>", calc.ToString());
         }
 
+        [Fact(DisplayName = Prefix + nameof(EnumFromScopedModule))]
+        public void UseEnumFromScopedModule()
+        {
+            ReferenceEnumFromScopedPackage obj = new ReferenceEnumFromScopedPackage();
+            Assert.Equal(EnumFromScopedModule.Value2, obj.Foo);
+            obj.Foo = EnumFromScopedModule.Value1;
+            Assert.Equal(EnumFromScopedModule.Value1, obj.LoadFoo());
+            obj.SaveFoo(EnumFromScopedModule.Value2);
+            Assert.Equal(EnumFromScopedModule.Value2, obj.Foo);
+        }
+
         [Fact(DisplayName = Prefix + nameof(UndefinedAndNull))]
         public void UndefinedAndNull()
         {

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
@@ -24,12 +24,14 @@
 import software.amazon.jsii.tests.calculator.NumberGenerator;
 import software.amazon.jsii.tests.calculator.Polymorphism;
 import software.amazon.jsii.tests.calculator.Power;
+import software.amazon.jsii.tests.calculator.ReferenceEnumFromScopedPackage;
 import software.amazon.jsii.tests.calculator.Statics;
 import software.amazon.jsii.tests.calculator.Sum;
 import software.amazon.jsii.tests.calculator.SyncVirtualMethods;
 import software.amazon.jsii.tests.calculator.UnionProperties;
 import software.amazon.jsii.tests.calculator.UsesInterfaceWithProperties;
 import software.amazon.jsii.tests.calculator.composition.CompositionStringStyle;
+import software.amazon.jsii.tests.calculator.lib.EnumFromScopedModule;
 import software.amazon.jsii.tests.calculator.lib.IFriendly;
 import software.amazon.jsii.tests.calculator.lib.MyFirstStruct;
 import software.amazon.jsii.tests.calculator.lib.Number;
@@ -242,6 +244,16 @@ public void getAndSetEnumValues() {
         assertEquals("<<[[{{(((1 * (0 + 9)) * (0 + 9)) * (0 + 9))}}]]>>", calc.toString());
     }
 
+    @Test
+    public void useEnumFromScopedModule() {
+        ReferenceEnumFromScopedPackage obj = new ReferenceEnumFromScopedPackage();
+        assertEquals(EnumFromScopedModule.Value2, obj.getFoo());
+        obj.setFoo(EnumFromScopedModule.Value1);
+        assertEquals(EnumFromScopedModule.Value1, obj.loadFoo());
+        obj.saveFoo(EnumFromScopedModule.Value2);
+        assertEquals(EnumFromScopedModule.Value2, obj.getFoo());
+    }
+
     @Test
     public void undefinedAndNull() {
         Calculator calculator = new Calculator();

diff --git a/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java b/packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java
@@ -264,13 +264,13 @@ private JsiiObject createNative(final String fqn) {
      */
     @SuppressWarnings({ "unchecked", "rawtypes" })
     public Enum<?> findEnumValue(final String enumRef) {
-        String[] parts = enumRef.split("/");
-        if (parts.length != 2) {
+        int sep = enumRef.lastIndexOf('/');
+        if (sep == -1) {
             throw new JsiiException("Malformed enum reference: " + enumRef);
         }
 
-        String typeName = parts[0];
-        String valueName = parts[1];
+        String typeName = enumRef.substring(0, sep);
+        String valueName = enumRef.substring(sep + 1);
 
         String enumClass = resolveJavaClassName(typeName);
         try {

diff --git a/packages/jsii-kernel/lib/kernel.ts b/packages/jsii-kernel/lib/kernel.ts
@@ -855,14 +855,21 @@ export class Kernel {
         // enums
         if (typeof v === 'object' && TOKEN_ENUM in v) {
             this._debug('Enum:', v);
-            const parts = v[TOKEN_ENUM].split('/');
-            if (parts.length !== 2) {
+
+            const value = v[TOKEN_ENUM] as string;
+            const sep = value.lastIndexOf('/');
+            if (sep === -1) {
                 throw new Error(`Malformed enum value: ${v[TOKEN_ENUM]}`);
             }
-            const typeName = parts[0];
-            const valueName = parts[1];
+
+            const typeName = value.substr(0, sep);
+            const valueName = value.substr(sep + 1);
 
             const enumValue = this._findSymbol(typeName)[valueName];
+            if (enumValue === undefined) {
+                throw new Error(`No enum member named ${valueName} in ${typeName}`);
+            }
+
             this._debug('resolved enum value:', enumValue);
             return enumValue;
         }

diff --git a/packages/jsii-kernel/test/test.kernel.ts b/packages/jsii-kernel/test/test.kernel.ts
@@ -167,6 +167,29 @@ defineTest('in/out enum values', async (test, sandbox) => {
     test.deepEqual(sandbox.get({ objref: alltypes, property: 'enumProperty' }).value, { '$jsii.enum': 'jsii-calc.AllTypesEnum/ThisIsGreat' });
 });
 
+defineTest('enum values from @scoped packages awslabs/jsii#138', async (test, sandbox) => {
+    const objref = sandbox.create({ fqn: 'jsii-calc.ReferenceEnumFromScopedPackage' });
+
+    const value = sandbox.get({ objref, property: 'foo' });
+    test.deepEqual(value, { value: { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/Value2' } });
+
+    sandbox.set({ objref, property: 'foo', value: { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/Value1' }});
+    const ret = sandbox.invoke({ objref, method: 'loadFoo' });
+    test.deepEqual(ret, { result: { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/Value1' } });
+
+    sandbox.invoke({ objref, method: 'saveFoo', args: [ { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/Value2' } ] });
+    const value2 = sandbox.get({ objref, property: 'foo' });
+    test.deepEqual(value2, { value: { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/Value2' } });
+});
+
+defineTest('fails for invalid enum member name', async (test, sandbox) => {
+    const objref = sandbox.create({ fqn: 'jsii-calc.ReferenceEnumFromScopedPackage' });
+
+    test.throws(() => {
+        sandbox.set({ objref, property: 'foo', value: { '$jsii.enum': '@scope/jsii-calc-lib.EnumFromScopedModule/ValueX' }});
+    }, /No enum member named ValueX/);
+});
+
 defineTest('set for a non existing property', async (test, sandbox) => {
     const obj = sandbox.create({ fqn: 'jsii-calc.SyncVirtualMethods' });
     test.throws(() => sandbox.set({ objref: obj, property: 'idontexist', value: 'Foo' }));

diff --git a/packages/jsii-pacmak/bin/jsii-pacmak.ts b/packages/jsii-pacmak/bin/jsii-pacmak.ts
@@ -87,6 +87,8 @@ import { SPEC_FILE_NAME } from '../node_modules/jsii-spec';
             return; // already built
         }
 
+        visited.add(packageDir);
+
         // read package.json and extract the "jsii" configuration from it.
         const pkg = await fs.readJson(path.join(packageDir, 'package.json'));
         if (!pkg.jsii || !pkg.jsii.outdir || !pkg.jsii.targets) {

diff --git a/packages/jsii-pacmak/test/diff-test.sh b/packages/jsii-pacmak/test/diff-test.sh
@@ -43,7 +43,7 @@ function assert-generator() {
         echo "The diff-test.sh harness will replace it with the real expected tarball" >> ${tarball_placeholder}
     done
 
-    if ! diff -arq ${outdir} ${expected}; then
+    if ! diff --strip-trailing-cr -arq ${outdir} ${expected}; then
         echo
         echo "------------------------------------------------------------------------"
         echo " diff-test for pacmak generator ${module} failed"

diff --git a/...ges/jsii-pacmak/test/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.Calculator.Lib/.jsii b/...ges/jsii-pacmak/test/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.Calculator.Lib/.jsii
@@ -1,5 +1,5 @@
 {
-  "fingerprint": "GbVnxjsdaq598NCRdOJ8entjQRwThRmFgdeLNaQbIZY=",
+  "fingerprint": "yzHGjyI4/53o87fPNjR/Nz/4G58kg5jVoKtgwdqaEU8=",
   "author": {
     "name": "Amazon Web Services",
     "organization": true,
@@ -73,6 +73,24 @@
     }
   },
   "types": {
+    "@scope/jsii-calc-lib.EnumFromScopedModule": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "comment": "Check that enums from @scoped packages can be references.\nSee awslabs/jsii#138"
+      },
+      "fqn": "@scope/jsii-calc-lib.EnumFromScopedModule",
+      "kind": "enum",
+      "members": [
+        {
+          "name": "Value1"
+        },
+        {
+          "name": "Value2"
+        }
+      ],
+      "name": "EnumFromScopedModule",
+      "namespace": "@scope/jsii-calc-lib"
+    },
     "@scope/jsii-calc-lib.IFriendly": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {

diff --git a/...st/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.Calculator.Lib/EnumFromScopedModule.cs b/...st/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.Calculator.Lib/EnumFromScopedModule.cs
@@ -0,0 +1,17 @@
+using Amazon.JSII.Runtime.Deputy;
+
+namespace Amazon.JSII.Tests.Calculator.Lib
+{
+    /// <summary>
+    /// Check that enums from @scoped packages can be references.
+    /// See awslabs/jsii#138
+    /// </summary>
+    [JsiiEnum(typeof(EnumFromScopedModule), "@scope/jsii-calc-lib.EnumFromScopedModule")]
+    public enum EnumFromScopedModule
+    {
+        [JsiiEnumMember("Value1")]
+        Value1,
+        [JsiiEnumMember("Value2")]
+        Value2
+    }
+}
diff --git a/...ib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/EnumFromScopedModule.java b/...ib/java/src/main/java/software/amazon/jsii/tests/calculator/lib/EnumFromScopedModule.java
@@ -0,0 +1,10 @@
+package software.amazon.jsii.tests.calculator.lib;
+/**
+ * Check that enums from @scoped packages can be references.
+ * See awslabs/jsii#138
+ */
+@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.lib.$Module.class, fqn = "@scope/jsii-calc-lib.EnumFromScopedModule")
+public enum EnumFromScopedModule {
+    Value1,
+    Value2,
+}
diff --git a/packages/jsii-pacmak/test/expected.jsii-calc-lib/sphinx/_scope_jsii-calc-lib.rst b/packages/jsii-pacmak/test/expected.jsii-calc-lib/sphinx/_scope_jsii-calc-lib.rst
@@ -6,6 +6,16 @@ Reference
 
 .. py:module:: @scope/jsii-calc-lib
 
+EnumFromScopedModule (enum)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. py:class:: EnumFromScopedModule
+
+   .. py:data:: Value1
+
+   .. py:data:: Value2
+
+
 IFriendly (interface)
 ^^^^^^^^^^^^^^^^^^^^^