fix(pacmak): multiple go (and python) issues (#2622)

The `go build` command we run during pacmak did not descend into submodules. After adding `./...` to the `go build` command, a few other issues were surfaced related to static member handling. The common issue was that `GoClass` had two separate lists (for static and non-static methods/props). This caused various places in the implementation to only take into account non-static members. Added a new list called `members` which collected both static and non-static members and used it in the various dependency props. An empty module in Python (with only submodules) will result in an empty `__all__` list which yields a MyPy error. Fixes #2618 - go build does not build submodule directories Fixes #2619 - go/python module with only submodules (no direct types) fails compilation Fixes #2620 - go missing imports if class includes only static members Fixes #2621 - invalid go imports when types referenced by static members
aws · Mar 1, 2021 · c2bd156 · c2bd156
1 parent adfe8b9
commit c2bd156
Show file tree

Hide file tree

Showing 17 changed files with 895 additions and 27 deletions.
diff --git a/packages/jsii-calc/lib/index.ts b/packages/jsii-calc/lib/index.ts
@@ -9,3 +9,5 @@ export * from './stability';
 export * from './submodules';
 
 export * as submodule from './submodule';
+export * as onlystatic from './only-static';
+export * as nodirect from './no-direct-types';
diff --git a/packages/jsii-calc/lib/no-direct-types/index.ts b/packages/jsii-calc/lib/no-direct-types/index.ts
@@ -0,0 +1,4 @@
+// THIS MODULE ONLY EXPORTS SUBMODULES: it tests the case in which a module does not export direct
+// types but rather only exports submodules (see https://github.com/aws/jsii/issues/2619)
+export * as sub1 from './sub1';
+export * as sub2 from './sub2';
diff --git a/packages/jsii-calc/lib/no-direct-types/sub1/index.ts b/packages/jsii-calc/lib/no-direct-types/sub1/index.ts
@@ -0,0 +1,5 @@
+export class TypeFromSub1 {
+  public sub1() {
+    return 'foo';
+  }
+}
diff --git a/packages/jsii-calc/lib/no-direct-types/sub2/index.ts b/packages/jsii-calc/lib/no-direct-types/sub2/index.ts
@@ -0,0 +1,5 @@
+export class TypeFromSub2 {
+  public sub2() {
+    return 'foo';
+  }
+}
diff --git a/packages/jsii-calc/lib/only-static/index.ts b/packages/jsii-calc/lib/only-static/index.ts
@@ -0,0 +1,10 @@
+/**
+ * Test for https://github.com/aws/jsii/issues/2617
+ */
+export class OnlyStaticMethods {
+  public static staticMethod() {
+    return 'hello';
+  }
+
+  private constructor() {}
+}
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -206,6 +206,30 @@
         "line": 142
       }
     },
+    "jsii-calc.nodirect": {
+      "locationInModule": {
+        "filename": "lib/index.ts",
+        "line": 13
+      }
+    },
+    "jsii-calc.nodirect.sub1": {
+      "locationInModule": {
+        "filename": "lib/no-direct-types/index.ts",
+        "line": 3
+      }
+    },
+    "jsii-calc.nodirect.sub2": {
+      "locationInModule": {
+        "filename": "lib/no-direct-types/index.ts",
+        "line": 4
+      }
+    },
+    "jsii-calc.onlystatic": {
+      "locationInModule": {
+        "filename": "lib/index.ts",
+        "line": 12
+      }
+    },
     "jsii-calc.submodule": {
       "locationInModule": {
         "filename": "lib/index.ts",
@@ -13860,6 +13884,111 @@
       "name": "CompositionStringStyle",
       "namespace": "composition.CompositeOperation"
     },
+    "jsii-calc.nodirect.sub1.TypeFromSub1": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "stable"
+      },
+      "fqn": "jsii-calc.nodirect.sub1.TypeFromSub1",
+      "initializer": {
+        "docs": {
+          "stability": "stable"
+        }
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/no-direct-types/sub1/index.ts",
+        "line": 1
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/no-direct-types/sub1/index.ts",
+            "line": 2
+          },
+          "name": "sub1",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        }
+      ],
+      "name": "TypeFromSub1",
+      "namespace": "nodirect.sub1"
+    },
+    "jsii-calc.nodirect.sub2.TypeFromSub2": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "stable"
+      },
+      "fqn": "jsii-calc.nodirect.sub2.TypeFromSub2",
+      "initializer": {
+        "docs": {
+          "stability": "stable"
+        }
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/no-direct-types/sub2/index.ts",
+        "line": 1
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/no-direct-types/sub2/index.ts",
+            "line": 2
+          },
+          "name": "sub2",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        }
+      ],
+      "name": "TypeFromSub2",
+      "namespace": "nodirect.sub2"
+    },
+    "jsii-calc.onlystatic.OnlyStaticMethods": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "stable",
+        "summary": "Test for https://github.com/aws/jsii/issues/2617."
+      },
+      "fqn": "jsii-calc.onlystatic.OnlyStaticMethods",
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/only-static/index.ts",
+        "line": 4
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/only-static/index.ts",
+            "line": 5
+          },
+          "name": "staticMethod",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          },
+          "static": true
+        }
+      ],
+      "name": "OnlyStaticMethods",
+      "namespace": "onlystatic"
+    },
     "jsii-calc.submodule.MyClass": {
       "assembly": "jsii-calc",
       "docs": {
@@ -14477,5 +14606,5 @@
     }
   },
   "version": "3.20.120",
-  "fingerprint": "DPUvV1cBxUeRNl5GK1DVzpU7/802/Hb8znpxKRttRUk="
+  "fingerprint": "JMTvoEnsqFjh72kI95PtIx01MmNxAbZV41K/nIgoZ4Q="
 }
diff --git a/packages/jsii-pacmak/lib/targets/go.ts b/packages/jsii-pacmak/lib/targets/go.ts
@@ -43,7 +43,7 @@ export class Golang extends Target {
     // run `go build` with local.go.mod, go 1.16 requires that we download
     // modules explicit so go.sum is updated.
     await go('mod', ['download', '-modfile', localGoMod], { cwd: pkgDir });
-    await go('build', ['-modfile', localGoMod], { cwd: pkgDir });
+    await go('build', ['-modfile', localGoMod, './...'], { cwd: pkgDir });
 
     // delete local.go.mod and local.go.sum from the output directory so it doesn't get published
     const localGoSum = `${path.basename(localGoMod, '.mod')}.sum`;

diff --git a/packages/jsii-pacmak/lib/targets/go/package.ts b/packages/jsii-pacmak/lib/targets/go/package.ts
@@ -125,17 +125,11 @@ export abstract class Package {
   }
 
   protected get usesRuntimePackage(): boolean {
-    return (
-      this.types.some((type) => type.usesRuntimePackage) ||
-      this.submodules.some((sub) => sub.usesRuntimePackage)
-    );
+    return this.types.some((type) => type.usesRuntimePackage);
   }
 
   protected get usesInitPackage(): boolean {
-    return (
-      this.types.some((type) => type.usesInitPackage) ||
-      this.submodules.some((sub) => sub.usesInitPackage)
-    );
+    return this.types.some((type) => type.usesInitPackage);
   }
 
   /**

diff --git a/packages/jsii-pacmak/lib/targets/go/types/class.ts b/packages/jsii-pacmak/lib/targets/go/types/class.ts
@@ -15,7 +15,7 @@ import { getMemberDependencies, getParamDependencies } from '../util';
 import { GoType } from './go-type';
 import { GoTypeRef } from './go-type-reference';
 import { GoInterface } from './interface';
-import { GoParameter, GoMethod, GoProperty } from './type-member';
+import { GoParameter, GoMethod, GoProperty, GoTypeMember } from './type-member';
 
 /*
  * GoClass wraps a Typescript class as a Go custom struct type
@@ -156,20 +156,23 @@ export class GoClass extends GoType {
     code.close(')');
   }
 
+  public get members(): GoTypeMember[] {
+    return [
+      ...this.methods,
+      ...this.properties,
+      ...this.staticMethods,
+      ...this.staticProperties,
+    ];
+  }
+
   public get usesInitPackage() {
     return (
-      this.initializer != null ||
-      this.methods.some((m) => m.usesInitPackage) ||
-      this.properties.some((p) => p.usesInitPackage)
+      this.initializer != null || this.members.some((m) => m.usesInitPackage)
     );
   }
 
   public get usesRuntimePackage() {
-    return (
-      this.initializer != null ||
-      this.methods.length > 0 ||
-      this.properties.length > 0
-    );
+    return this.initializer != null || this.members.length > 0;
   }
 
   protected emitInterface(context: EmitContext): void {
@@ -289,9 +292,8 @@ export class GoClass extends GoType {
     // need to add dependencies of method arguments and constructor arguments
     return [
       ...this.baseTypes.map((ref) => ref.pkg),
-      ...getMemberDependencies(this.properties),
-      ...getMemberDependencies(this.methods),
-      ...getParamDependencies(this.methods),
+      ...getMemberDependencies(this.members),
+      ...getParamDependencies(this.members.filter(isGoMethod)),
     ];
   }
 
@@ -412,3 +414,7 @@ export class StaticMethod extends ClassMethod {
     code.line();
   }
 }
+
+function isGoMethod(m: GoTypeMember): m is GoMethod {
+  return m instanceof GoMethod;
+}
diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts
@@ -1503,12 +1503,15 @@ class PythonModule implements PythonType {
       code.line();
     }
     code.line();
-    code.indent('__all__ = [');
-    for (const member of exportedMembers.sort()) {
-      // Writing one by line might be _a lot_ of lines, but it'll make reviewing changes to the list easier. Trust me.
-      code.line(`${member},`);
+
+    if (exportedMembers.length > 0) {
+      code.indent('__all__ = [');
+      for (const member of exportedMembers.sort()) {
+        // Writing one by line might be _a lot_ of lines, but it'll make reviewing changes to the list easier. Trust me.
+        code.line(`${member},`);
+      }
+      code.unindent(']');
     }
-    code.unindent(']');
 
     // Finally, we'll use publication to ensure that all of the non-public names
     // get hidden from dir(), tab-complete, etc.