From f8136204af43f2ed39e1a4b5105edb77f483113b Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Thu, 28 Mar 2019 14:22:26 -0400 Subject: [PATCH] fix(python): Lift the entire data class hierarchy (#408) They Python code generation was only lifting the properties of the specified data class, but it wasn't lifting the properties of all of the data classes in the inheritance tree. We have to do that, so we'll go ahead and walk our entire interface hierarchy and build up a flat list of all of the properties, and use that list to generate our lifted properties. --- packages/jsii-pacmak/lib/targets/python.ts | 52 ++++++++++++++++--- .../python/src/jsii_calc/__init__.py | 14 +++-- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index 3ecf87303e..d74c28eaf0 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -363,14 +363,15 @@ abstract class BaseMethod implements PythonBase { if (this.liftedProp !== undefined) { // Remove our last item. pythonParams.pop(); + const liftedProperties = this.getLiftedProperties(resolver); - if (this.liftedProp.properties !== undefined && this.liftedProp.properties.length >= 1) { + if (liftedProperties.length >= 1) { // All of these parameters are keyword only arguments, so we'll mark them // as such. pythonParams.push("*"); // Iterate over all of our props, and reflect them into our params. - for (const prop of this.liftedProp.properties) { + for (const prop of liftedProperties) { const paramName = toPythonParameterName(prop.name); const paramType = resolver.resolve(prop.type, { forwardReferences: false }); const paramDefault = prop.type.optional ? "=None" : ""; @@ -431,7 +432,7 @@ abstract class BaseMethod implements PythonBase { // We need to build up a list of properties, which are mandatory, these are the // ones we will specifiy to start with in our dictionary literal. const mandatoryPropMembers: string[] = []; - for (const prop of this.liftedProp!.properties || []) { + for (const prop of this.getLiftedProperties(resolver)) { if (prop.type.optional) { continue; } @@ -443,7 +444,7 @@ abstract class BaseMethod implements PythonBase { // Now we'll go through our optional properties, and if they haven't been set // we'll add them to our dictionary. - for (const prop of this.liftedProp!.properties || []) { + for (const prop of this.getLiftedProperties(resolver)) { if (!prop.type.optional) { continue; } @@ -476,6 +477,29 @@ abstract class BaseMethod implements PythonBase { code.line(`${methodPrefix}jsii.${this.jsiiMethod}(${jsiiMethodParams.join(", ")}, [${paramNames.join(", ")}])`); } + + private getLiftedProperties(resolver: TypeResolver): spec.Property[] { + const liftedProperties: spec.Property[] = []; + + const stack = [this.liftedProp]; + let current = stack.shift(); + while (current !== undefined) { + // Add any interfaces that this interface depends on, to the list. + if (current.interfaces !== undefined) { + stack.push(...current.interfaces.map(ifc => resolver.dereference(ifc) as spec.InterfaceType)); + } + + // Add all of the properties of this interface to our list of properties. + if (current.properties !== undefined) { + liftedProperties.push(...current.properties); + } + + // Finally, grab our next item. + current = stack.shift(); + } + + return liftedProperties; + } } interface BasePropertyOpts { @@ -1139,6 +1163,7 @@ class Package { } type FindModuleCallback = (fqn: string) => spec.Assembly | spec.PackageVersion; +type FindTypeCallback = (fqn: string) => spec.Type; interface TypeResolverOpts { forwardReferences?: boolean; @@ -1154,10 +1179,16 @@ class TypeResolver { private readonly moduleName?: string; private readonly moduleRe: RegExp; private readonly findModule: FindModuleCallback; + private readonly findType: FindTypeCallback; - constructor(types: Map, findModule: FindModuleCallback, boundTo?: string, moduleName?: string) { + constructor(types: Map, + findModule: FindModuleCallback, + findType: FindTypeCallback, + boundTo?: string, + moduleName?: string) { this.types = types; this.findModule = findModule; + this.findType = findType; this.moduleName = moduleName; this.boundTo = boundTo !== undefined ? this.toPythonFQN(boundTo) : boundTo; @@ -1174,6 +1205,7 @@ class TypeResolver { return new TypeResolver( this.types, this.findModule, + this.findType, fqn, moduleName !== undefined ? moduleName : this.moduleName, ); @@ -1211,6 +1243,10 @@ class TypeResolver { return type; } + public dereference(typeRef: spec.NamedTypeReference): spec.Type { + return this.findType(typeRef.fqn); + } + public resolve( typeRef: spec.TypeReference, opts: TypeResolverOpts = { forwardReferences: true, ignoreOptional: false }): string { @@ -1381,7 +1417,11 @@ class PythonGenerator extends Generator { } protected onEndAssembly(_assm: spec.Assembly, _fingerprint: boolean) { - const resolver = new TypeResolver(this.types, (fqn: string) => this.findModule(fqn)); + const resolver = new TypeResolver( + this.types, + (fqn: string) => this.findModule(fqn), + (fqn: string) => this.findType(fqn), + ); this.package.write(this.code, resolver); } diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py b/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py index 39837433c1..1a5860bded 100644 --- a/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py +++ b/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py @@ -559,8 +559,8 @@ def __init__(self) -> None: jsii.create(GiveMeStructs, self, []) @jsii.member(jsii_name="derivedToFirst") - def derived_to_first(self, *, another_required: datetime.datetime, bool: bool, non_primitive: "DoubleTrouble", another_optional: typing.Optional[typing.Mapping[str,scope.jsii_calc_lib.Value]]=None, optional_any: typing.Any=None, optional_array: typing.Optional[typing.List[str]]=None) -> scope.jsii_calc_lib.MyFirstStruct: - derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive} + def derived_to_first(self, *, another_required: datetime.datetime, bool: bool, non_primitive: "DoubleTrouble", another_optional: typing.Optional[typing.Mapping[str,scope.jsii_calc_lib.Value]]=None, optional_any: typing.Any=None, optional_array: typing.Optional[typing.List[str]]=None, anumber: jsii.Number, astring: str, first_optional: typing.Optional[typing.List[str]]=None) -> scope.jsii_calc_lib.MyFirstStruct: + derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive, "anumber": anumber, "astring": astring} if another_optional is not None: derived["anotherOptional"] = another_optional @@ -571,11 +571,14 @@ def derived_to_first(self, *, another_required: datetime.datetime, bool: bool, n if optional_array is not None: derived["optionalArray"] = optional_array + if first_optional is not None: + derived["firstOptional"] = first_optional + return jsii.invoke(self, "derivedToFirst", [derived]) @jsii.member(jsii_name="readDerivedNonPrimitive") - def read_derived_non_primitive(self, *, another_required: datetime.datetime, bool: bool, non_primitive: "DoubleTrouble", another_optional: typing.Optional[typing.Mapping[str,scope.jsii_calc_lib.Value]]=None, optional_any: typing.Any=None, optional_array: typing.Optional[typing.List[str]]=None) -> "DoubleTrouble": - derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive} + def read_derived_non_primitive(self, *, another_required: datetime.datetime, bool: bool, non_primitive: "DoubleTrouble", another_optional: typing.Optional[typing.Mapping[str,scope.jsii_calc_lib.Value]]=None, optional_any: typing.Any=None, optional_array: typing.Optional[typing.List[str]]=None, anumber: jsii.Number, astring: str, first_optional: typing.Optional[typing.List[str]]=None) -> "DoubleTrouble": + derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive, "anumber": anumber, "astring": astring} if another_optional is not None: derived["anotherOptional"] = another_optional @@ -586,6 +589,9 @@ def read_derived_non_primitive(self, *, another_required: datetime.datetime, boo if optional_array is not None: derived["optionalArray"] = optional_array + if first_optional is not None: + derived["firstOptional"] = first_optional + return jsii.invoke(self, "readDerivedNonPrimitive", [derived]) @jsii.member(jsii_name="readFirstNumber")