fix(go): Method call return reference an implMap

Fixes code generation for go method calls to correctly pass an implMap where needed and a reference to the return value instead of a concrete type by value. Adds test for method call with named return type in TestReturnSpecialParam to cover this. Fixes logic for `usesReflectionPackage` which only needs to know if the type has methods or parameters since all getters and methods need to reference `reflect.Type` regardless of the contents of the implMap. Refactor runtime calls to not reimplement the emission logic for implMaps.
aws · Nov 20, 2020 · af5b27b · af5b27b
1 parent 1373cb3
commit af5b27b
Show file tree

Hide file tree

Showing 13 changed files with 205 additions and 153 deletions.
diff --git a/packages/@jsii/go-runtime/go.mod b/packages/@jsii/go-runtime/go.mod
@@ -4,6 +4,7 @@ go 1.15
 
 require (
 	github.com/aws-cdk/jsii/jsii-calc/go/jsiicalc v0.0.0
+	github.com/aws-cdk/jsii/jsii-calc/go/scopejsiicalclib v0.0.0
 	github.com/aws-cdk/jsii/jsii-experimental v0.0.0
 )
 

diff --git a/packages/@jsii/go-runtime/jsii-calc-test/main_test.go b/packages/@jsii/go-runtime/jsii-calc-test/main_test.go
@@ -3,6 +3,8 @@ package main
 import (
 	"fmt"
 	calc "github.com/aws-cdk/jsii/jsii-calc/go/jsiicalc"
+        returnsParam "github.com/aws-cdk/jsii/jsii-calc/go/jsiicalc/submodule/returnsparam"
+        param "github.com/aws-cdk/jsii/jsii-calc/go/jsiicalc/submodule/param"
 	calclib "github.com/aws-cdk/jsii/jsii-calc/go/scopejsiicalclib"
 	"github.com/aws-cdk/jsii/jsii-experimental"
 	"math"
@@ -153,3 +155,13 @@ func TestAllTypes(t *testing.T) {
 		}
 	})
 }
+
+func TestReturnsSpecialParam(t *testing.T) {
+  retSpecialParam := returnsParam.NewReturnsSpecialParameter()
+  val := retSpecialParam.ReturnsSpecialParam()
+  expected := reflect.TypeOf(&param.SpecialParameter{})
+  actual := reflect.TypeOf(val)
+  if actual != expected {
+    t.Errorf("Expected type: %s; Actual: %s", expected, actual)
+  }
+}
diff --git a/packages/@jsii/go-runtime/jsii-experimental/runtime.go b/packages/@jsii/go-runtime/jsii-experimental/runtime.go
@@ -8,6 +8,9 @@ import (
 	"regexp"
 )
 
+// Maps interface types to their concrete implementation structs. Used by
+// `castAndSetToPtr` to instantiate a concrete type that implements the
+// the interface as dictated by the type of the &returnsPtr value.
 type implementationMap = map[reflect.Type]reflect.Type
 
 // Load ensures a npm package is loaded in the jsii kernel.

diff --git a/packages/jsii-pacmak/lib/targets/go/package.ts b/packages/jsii-pacmak/lib/targets/go/package.ts
@@ -94,14 +94,6 @@ export abstract class Package {
     return findTypeInTree(this, fqn);
   }
 
-  public get importPath(): string {
-    const moduleName = this.root.moduleName;
-    const prefix = moduleName !== '' ? `${moduleName}/` : '';
-    const rootPackageName = this.root.packageName;
-    const suffix = this.filePath !== '' ? `/${this.filePath}` : '';
-    return `${prefix}${rootPackageName}${suffix}`;
-  }
-
   public emit(context: EmitContext): void {
     const { code } = context;
 

diff --git a/packages/jsii-pacmak/lib/targets/go/runtime/function-call.ts b/packages/jsii-pacmak/lib/targets/go/runtime/function-call.ts
@@ -0,0 +1,39 @@
+import {CodeMaker} from 'codemaker';
+import {JSII_IMPL_MAP_TYPE} from './constants';
+import {GoTypeMember} from "../types";
+
+export abstract class FunctionCall {
+  public constructor(public readonly parent: GoTypeMember) {}
+
+  protected get implMap(): string[] {
+    return this.parent.reference?.scopedImplMap(this.parent.parent.pkg) ?? [];
+  }
+
+  /**
+   * Emits map of interface type to concrete struct type for use in runtime to
+   * cast data to expected return type.
+   */
+  protected emitImplMapVal(code: CodeMaker) {
+    if (this.implMap.length) {
+      const [interfaceName, structName] = this.implMap;
+      code.open(`${JSII_IMPL_MAP_TYPE}{`);
+
+      // `reflect.TypeOf((*SomeType)(nil)).Elem()` is a reliable way to create
+      // an instance of reflect.Type for any type. `(*SomeInterface)(nil)`
+      // creates a "zero value" with the type `SomeInterface` which otherwise
+      // has no way to instantiate.
+      code.line(`reflect.TypeOf((*${interfaceName})(nil)).Elem(): reflect.TypeOf((*${structName})(nil)).Elem(),`);
+      code.close('},');
+    } else {
+      code.line(`${JSII_IMPL_MAP_TYPE}{},`);
+    }
+  }
+
+  protected get returnsVal(): boolean {
+    return Boolean(this.parent.reference && !this.parent.reference.void);
+  }
+
+  protected get returnType(): string {
+    return this.parent.returnType || 'interface{}';
+  }
+}
diff --git a/packages/jsii-pacmak/lib/targets/go/runtime/method-call.ts b/packages/jsii-pacmak/lib/targets/go/runtime/method-call.ts
@@ -4,12 +4,15 @@ import { GoMethod } from '../types';
 import {
   JSII_INVOKE_FUNC,
   JSII_SINVOKE_FUNC,
-  JSII_IMPL_MAP_TYPE,
 } from './constants';
 import { slugify, emitInitialization } from './util';
+import { FunctionCall } from './function-call';
 
-export class MethodCall {
-  public constructor(public readonly parent: GoMethod) {}
+export class MethodCall extends FunctionCall {
+  private _returnVarName = '';
+  public constructor(public readonly parent: GoMethod) {
+    super(parent)
+  }
 
   public emit(code: CodeMaker) {
     if (this.inStatic) {
@@ -20,20 +23,15 @@ export class MethodCall {
   }
 
   private emitDynamic(code: CodeMaker) {
-    code.line(`var ${this.returnVarName} ${this.concreteReturnType}`);
-    code.line(`${this.implMapVar} := make(${JSII_IMPL_MAP_TYPE})`);
+    code.line(`var ${this.returnVarName} ${this.returnType}`);
     code.open(`${JSII_INVOKE_FUNC}(`);
 
-    const returnsArg = this.parent.returnsRef
-      ? this.returnVarName
-      : `&${this.returnVarName}`;
-
     code.line(`${this.parent.instanceArg},`);
     code.line(`"${this.parent.method.name}",`);
     code.line(`${this.argsString},`);
     code.line(`${this.returnsVal ? 'true' : 'false'},`);
-    code.line(`${returnsArg},`);
-    code.line(`${this.implMapVar},`);
+    code.line(`&${this.returnVarName},`);
+    this.emitImplMapVal(code);
 
     code.close(`)`);
 
@@ -44,8 +42,7 @@ export class MethodCall {
 
   private emitStatic(code: CodeMaker) {
     emitInitialization(code);
-    code.line(`var ${this.returnVarName} ${this.concreteReturnType}`);
-    code.line(`${this.implMapVar} := make(${JSII_IMPL_MAP_TYPE})`);
+    code.line(`var ${this.returnVarName} ${this.returnType}`);
 
     code.open(`${JSII_SINVOKE_FUNC}(`);
 
@@ -54,7 +51,7 @@ export class MethodCall {
     code.line(`${this.argsString},`);
     code.line(`${this.returnsVal ? 'true' : 'false'},`);
     code.line(`&${this.returnVarName},`);
-    code.line(`${this.implMapVar},`);
+    this.emitImplMapVal(code);
 
     code.close(`)`);
 
@@ -64,29 +61,13 @@ export class MethodCall {
   }
 
   private get returnVarName(): string {
-    return slugify(
-      'returns',
-      this.parent.parameters.map((p) => p.name),
-    );
-  }
-
-  private get implMapVar(): string {
-    return slugify(
-      'implMap',
-      this.parent.parameters.map((p) => p.name),
-    );
-  }
-
-  private get returnsVal(): boolean {
-    return Boolean(this.parent.reference && !this.parent.reference.void);
-  }
-
-  private get concreteReturnType(): string | undefined {
-    if (this.returnsVal) {
-      return this.parent.concreteReturnType;
+    if (this._returnVarName === '') {
+      this._returnVarName = slugify(
+        'returns',
+        this.parent.parameters.map((p) => p.name),
+      );
     }
-
-    return 'interface{}';
+    return this._returnVarName;
   }
 
   private get inStatic(): boolean {

diff --git a/packages/jsii-pacmak/lib/targets/go/runtime/property-access.ts b/packages/jsii-pacmak/lib/targets/go/runtime/property-access.ts
@@ -6,33 +6,24 @@ import {
   JSII_SET_FUNC,
   JSII_SGET_FUNC,
   JSII_SSET_FUNC,
-  JSII_IMPL_MAP_TYPE,
 } from './constants';
 import { slugify, emitInitialization } from './util';
+import {FunctionCall} from './function-call';
 
-export class GetProperty {
-  public constructor(public readonly parent: GoProperty) {}
+export class GetProperty extends FunctionCall {
+  public constructor(public readonly parent: GoProperty) {
+    super(parent);
+  }
 
   public emit(code: CodeMaker) {
     const resultVar = slugify('returns', [this.parent.instanceArg]);
-    const implMapVar = slugify('implMap', [this.parent.instanceArg]);
-    code.line(`var ${resultVar} ${this.parent.returnType}`);
-    code.line(`${implMapVar} := make(${JSII_IMPL_MAP_TYPE})`);
-
-    const implMap =
-      this.parent.reference?.scopedImplMap(this.parent.parent.pkg) ?? [];
-    if (implMap.length) {
-      const [interfaceName, structName] = implMap;
-      code.line(
-        `${implMapVar}[reflect.TypeOf((*${interfaceName})(nil)).Elem()] = reflect.TypeOf((*${structName})(nil)).Elem()`,
-      );
-    }
+    code.line(`var ${resultVar} ${this.returnType}`);
 
     code.open(`${JSII_GET_FUNC}(`);
     code.line(`${this.parent.instanceArg},`);
     code.line(`"${this.parent.property.name}",`);
     code.line(`&${resultVar},`);
-    code.line(`${implMapVar},`);
+    this.emitImplMapVal(code);
     code.close(`)`);
 
     code.line(`return ${resultVar}`);
@@ -51,30 +42,21 @@ export class SetProperty {
   }
 }
 
-export class StaticGetProperty {
-  public constructor(public readonly parent: GoProperty) {}
+export class StaticGetProperty extends FunctionCall {
+  public constructor(public readonly parent: GoProperty) {
+    super(parent);
+  }
 
   public emit(code: CodeMaker) {
     emitInitialization(code);
     const resultVar = slugify('returns', []);
-    const implMapVar = slugify('implMap', [this.parent.instanceArg]);
-    code.line(`var ${resultVar} ${this.parent.returnType}`);
-    code.line(`${implMapVar} := make(${JSII_IMPL_MAP_TYPE})`);
-
-    const implMap =
-      this.parent.reference?.scopedImplMap(this.parent.parent.pkg) ?? [];
-    if (implMap.length) {
-      const [interfaceName, structName] = implMap;
-      code.line(
-        `${implMapVar}[reflect.TypeOf((*${interfaceName})(nil)).Elem()] = reflect.TypeOf((*${structName})(nil)).Elem()`,
-      );
-    }
+    code.line(`var ${resultVar} ${this.returnType}`);
 
     code.open(`${JSII_SGET_FUNC}(`);
     code.line(`"${this.parent.parent.fqn}",`);
     code.line(`"${this.parent.property.name}",`);
     code.line(`&${resultVar},`);
-    code.line(`${implMapVar},`);
+    this.emitImplMapVal(code);
     code.close(`)`);
 
     code.line(`return ${resultVar}`);

diff --git a/packages/jsii-pacmak/lib/targets/go/types/class.ts b/packages/jsii-pacmak/lib/targets/go/types/class.ts
@@ -84,11 +84,8 @@ export class GoClass extends GoStruct {
     );
   }
 
-  public get usesReflectionPackage() {
-    return (
-      this.properties.some((p) => p.usesReflectionPackage) ||
-      this.methods.some((m) => m.usesReflectionPackage)
-    );
+  public get usesReflectionPackage(): boolean {
+    return this.properties.length > 0 || this.methods.length > 0;
   }
 
   protected emitInterface(context: EmitContext): void {
@@ -248,10 +245,6 @@ export class ClassMethod extends GoMethod {
   public get instanceArg(): string {
     return this.parent.name.substring(0, 1).toLowerCase();
   }
-
-  public get usesReflectionPackage() {
-    return Boolean(this.reference?.scopedImplMap(this.parent.pkg).length);
-  }
 }
 
 export class StaticMethod extends ClassMethod {

diff --git a/packages/jsii-pacmak/lib/targets/go/types/go-type.ts b/packages/jsii-pacmak/lib/targets/go/types/go-type.ts
@@ -70,6 +70,10 @@ export abstract class GoStruct extends GoType {
     return this.properties.some((p) => p.usesRuntimePackage);
   }
 
+  public get usesReflectionPackage(): boolean {
+    return this.properties.length > 0;
+  }
+
   protected emitInterface(context: EmitContext): void {
     const { code } = context;
     code.line(

diff --git a/packages/jsii-pacmak/lib/targets/go/types/interface.ts b/packages/jsii-pacmak/lib/targets/go/types/interface.ts
@@ -88,11 +88,8 @@ export class Interface extends GoType {
     );
   }
 
-  public get usesReflectionPackage() {
-    return (
-      this.properties.some((p) => p.usesReflectionPackage) ||
-      this.methods.some((m) => m.usesReflectionPackage)
-    );
+  public get usesReflectionPackage(): boolean {
+    return this.properties.length > 0 || this.methods.length > 0;
   }
 
   public get extends(): GoTypeRef[] {
@@ -190,8 +187,4 @@ class InterfaceMethod extends GoMethod {
   private get returnTypeString(): string {
     return this.reference?.void ? '' : ` ${this.returnType}`;
   }
-
-  public get usesReflectionPackage() {
-    return Boolean(this.reference?.scopedImplMap(this.parent.pkg).length);
-  }
 }
diff --git a/packages/jsii-pacmak/lib/targets/go/types/struct.ts b/packages/jsii-pacmak/lib/targets/go/types/struct.ts
@@ -17,6 +17,6 @@ export class Struct extends GoStruct {
   }
 
   public get usesReflectionPackage() {
-    return this.properties.some((p) => p.usesReflectionPackage);
+    return this.properties.length > 0;
   }
 }
diff --git a/packages/jsii-pacmak/lib/targets/go/types/type-member.ts b/packages/jsii-pacmak/lib/targets/go/types/type-member.ts
@@ -18,7 +18,6 @@ export interface GoTypeMember {
 
   usesInitPackage: boolean;
   usesRuntimePackage: boolean;
-  usesReflectionPackage: boolean;
 }
 
 /*
@@ -52,10 +51,6 @@ export class GoProperty implements GoTypeMember {
     return true;
   }
 
-  public get usesReflectionPackage() {
-    return Boolean(this.reference?.scopedImplMap(this.parent.pkg).length);
-  }
-
   public get static(): boolean {
     return !!this.property.static;
   }
@@ -157,7 +152,6 @@ export abstract class GoMethod implements GoTypeMember {
   public abstract emit(context: EmitContext): void;
   public abstract get usesInitPackage(): boolean;
   public abstract get usesRuntimePackage(): boolean;
-  public abstract get usesReflectionPackage(): boolean;
 
   public get returnsRef(): boolean {
     if (