fix(runtime): type not found (#3763)

When a jsii module returned a private class implementation of an interface, consuming jsii.rtti data may return an FQN for the internal class, which does not exist in foreign languages, leading to an error. Revert to tagging FQNs directly instead. Fixes #3742
aws · Sep 22, 2022 · c7dfad3 · c7dfad3
1 parent ad085eb
commit c7dfad3
Show file tree

Hide file tree

Showing 25 changed files with 589 additions and 36 deletions.
diff --git a/.all-contributorsrc b/.all-contributorsrc
@@ -1457,6 +1457,15 @@
       "contributions": [
         "review"
       ]
+    },
+    {
+      "login": "cn-cit",
+      "name": "cn-cit",
+      "avatar_url": "https://avatars.githubusercontent.com/u/27255477?v=4",
+      "profile": "https://github.com/cn-cit",
+      "contributions": [
+        "bug"
+      ]
     }
   ],
   "repoType": "github",

diff --git a/README.md b/README.md
@@ -228,23 +228,24 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
     </tr>
     <tr>
       <td align="center"><a href="https://github.com/arnogeurts-sqills"><img src="https://avatars.githubusercontent.com/u/79304871?v=4?s=100" width="100px;" alt=""/><br /><sub><b>arnogeurts-sqills</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Aarnogeurts-sqills+label%3Abug" title="Bug reports">🐛</a> <a href="https://github.com/aws/jsii/commits?author=arnogeurts-sqills" title="Code">💻</a></td>
+      <td align="center"><a href="https://github.com/cn-cit"><img src="https://avatars.githubusercontent.com/u/27255477?v=4?s=100" width="100px;" alt=""/><br /><sub><b>cn-cit</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Acn-cit+label%3Abug" title="Bug reports">🐛</a></td>
       <td align="center"><a href="https://github.com/deccy-mcc"><img src="https://avatars0.githubusercontent.com/u/45844893?v=4?s=100" width="100px;" alt=""/><br /><sub><b>deccy-mcc</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Adeccy-mcc+label%3Abug" title="Bug reports">🐛</a></td>
       <td align="center"><a href="https://github.com/apps/dependabot-preview"><img src="https://avatars3.githubusercontent.com/in/2141?v=4?s=100" width="100px;" alt=""/><br /><sub><b>dependabot-preview[bot]</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Adependabot-preview[bot]+label%3Abug" title="Bug reports">🐛</a> <a href="https://github.com/aws/jsii/pulls?q=is%3Apr+author%3Adependabot-preview[bot]" title="Maintenance">🚧</a></td>
       <td align="center"><a href="https://github.com/apps/dependabot"><img src="https://avatars0.githubusercontent.com/in/29110?v=4?s=100" width="100px;" alt=""/><br /><sub><b>dependabot[bot]</b></sub></a><br /><a href="https://github.com/aws/jsii/pulls?q=is%3Apr+author%3Adependabot[bot]" title="Maintenance">🚧</a></td>
       <td align="center"><a href="https://github.com/dheffx"><img src="https://avatars0.githubusercontent.com/u/22029918?v=4?s=100" width="100px;" alt=""/><br /><sub><b>dheffx</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Adheffx+label%3Abug" title="Bug reports">🐛</a></td>
       <td align="center"><a href="https://github.com/gregswdl"><img src="https://avatars0.githubusercontent.com/u/47365273?v=4?s=100" width="100px;" alt=""/><br /><sub><b>gregswdl</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Agregswdl+label%3Abug" title="Bug reports">🐛</a></td>
-      <td align="center"><a href="https://github.com/guyroberts21"><img src="https://avatars.githubusercontent.com/u/47118902?v=4?s=100" width="100px;" alt=""/><br /><sub><b>guyroberts21</b></sub></a><br /><a href="https://github.com/aws/jsii/commits?author=guyroberts21" title="Documentation">📖</a></td>
     </tr>
     <tr>
+      <td align="center"><a href="https://github.com/guyroberts21"><img src="https://avatars.githubusercontent.com/u/47118902?v=4?s=100" width="100px;" alt=""/><br /><sub><b>guyroberts21</b></sub></a><br /><a href="https://github.com/aws/jsii/commits?author=guyroberts21" title="Documentation">📖</a></td>
       <td align="center"><a href="https://github.com/mattBrzezinski"><img src="https://avatars.githubusercontent.com/u/4356074?v=4?s=100" width="100px;" alt=""/><br /><sub><b>mattBrzezinski</b></sub></a><br /><a href="https://github.com/aws/jsii/commits?author=mattBrzezinski" title="Documentation">📖</a></td>
       <td align="center"><a href="https://github.com/mergify"><img src="https://avatars.githubusercontent.com/u/18240476?v=4?s=100" width="100px;" alt=""/><br /><sub><b>mergify</b></sub></a><br /><a href="https://github.com/aws/jsii/pulls?q=is%3Apr+author%3Amergify" title="Maintenance">🚧</a></td>
       <td align="center"><a href="https://github.com/apps/mergify"><img src="https://avatars1.githubusercontent.com/in/10562?v=4?s=100" width="100px;" alt=""/><br /><sub><b>mergify[bot]</b></sub></a><br /><a href="https://github.com/aws/jsii/pulls?q=is%3Apr+author%3Amergify[bot]" title="Maintenance">🚧</a></td>
       <td align="center"><a href="https://github.com/nathannaveen"><img src="https://avatars.githubusercontent.com/u/42319948?v=4?s=100" width="100px;" alt=""/><br /><sub><b>nathannaveen</b></sub></a><br /><a href="https://github.com/aws/jsii/pulls?q=is%3Apr+author%3Anathannaveen" title="Maintenance">🚧</a></td>
       <td align="center"><a href="https://github.com/seiyashima"><img src="https://avatars2.githubusercontent.com/u/4947101?v=4?s=100" width="100px;" alt=""/><br /><sub><b>seiyashima42</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Aseiyashima+label%3Abug" title="Bug reports">🐛</a> <a href="https://github.com/aws/jsii/commits?author=seiyashima" title="Code">💻</a> <a href="https://github.com/aws/jsii/commits?author=seiyashima" title="Documentation">📖</a></td>
       <td align="center"><a href="https://github.com/sullis"><img src="https://avatars3.githubusercontent.com/u/30938?v=4?s=100" width="100px;" alt=""/><br /><sub><b>sullis</b></sub></a><br /><a href="https://github.com/aws/jsii/commits?author=sullis" title="Code">💻</a></td>
-      <td align="center"><a href="https://github.com/vaneek"><img src="https://avatars1.githubusercontent.com/u/8113305?v=4?s=100" width="100px;" alt=""/><br /><sub><b>vaneek</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Avaneek+label%3Abug" title="Bug reports">🐛</a></td>
     </tr>
     <tr>
+      <td align="center"><a href="https://github.com/vaneek"><img src="https://avatars1.githubusercontent.com/u/8113305?v=4?s=100" width="100px;" alt=""/><br /><sub><b>vaneek</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Avaneek+label%3Abug" title="Bug reports">🐛</a></td>
       <td align="center"><a href="https://github.com/wendysophie"><img src="https://avatars.githubusercontent.com/u/54415551?v=4?s=100" width="100px;" alt=""/><br /><sub><b>wendysophie</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Awendysophie+label%3Abug" title="Bug reports">🐛</a></td>
     </tr>
   </tbody>

diff --git a/gh-pages/content/specification/6-compliance-report.md b/gh-pages/content/specification/6-compliance-report.md
@@ -5,7 +5,7 @@
 This section details the current state of each language binding with respect to our standard compliance suite.
 
 
-| number | test                                                                                                                                                             | java (98.33%) | golang (79.17%)                              | Dotnet | Python |
+| number | test                                                                                                                                                             | java (98.35%) | golang (79.34%)                              | Dotnet | Python |
 | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | -------------------------------------------- | ------ | ------ |
 | 1      | asyncOverrides_overrideCallsSuper                                                                                                                                | 🟢            | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕      | ⭕      |
 | 2      | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements")                                                                  | 🟢            | 🟢                                           | ⭕      | ⭕      |
@@ -127,3 +127,4 @@ This section details the current state of each language binding with respect to
 | 118    | [callbackParameterIsInterface]("Validates pure interfaces can be passed to callbacks")                                                                           | ⭕             | 🟢                                           | ⭕      | ⭕      |
 | 119    | [classCanBeUsedWhenNotExpressedlyLoaded]("Validates that types not explicitly loaded by the user can safely be returned by JS code")                             | 🟢            | 🟢                                           | ⭕      | ⭕      |
 | 120    | [downcasting]("Ensures unsafe-cast features work as expected")                                                                                                   | ⭕             | 🟢                                           | ⭕      | ⭕      |
+| 121    | [strippedDeprecatedMemberCanBeReceived]("Ensures --strip-deprecated does not cause odd runtime errors")                                                          | 🟢            | 🟢                                           | ⭕      | ⭕      |
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
@@ -7,6 +7,7 @@
 using CompositeOperation = Amazon.JSII.Tests.CalculatorNamespace.Composition.CompositeOperation;
 using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace;
 using Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace;
+using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.DeprecationRemoval;
 using Newtonsoft.Json.Linq;
 using Xunit;
 using Xunit.Abstractions;
@@ -1534,9 +1535,15 @@ public void ClassCanBeUsedWhenNotExpressedlyLoaded()
         }
 
         private sealed class Cdk16625Impl: Cdk16625 {
-                protected override double Unwrap(IRandomNumberGenerator rng) {
-                    return rng.Next();
-                }
+            protected override double Unwrap(IRandomNumberGenerator rng) {
+                return rng.Next();
             }
+        }
+
+        [Fact(DisplayName = Prefix + nameof(StrippedDeprecatedMemberCanBeReceived))]
+        public void StrippedDeprecatedMemberCanBeReceived()
+        {
+            Assert.NotNull(InterfaceFactory.Create());
+        }
     }
 }
diff --git a/packages/@jsii/go-runtime-test/project/compliance_test.go b/packages/@jsii/go-runtime-test/project/compliance_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/submodule/child"
 	calclib "github.com/aws/jsii/jsii-calc/go/scopejsiicalclib"
 	"github.com/aws/jsii/jsii-calc/go/scopejsiicalclib/customsubmodulename"
+	"github.com/aws/jsii/jsii-calc/go/scopejsiicalclib/deprecationremoval"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
@@ -1655,6 +1656,12 @@ func (suite *ComplianceSuite) TestDownCasting() {
 	require.Equal(realValue.Foo(), jsii.Number(1337))
 }
 
+func (suite *ComplianceSuite) TestStrippedDeprecatedMemberCanBeReceived() {
+	require := suite.Require()
+
+	require.NotNil(deprecationremoval.InterfaceFactory_Create())
+}
+
 // required to make `go test` recognize the suite.
 func TestComplianceSuite(t *testing.T) {
 	suite.Run(t, new(ComplianceSuite))

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
@@ -3,6 +3,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import software.amazon.jsii.ComplianceSuiteHarness;
@@ -14,6 +15,7 @@
 import software.amazon.jsii.tests.calculator.cdk16625.Cdk16625;
 import software.amazon.jsii.tests.calculator.composition.CompositeOperation;
 import software.amazon.jsii.tests.calculator.custom_submodule_name.NestingClass.NestedStruct;
+import software.amazon.jsii.tests.calculator.lib.deprecation_removal.InterfaceFactory;
 import software.amazon.jsii.tests.calculator.lib.EnumFromScopedModule;
 import software.amazon.jsii.tests.calculator.lib.IFriendly;
 import software.amazon.jsii.tests.calculator.lib.MyFirstStruct;
@@ -24,10 +26,6 @@
 
 import java.io.IOException;
 import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.time.Instant;
@@ -1790,13 +1788,17 @@ public void iso8601DoesNotDeserializeToDate() {
         final String nowAsISO = df.format(new Date());
 
         final IWallClock wallClock = new IWallClock() {
+            @NotNull
+            @Override
             public String iso8601Now() {
                 return nowAsISO;
             }
         };
 
         final Entropy entropy = new Entropy(wallClock) {
-            public String repeat(final String word) {
+            @NotNull
+            @Override
+            public String repeat(@NotNull final String word) {
                 return word;
             }
         };
@@ -1807,11 +1809,17 @@ public String repeat(final String word) {
     @Test
     public void classCanBeUsedWhenNotExpressedlyLoaded() {
         final Cdk16625 subject = new Cdk16625() {
+            @NotNull
             @Override
             protected java.lang.Number unwrap(final IRandomNumberGenerator rng) {
                 return rng.next();
             }
         };
         subject.test();
     }
+
+    @Test
+    public void strippedDeprecatedMemberCanBeReceived() {
+        assertNotNull(InterfaceFactory.create());
+    }
 }
diff --git a/packages/@jsii/kernel/src/kernel.ts b/packages/@jsii/kernel/src/kernel.ts
@@ -583,7 +583,7 @@ export class Kernel {
         case spec.TypeKind.Class:
         case spec.TypeKind.Enum:
           const constructor = this._findSymbol(fqn);
-          tagJsiiConstructor(constructor, fqn, assm.metadata.version);
+          tagJsiiConstructor(constructor, fqn);
       }
     }
   }

diff --git a/packages/@jsii/kernel/src/objects.ts b/packages/@jsii/kernel/src/objects.ts
@@ -1,4 +1,5 @@
 import * as spec from '@jsii/spec';
+import * as assert from 'assert';
 
 import * as api from './api';
 import { JsiiFault } from './kernel';
@@ -15,15 +16,12 @@ const OBJID_SYMBOL = Symbol.for('$__jsii__objid__$');
 const IFACES_SYMBOL = Symbol.for('$__jsii__interfaces__$');
 
 /**
- * Symbol we use to tag the constructor of a JSII class
+ * Symbol we use to tag constructors that are exported from a JSII module.
  */
-const JSII_RTTI_SYMBOL = Symbol.for('jsii.rtti');
+const JSII_TYPE_FQN_SYMBOL = Symbol('$__jsii__fqn__$');
 
 interface ManagedConstructor {
-  readonly [JSII_RTTI_SYMBOL]: {
-    readonly fqn: string;
-    readonly version: string;
-  };
+  readonly [JSII_TYPE_FQN_SYMBOL]: string;
 }
 
 type MaybeManagedConstructor = Partial<ManagedConstructor>;
@@ -36,7 +34,7 @@ type MaybeManagedConstructor = Partial<ManagedConstructor>;
  * information.
  */
 export function jsiiTypeFqn(obj: any): string | undefined {
-  return (obj.constructor as MaybeManagedConstructor)[JSII_RTTI_SYMBOL]?.fqn;
+  return (obj.constructor as MaybeManagedConstructor)[JSII_TYPE_FQN_SYMBOL];
 }
 
 /**
@@ -96,19 +94,21 @@ function tagObject(obj: unknown, objid: string, interfaces?: string[]) {
 /**
  * Set the JSII FQN for classes produced by a given constructor
  */
-export function tagJsiiConstructor(
-  constructor: any,
-  fqn: string,
-  version: string,
-) {
-  if (Object.prototype.hasOwnProperty.call(constructor, JSII_RTTI_SYMBOL)) {
-    return;
+export function tagJsiiConstructor(constructor: any, fqn: string) {
+  if (Object.prototype.hasOwnProperty.call(constructor, JSII_TYPE_FQN_SYMBOL)) {
+    return assert(
+      constructor[JSII_TYPE_FQN_SYMBOL] === fqn,
+      `Unable to register ${constructor.name} as ${fqn}: it is already registerd with FQN ${constructor[JSII_TYPE_FQN_SYMBOL]}`,
+    );
   }
-  Object.defineProperty(constructor, JSII_RTTI_SYMBOL, {
+
+  // Mark this constructor as exported from a jsii module, so we know we
+  // should be considering it's FQN as a valid exported type.
+  Object.defineProperty(constructor, JSII_TYPE_FQN_SYMBOL, {
     configurable: false,
     enumerable: false,
     writable: false,
-    value: { fqn, version },
+    value: fqn,
   });
 }
 

diff --git a/packages/@jsii/python-runtime/tests/test_compliance.py b/packages/@jsii/python-runtime/tests/test_compliance.py
@@ -86,6 +86,7 @@
 from jsii_calc.submodule.child import SomeEnum
 from scope.jsii_calc_lib import IFriendly, EnumFromScopedModule, Number
 from scope.jsii_calc_lib.custom_submodule_name import IReflectable, ReflectableEntry
+from scope.jsii_calc_lib.deprecation_removal import InterfaceFactory
 
 # Note: The names of these test functions have been chosen to map as closely to the
 #       Java Compliance tests as possible.
@@ -1343,3 +1344,7 @@ def _unwrap(self, rng: IRandomNumberGenerator):
 
     # This should NOT throw
     Subject().test()
+
+
+def test_stripped_deprecated_member_can_be_received():
+    assert InterfaceFactory.create() is not None
diff --git a/packages/@scope/jsii-calc-lib/deprecated-to-strip.txt b/packages/@scope/jsii-calc-lib/deprecated-to-strip.txt
@@ -0,0 +1 @@
+@scope/jsii-calc-lib.deprecationRemoval.DeprecatedImplementation
diff --git a/packages/@scope/jsii-calc-lib/lib/deprecation-removal.ts b/packages/@scope/jsii-calc-lib/lib/deprecation-removal.ts
@@ -0,0 +1,16 @@
+export interface IInterface {
+  method(): void;
+}
+
+/** @deprecated do not use me! */
+export class DeprecatedImplementation implements IInterface {
+  public method(): void { }
+}
+
+export class InterfaceFactory {
+  public static create(): IInterface {
+    return new DeprecatedImplementation();
+  }
+
+  private constructor() { }
+}
diff --git a/packages/@scope/jsii-calc-lib/lib/index.ts b/packages/@scope/jsii-calc-lib/lib/index.ts
@@ -129,3 +129,4 @@ export class BaseFor2647 {
 
 export * as submodule from './submodule';
 export * from './duplicate-inherited-prop';
+export * as deprecationRemoval from './deprecation-removal';
diff --git a/packages/@scope/jsii-calc-lib/package.json b/packages/@scope/jsii-calc-lib/package.json
@@ -26,7 +26,7 @@
   "main": "build/index.js",
   "types": "build/index.d.ts",
   "scripts": {
-    "build": "jsii --project-references && jsii-rosetta",
+    "build": "jsii --project-references --strip-deprecated=deprecated-to-strip.txt && jsii-rosetta",
     "pacmak": "jsii-pacmak",
     "test": "diff-test test/assembly.jsii .jsii",
     "test:update": "npm run build && UPDATE_DIFF=1 npm run test"

diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii
@@ -92,6 +92,13 @@
   },
   "schema": "jsii/0.10.0",
   "submodules": {
+    "@scope/jsii-calc-lib.deprecationRemoval": {
+      "locationInModule": {
+        "filename": "lib/index.ts",
+        "line": 132
+      },
+      "symbolId": "lib/deprecation-removal:"
+    },
     "@scope/jsii-calc-lib.submodule": {
       "locationInModule": {
         "filename": "lib/index.ts",
@@ -729,6 +736,67 @@
       ],
       "symbolId": "lib/index:StructWithOnlyOptionals"
     },
+    "@scope/jsii-calc-lib.deprecationRemoval.IInterface": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "stability": "deprecated"
+      },
+      "fqn": "@scope/jsii-calc-lib.deprecationRemoval.IInterface",
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/deprecation-removal.ts",
+        "line": 1
+      },
+      "methods": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "deprecated"
+          },
+          "locationInModule": {
+            "filename": "lib/deprecation-removal.ts",
+            "line": 2
+          },
+          "name": "method"
+        }
+      ],
+      "name": "IInterface",
+      "namespace": "deprecationRemoval",
+      "symbolId": "lib/deprecation-removal:IInterface"
+    },
+    "@scope/jsii-calc-lib.deprecationRemoval.InterfaceFactory": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "stability": "deprecated"
+      },
+      "fqn": "@scope/jsii-calc-lib.deprecationRemoval.InterfaceFactory",
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/deprecation-removal.ts",
+        "line": 10
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "deprecated"
+          },
+          "locationInModule": {
+            "filename": "lib/deprecation-removal.ts",
+            "line": 11
+          },
+          "name": "create",
+          "returns": {
+            "type": {
+              "fqn": "@scope/jsii-calc-lib.deprecationRemoval.IInterface"
+            }
+          },
+          "static": true
+        }
+      ],
+      "name": "InterfaceFactory",
+      "namespace": "deprecationRemoval",
+      "symbolId": "lib/deprecation-removal:InterfaceFactory"
+    },
     "@scope/jsii-calc-lib.submodule.IReflectable": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {
@@ -954,5 +1022,5 @@
     }
   },
   "version": "0.0.0",
-  "fingerprint": "kYLYyNyPod3JTyJWmgPJL1Z85k0HEEhQeMIs4zH4bEQ="
+  "fingerprint": "XDMAZYhhgc09X8VS8hpn3ch21YxKDn+HB0w82IofsRM="
 }