fix(jsii): unable to use nested types from dependencies (#1866)

Using `--project-references`, nested types from dependencies could not be used as they would result in the following `jsii` error: ``` Type "<type name>" cannot be used as the return type because it is private or @internal ``` This is because in `--project-references` mode, the ambient declaration for the nested type is the declaration that gets resolved to, and those cannot have the `export` keyword: instead, the surrounding module declaration is annotated with `export ambient`. This adds the necessary code path to actually identify this scenario and to appropriately detect the type is actually visible and exported. The check that was failing had been added in #1861 as a way to provide a more helpful error message when private or `@internal` types are mistakenly used on exported APIs; which explains why this situation did not previously occur. > Hidden gem: we were previously adding the `static` keyword when > generating nested classes in **C#**. This is a **Java**-ism and results > in code that won't compile, since **C#** does not allow `static` nested > classes to declare `protected` members (which we actually emit some > of).
aws · Aug 12, 2020 · 44f9109 · 44f9109
1 parent 41d79e7
commit 44f9109
Show file tree

Hide file tree

Showing 16 changed files with 19,616 additions and 20,441 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -59,8 +59,10 @@ jobs:
             ${{ steps.cache-locations.outputs.pip-cache }}
             ${{ steps.cache-locations.outputs.yarn-cache }}
             ~/.m2/repository
+            !~/.m2/repository/software/amazon/jsii/
             ~/.nuget/packages
-          key: ${{ runner.os }}-node@[email protected]${{ hashFiles('**/yarn.lock') }}
+            !~/.nuget/packages/amazon.jsii.*
+          key: ${{ runner.os }}-node@[email protected]${{ hashFiles('**/yarn.lock', '**/Directory.Build.targets') }}
           restore-keys: |-
             ${{ runner.os }}-node@[email protected]
             ${{ runner.os }}-node@12-
@@ -130,8 +132,10 @@ jobs:
             ${{ steps.cache-locations.outputs.pip-cache }}
             ${{ steps.cache-locations.outputs.yarn-cache }}
             ~/.m2/repository
+            !~/.m2/repository/software/amazon/jsii/
             ~/.nuget/packages
-          key: ${{ runner.os }}-node@[email protected]${{ hashFiles('**/yarn.lock') }}
+            !~/.nuget/packages/amazon.jsii.*
+          key: ${{ runner.os }}-node@[email protected]${{ hashFiles('**/yarn.lock', '**/Directory.Build.targets') }}
           restore-keys: |-
             ${{ runner.os }}-node@[email protected]
             ${{ runner.os }}-node@12-
@@ -276,9 +280,11 @@ jobs:
             ${{ steps.cache-locations.outputs.pip-cache }}
             ${{ steps.cache-locations.outputs.yarn-cache }}
             ~/.m2/repository
+            !~/.m2/repository/software/amazon/jsii/
             ~/.nuget/packages
+            !~/.nuget/packages/amazon.jsii.*
           # Not including .NET / Java in the cache keys, those artifacts are SDK-version-independent
-          key: ${{ runner.os }}-node@${{ matrix.node }}-python@${{ matrix.python }}-${{ hashFiles('**/yarn.lock') }}
+          key: ${{ runner.os }}-node@${{ matrix.node }}-python@${{ matrix.python }}-${{ hashFiles('**/yarn.lock', '**/Directory.Build.targets') }}
           restore-keys: |-
             ${{ runner.os }}-node@${{ matrix.node }}-python@${{ matrix.python }}-
             ${{ runner.os }}-node@${{ matrix.node }}-
@@ -353,6 +359,10 @@ jobs:
       # Run the integration test
       - name: Install Dependencies
         run: |-
+          # Python tools used during packaging
+          python3 -m pip install --upgrade pipx setuptools twine wheel
+
+          # TypeScript project dependencies
           yarn install --frozen-lockfile
         working-directory: aws-cdk
       - name: Install Tested Packages

diff --git a/packages/@scope/jsii-calc-lib/lib/submodule/index.ts b/packages/@scope/jsii-calc-lib/lib/submodule/index.ts
@@ -8,15 +8,26 @@ export interface ReflectableEntry {
 }
 
 export class Reflector {
-  public constructor() { }
-
   public asMap(reflectable: IReflectable): Record<string, unknown> {
-    return reflectable.entries.reduce(
-      (mapping, entry) => {
-        mapping[entry.key] = entry.value;
-        return mapping;
-      },
-      {} as Record<string, unknown>,
-    );
+    return reflectable.entries.reduce((mapping, entry) => {
+      mapping[entry.key] = entry.value;
+      return mapping;
+    }, {} as Record<string, unknown>);
+  }
+}
+
+/**
+ * This class is here to show we can use nested classes across module boundaries.
+ */
+export class NestingClass {
+  private constructor() {}
+}
+// eslint-disable-next-line @typescript-eslint/no-namespace
+export namespace NestingClass {
+  /**
+   * This class is here to show we can use nested classes across module boundaries.
+   */
+  export class NestedClass {
+    public readonly property: string = 'property';
   }
 }
diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii
@@ -574,6 +574,57 @@
         }
       ]
     },
+    "@scope/jsii-calc-lib.submodule.NestingClass": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "stability": "deprecated",
+        "summary": "This class is here to show we can use nested classes across module boundaries."
+      },
+      "fqn": "@scope/jsii-calc-lib.submodule.NestingClass",
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/submodule/index.ts",
+        "line": 22
+      },
+      "name": "NestingClass",
+      "namespace": "submodule"
+    },
+    "@scope/jsii-calc-lib.submodule.NestingClass.NestedClass": {
+      "assembly": "@scope/jsii-calc-lib",
+      "docs": {
+        "stability": "deprecated",
+        "summary": "This class is here to show we can use nested classes across module boundaries."
+      },
+      "fqn": "@scope/jsii-calc-lib.submodule.NestingClass.NestedClass",
+      "initializer": {
+        "docs": {
+          "stability": "deprecated"
+        }
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/submodule/index.ts",
+        "line": 30
+      },
+      "name": "NestedClass",
+      "namespace": "submodule.NestingClass",
+      "properties": [
+        {
+          "docs": {
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/submodule/index.ts",
+            "line": 31
+          },
+          "name": "property",
+          "type": {
+            "primitive": "string"
+          }
+        }
+      ]
+    },
     "@scope/jsii-calc-lib.submodule.ReflectableEntry": {
       "assembly": "@scope/jsii-calc-lib",
       "datatype": true,
@@ -630,10 +681,6 @@
       "initializer": {
         "docs": {
           "stability": "deprecated"
-        },
-        "locationInModule": {
-          "filename": "lib/submodule/index.ts",
-          "line": 11
         }
       },
       "kind": "class",
@@ -648,7 +695,7 @@
           },
           "locationInModule": {
             "filename": "lib/submodule/index.ts",
-            "line": 13
+            "line": 11
           },
           "name": "asMap",
           "parameters": [
@@ -676,5 +723,5 @@
     }
   },
   "version": "0.0.0",
-  "fingerprint": "fVfpIK7xUajlT1zkHIJ8uYJPvy0gLgEe5BM8afu1mVg="
+  "fingerprint": "f/4VuNiOkSgTgLR80loQUAzAuzFi+25rmfLcRWKDCrY="
 }
diff --git a/packages/jsii-calc/lib/index.ts b/packages/jsii-calc/lib/index.ts
@@ -2,6 +2,7 @@ export * from './calculator';
 export * from './compliance';
 export * from './documented';
 export * from './erasures';
+export * from './nested-class';
 export * from './stability';
 export * from './submodules';
 

diff --git a/packages/jsii-calc/lib/nested-class.ts b/packages/jsii-calc/lib/nested-class.ts
@@ -0,0 +1,9 @@
+import { submodule } from '@scope/jsii-calc-lib';
+
+export class NestedClassInstance {
+  public static makeInstance(): submodule.NestingClass.NestedClass {
+    return new submodule.NestingClass.NestedClass();
+  }
+
+  private constructor() {}
+}
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -196,7 +196,7 @@
     "jsii-calc.submodule": {
       "locationInModule": {
         "filename": "lib/index.ts",
-        "line": 8
+        "line": 9
       }
     },
     "jsii-calc.submodule.back_references": {
@@ -8439,6 +8439,37 @@
         }
       ]
     },
+    "jsii-calc.NestedClassInstance": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "experimental"
+      },
+      "fqn": "jsii-calc.NestedClassInstance",
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/nested-class.ts",
+        "line": 3
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/nested-class.ts",
+            "line": 4
+          },
+          "name": "makeInstance",
+          "returns": {
+            "type": {
+              "fqn": "@scope/jsii-calc-lib.submodule.NestingClass.NestedClass"
+            }
+          },
+          "static": true
+        }
+      ],
+      "name": "NestedClassInstance"
+    },
     "jsii-calc.NestedStruct": {
       "assembly": "jsii-calc",
       "datatype": true,
@@ -13788,5 +13819,5 @@
     }
   },
   "version": "0.0.0",
-  "fingerprint": "kQYWZjxtnycywR9qo/KXKyeVPmP6HoAefGhHN7SidkM="
+  "fingerprint": "NsqdwWgXi+kjrpLQtQ27eA/znULJ7TtXy03ht68N9Ms="
 }
diff --git a/packages/jsii-pacmak/lib/targets/dotnet.ts b/packages/jsii-pacmak/lib/targets/dotnet.ts
@@ -53,7 +53,7 @@ export class DotnetBuilder implements TargetBuilder {
 
       // Build solution
       logging.debug('Building .NET');
-      await shell('dotnet', ['build', '-c', 'Release'], {
+      await shell('dotnet', ['build', '--force', '-c', 'Release'], {
         cwd: tempSourceDir.directory,
       });
 

diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
@@ -283,7 +283,6 @@ export class DotNetGenerator extends Generator {
 
     // Nested classes will be dealt with during calc code generation
     const nested = this.isNested(cls);
-    const inner = nested ? ' static' : '';
     const absPrefix = abstract ? ' abstract' : '';
 
     this.openFileIfNeeded(className, namespace, nested);
@@ -294,7 +293,7 @@ export class DotNetGenerator extends Generator {
     this.dotnetRuntimeGenerator.emitAttributesForClass(cls);
 
     this.code.openBlock(
-      `public${inner}${absPrefix} class ${className}${implementsExpr}`,
+      `public${absPrefix} class ${className}${implementsExpr}`,
     );
 
     // Compute the class parameters

diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts
@@ -48,13 +48,25 @@ export default class Python extends Target {
     await shell('python3', ['setup.py', 'sdist', '--dist-dir', outDir], {
       cwd: sourceDir,
     });
-    await shell('python3', ['setup.py', 'bdist_wheel', '--dist-dir', outDir], {
-      cwd: sourceDir,
-    });
+    await shell(
+      'python3',
+      ['-m', 'pip', 'wheel', '--no-deps', '--wheel-dir', outDir, sourceDir],
+      {
+        cwd: sourceDir,
+      },
+    );
     if (await isPresent('twine', sourceDir)) {
       await shell('twine', ['check', path.join(outDir, '*')], {
         cwd: sourceDir,
       });
+    } else if (await isPresent('pipx', sourceDir)) {
+      await shell('pipx', ['run', 'twine', 'check', path.join(outDir, '*')], {
+        cwd: sourceDir,
+        env: {
+          ...process.env,
+          PIPX_HOME: path.join(sourceDir, '.pipx'),
+        },
+      });
     } else {
       warn(
         'Unable to validate distribution packages because `twine` is not present. ' +
@@ -1649,6 +1661,9 @@ class Package {
         (this.metadata.author.email !== undefined
           ? `<${this.metadata.author.email}>`
           : ''),
+      bdist_wheel: {
+        universal: true,
+      },
       project_urls: {
         Source: this.metadata.repository.url,
       },
@@ -1659,7 +1674,9 @@ class Package {
       install_requires: [
         `jsii${toPythonVersionRange(`^${jsiiVersionSimple}`)}`,
         'publication>=0.0.3',
-      ].concat(dependencies),
+      ]
+        .concat(dependencies)
+        .sort(),
       classifiers: [
         'Intended Audience :: Developers',
         'Operating System :: OS Independent',
@@ -1719,7 +1736,7 @@ class Package {
     // TODO: Might be easier to just use a TOML library to write this out.
     code.openFile('pyproject.toml');
     code.line('[build-system]');
-    code.line('requires = ["setuptools >= 38.6.0", "wheel >= 0.31.0"]');
+    code.line('requires = ["setuptools >= 49.3.1", "wheel >= 0.34.2"]');
     code.line('build-backend = "setuptools.build_meta"');
     code.closeFile('pyproject.toml');
 

diff --git a/packages/jsii-pacmak/package.json b/packages/jsii-pacmak/package.json
@@ -30,8 +30,9 @@
     "watch": "tsc --build -w",
     "lint": "eslint . --ext .js,.ts --ignore-path=.gitignore",
     "lint:fix": "yarn lint --fix",
-    "test": "jest && bash test/build-test.sh",
-    "test:update": "jest -u && bash test/build-test.sh",
+    "test": "jest && npm run test:build",
+    "test:build": "bash test/build-test.sh",
+    "test:update": "jest -u && npm run test:build",
     "package": "package-js"
   },
   "dependencies": {