Skip to content

Commit

Permalink
feat(go): represent jsii structs as go structs (only) (#2600)
Browse files Browse the repository at this point in the history
As proposed in aws/aws-cdk-rfcs#292 (this is *Approach 4*), stop
rendering go interfaces to represent jsii structs, and instead only emit
a plain go struct with the flattened list of fields (own + all super
interfaces).

Made the necessary code changes to de-serialize structs returned
by-reference by the `@jsii/kernel` by eagerly fecthing all properties.

Also, implemented the option to offer convenience conversion functions
to easily create a parent type from a child type.
  • Loading branch information
RomainMuller committed Feb 25, 2021
1 parent 6e74bf9 commit e7cc93e
Show file tree
Hide file tree
Showing 9 changed files with 335 additions and 1,630 deletions.
24 changes: 11 additions & 13 deletions packages/@jsii/go-runtime/jsii-calc-test/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (

"github.com/aws/jsii-runtime-go"
calc "github.com/aws/jsii/jsii-calc/go/jsiicalc/v3"
param "github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/submodule/param"
"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/submodule/param"
returnsParam "github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/submodule/returnsparam"
calclib "github.com/aws/jsii/jsii-calc/go/scopejsiicalclib"
"github.com/aws/jsii/jsii-calc/go/scopejsiicalclib/submodule"
)

func TestMain(m *testing.M) {
Expand All @@ -24,8 +25,10 @@ func TestMain(m *testing.M) {
// Only uses first argument as initial value. This is just a convenience for
// tests that want to assert against the initialValue
func initCalculator(initialValue float64) calc.CalculatorIface {
calculatorProps := calc.CalculatorProps{InitialValue: initialValue, MaximumValue: math.MaxFloat64}
return calc.NewCalculator(&calculatorProps)
return calc.NewCalculator(calc.CalculatorProps{
InitialValue: initialValue,
MaximumValue: math.MaxFloat64,
})
}

func TestCalculator(t *testing.T) {
Expand Down Expand Up @@ -111,15 +114,10 @@ func TestUpcasingReflectable(t *testing.T) {
t.Errorf("Entries expected to have length of: 1; Actual: %d", len(entries))
}

entry := entries[0]
upperKey := strings.ToUpper(key)
actualKey, actualVal := entry.GetKey(), entry.GetValue()
if actualKey != upperKey {
t.Errorf("Expected Key: %s; Received Key: %s", upperKey, actualKey)
}

if actualVal != val {
t.Errorf("Expected Value: %s; Received Value: %s", val, actualVal)
actual := entries[0]
expected := submodule.ReflectableEntry{Key: strings.ToUpper(key), Value: val}
if actual != expected {
t.Errorf("Expected %v; Received: %v", expected, actual)
}
}

Expand Down Expand Up @@ -200,7 +198,7 @@ func TestOptionalEnums(t *testing.T) {
func TestReturnsSpecialParam(t *testing.T) {
retSpecialParam := returnsParam.NewReturnsSpecialParameter()
val := retSpecialParam.ReturnsSpecialParam()
expected := reflect.TypeOf(&param.SpecialParameter{})
expected := reflect.TypeOf(param.SpecialParameter{})
actual := reflect.TypeOf(val)
if actual != expected {
t.Errorf("Expected type: %s; Actual: %s", expected, actual)
Expand Down
19 changes: 18 additions & 1 deletion packages/@jsii/go-runtime/jsii-runtime-go/kernel/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,24 @@ func (c *client) CastAndSetToPtr(ptr interface{}, data interface{}) {
dataVal := reflect.ValueOf(data)

if ref, isRef := castValToRef(data); isRef {
// If return data is JSII object references, add to objects table.
// If return data is a jsii struct passed by reference, de-reference it all.
if fields, isStruct := c.Types().StructFields(ptrVal.Type()); isStruct {
for _, field := range fields {
got, err := c.Get(GetRequest{
API: "get",
Property: field.Tag.Get("json"),
ObjRef: ref,
})
if err != nil {
panic(err)
}
fieldVal := ptrVal.FieldByIndex(field.Index)
c.CastAndSetToPtr(fieldVal.Addr().Interface(), got.Value)
}
return
}

// If return data is jsii object references, add to objects table.
if concreteType, err := c.Types().ConcreteTypeFor(ptrVal.Type()); err == nil {
ptrVal.Set(reflect.New(concreteType))
if err = c.RegisterInstance(ptrVal.Interface(), ref.InstanceID); err != nil {
Expand Down
9 changes: 4 additions & 5 deletions packages/@jsii/go-runtime/jsii-runtime-go/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,11 @@ func RegisterInterface(fqn FQN, iface reflect.Type, proxy reflect.Type) {
}

// RegisterStruct associates a struct's fully qualified name to the specified
// struct type, and struct interface. Panics if strct is not a struct, iface is
// not an interface, or if the provided fqn was already used to register a
// different type.
func RegisterStruct(fqn FQN, strct reflect.Type, iface reflect.Type) {
// struct type. Panics if strct is not a struct, or if the provided fqn was
// already used to register a different type.
func RegisterStruct(fqn FQN, strct reflect.Type) {
client := kernel.GetClient()
if err := client.Types().RegisterStruct(api.FQN(fqn), strct, iface); err != nil {
if err := client.Types().RegisterStruct(api.FQN(fqn), strct); err != nil {
panic(err)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type TypeRegisterer interface {
// RegisterStruct maps the given FQN to the provided struct type, and struct
// interface. Returns an error if the provided struct type is not a go struct,
// or the provided iface not a go interface.
RegisterStruct(fqn api.FQN, strct reflect.Type, iface reflect.Type) error
RegisterStruct(fqn api.FQN, strct reflect.Type) error
}

// RegisterClass maps the given FQN to the provided class type, and interface
Expand Down Expand Up @@ -112,23 +112,33 @@ func (t *typeRegistry) RegisterInterface(fqn api.FQN, iface reflect.Type, proxy
// RegisterStruct maps the given FQN to the provided struct type, and struct
// interface. Returns an error if the provided struct type is not a go struct,
// or the provided iface not a go interface.
func (t *typeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type, iface reflect.Type) error {
func (t *typeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type) error {
if strct.Kind() != reflect.Struct {
return fmt.Errorf("the provided struct is not a struct: %v", strct)
}
if iface.Kind() != reflect.Interface {
return fmt.Errorf("the provided interface is not an interface: %v", iface)
}

if existing, exists := t.fqnToType[fqn]; exists && existing != strct {
return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
}
if existing, exists := t.ifaceToStruct[iface]; exists && existing != strct {
return fmt.Errorf("another struct was already registered with %v: %v", iface, existing)

fields := []reflect.StructField{}
numField := strct.NumField()
for i := 0 ; i < numField ; i++ {
field := strct.Field(i)
if field.Anonymous {
return fmt.Errorf("unexpected anonymous field %v in struct %s (%v)", field, fqn, strct)
}
if field.PkgPath != "" {
return fmt.Errorf("unexpected un-exported field %v in struct %s (%v)", field, fqn, strct)
}
if field.Tag.Get("json") == "" {
return fmt.Errorf("missing json tag on struct field %v of %s (%v)", field, fqn, strct)
}
fields = append(fields, field)
}

t.fqnToType[fqn] = strct
t.ifaceToStruct[iface] = strct
t.structFields[strct] = fields

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import (
type TypeRegistry interface {
TypeRegisterer

// StructFields returns the list of fields that make a registered jsii struct.
StructFields(typ reflect.Type) (fields []reflect.StructField, found bool)

// ConcreteTypeFor returns the concrete implementation of the provided struct
// or interface type. If typ is a struct, returns typ without futher effort. If
// it is an interface, returns the struct associated to this interface type.
Expand Down Expand Up @@ -54,6 +57,9 @@ type typeRegistry struct {
// maps Go enum type ("StringEnum") to the corresponding jsii enum FQN (e.g.
// "jsii-calc.StringEnum")
typeToEnumFQN map[reflect.Type]api.FQN

// maps registered struct types to all their fields.
structFields map[reflect.Type][]reflect.StructField
}

// NewTypeRegistry creates a new type registry.
Expand All @@ -63,10 +69,21 @@ func NewTypeRegistry() TypeRegistry {
ifaceToStruct: make(map[reflect.Type]reflect.Type),
fqnToEnumMember: make(map[string]interface{}),
typeToEnumFQN: make(map[reflect.Type]api.FQN),
structFields: make(map[reflect.Type][]reflect.StructField),
}
}

// IsStruct returns true if the provided type is a registered jsii struct.
func (t *typeRegistry) StructFields(typ reflect.Type) (fields []reflect.StructField, ok bool) {
var found []reflect.StructField
found, ok = t.structFields[typ]
if ok {
fields = append(fields, found...)
}
return
}

// concreteTypeFor returns the concrete implementation of the provided struct
// ConcreteTypeFor returns the concrete implementation of the provided struct
// or interface type. If typ is a struct, returns typ without futher effort. If
// it is an interface, returns the struct associated to this interface type.
// Returns an error if the argument is an interface, and no struct was
Expand All @@ -86,7 +103,7 @@ func (t *typeRegistry) ConcreteTypeFor(typ reflect.Type) (structType reflect.Typ
return
}

// enumMemberForEnumRef returns the go enum member corresponding to a jsii fully
// EnumMemberForEnumRef returns the go enum member corresponding to a jsii fully
// qualified enum member name (e.g: "jsii-calc.StringEnum/A"). If no enum member
// was registered (via registerEnum) for the provided enumref, an error is
// returned.
Expand All @@ -97,7 +114,7 @@ func (t *typeRegistry) EnumMemberForEnumRef(ref api.EnumRef) (interface{}, error
return nil, fmt.Errorf("no enum member registered for %s", ref.MemberFQN)
}

// tryRenderEnumRef returns an enumref if the provided value corresponds to a
// TryRenderEnumRef returns an enumref if the provided value corresponds to a
// registered enum type. The returned enumref is nil if the provided enum value
// is a zero-value (i.e: "").
func (t *typeRegistry) TryRenderEnumRef(value reflect.Value) (ref *api.EnumRef, isEnumRef bool) {
Expand Down
79 changes: 72 additions & 7 deletions packages/jsii-pacmak/lib/targets/go/types/struct.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,93 @@
import * as assert from 'assert';
import { CodeMaker } from 'codemaker';
import { InterfaceType } from 'jsii-reflect';

import { EmitContext } from '../emit-context';
import { Package } from '../package';
import { JSII_RT_ALIAS } from '../runtime';
import { GoStruct } from './go-type';
import { getMemberDependencies } from '../util';
import { GoType } from './go-type';
import { GoTypeRef } from './go-type-reference';
import { GoProperty } from './type-member';

/*
* Struct wraps a JSII datatype interface aka, structs
*/
export class Struct extends GoStruct {
public constructor(parent: Package, type: InterfaceType) {
export class Struct extends GoType {
private readonly properties: readonly GoProperty[];

public constructor(parent: Package, public readonly type: InterfaceType) {
super(parent, type);
// TODO check if datatype? (isDataType() on jsii-reflect seems wrong)

assert(
type.isDataType(),
`The provided interface ${type.fqn} is not a struct!`,
);

this.properties = type.allProperties.map(
(prop) => new GoProperty(this, prop),
);
}

public get dependencies(): Package[] {
return getMemberDependencies(this.properties);
}

public get usesRuntimePackage(): boolean {
return false;
}

public get usesInitPackage(): boolean {
return false;
}

public emit(context: EmitContext): void {
const { code, documenter } = context;
documenter.emit(this.type.docs);
code.openBlock(`type ${this.name} struct`);
for (const property of this.properties) {
property.emitStructMember(context);
}
code.closeBlock();
code.line();

this.emitBaseConversions(context);
}

public emitRegistration(code: CodeMaker): void {
code.open(`${JSII_RT_ALIAS}.RegisterStruct(`);
code.line(`"${this.fqn}",`);
code.line(`reflect.TypeOf((*${this.name})(nil)).Elem(),`);
code.line(`reflect.TypeOf((*${this.interfaceName})(nil)).Elem(),`);
code.close(')');
}

public get usesRuntimePackage(): boolean {
return this.properties.some((p) => p.usesRuntimePackage);
private emitBaseConversions({ code }: EmitContext) {
for (const base of this.type.getInterfaces(true)) {
const baseType = this.pkg.root.findType(base.fqn) as Struct;
const funcName = `To${baseType.name}`;
const instanceVar = this.name[0].toLowerCase();
const valType = new GoTypeRef(this.pkg.root, base.reference).scopedName(
this.pkg,
);

code.line(
`// ${funcName} is a convenience function to obtain a new ${valType} from this ${this.name}.`,
);
// Note - using a pointer receiver here as a convenience, as otherwise
// user code that somehow has only a pointer would need to first
// dereference it, which tends to be a code smell.
code.openBlock(
`func (${instanceVar} *${this.name}) ${funcName}() ${valType}`,
);

code.openBlock(`return ${valType}`);
for (const prop of baseType.properties) {
code.line(`${prop.name}: ${instanceVar}.${prop.name},`);
}
code.closeBlock();

code.closeBlock();
code.line();
}
}
}
12 changes: 3 additions & 9 deletions packages/jsii-pacmak/lib/targets/go/types/type-member.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,8 @@ export class GoProperty implements GoTypeMember {
return this.parent.name.substring(0, 1).toLowerCase();
}

public emitStructMember(context: EmitContext) {
const docs = this.property.docs;
if (docs) {
context.documenter.emit(docs);
}
const { code } = context;
public emitStructMember({ code, documenter }: EmitContext) {
documenter.emit(this.property.docs);
const memberType =
this.reference?.type?.name === this.parent.name
? `*${this.returnType}`
Expand Down Expand Up @@ -101,9 +97,7 @@ export class GoProperty implements GoTypeMember {
const instanceArg = receiver.substring(0, 1).toLowerCase();

code.openBlock(
`func (${instanceArg} *${receiver}) ${
this.getter
}()${` ${this.returnType}`}`,
`func (${instanceArg} *${receiver}) ${this.getter}() ${this.returnType}`,
);

new GetProperty(this).emit(code);
Expand Down
4 changes: 3 additions & 1 deletion packages/jsii-pacmak/lib/targets/go/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ export function flatMap<T, R>(
/*
* Return module dependencies of a class or interface members
*/
export function getMemberDependencies(members: GoTypeMember[]): Package[] {
export function getMemberDependencies(
members: readonly GoTypeMember[],
): Package[] {
return members.reduce((accum: Package[], member) => {
return member.reference?.type?.pkg
? [...accum, member.reference?.type.pkg]
Expand Down
Loading

0 comments on commit e7cc93e

Please sign in to comment.