Skip to content

Commit f813620

Browse files
dstufftRomainMuller
authored andcommitted
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.
1 parent 66789e8 commit f813620

File tree

2 files changed

+56
-10
lines changed

2 files changed

+56
-10
lines changed

packages/jsii-pacmak/lib/targets/python.ts

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,14 +363,15 @@ abstract class BaseMethod implements PythonBase {
363363
if (this.liftedProp !== undefined) {
364364
// Remove our last item.
365365
pythonParams.pop();
366+
const liftedProperties = this.getLiftedProperties(resolver);
366367

367-
if (this.liftedProp.properties !== undefined && this.liftedProp.properties.length >= 1) {
368+
if (liftedProperties.length >= 1) {
368369
// All of these parameters are keyword only arguments, so we'll mark them
369370
// as such.
370371
pythonParams.push("*");
371372

372373
// Iterate over all of our props, and reflect them into our params.
373-
for (const prop of this.liftedProp.properties) {
374+
for (const prop of liftedProperties) {
374375
const paramName = toPythonParameterName(prop.name);
375376
const paramType = resolver.resolve(prop.type, { forwardReferences: false });
376377
const paramDefault = prop.type.optional ? "=None" : "";
@@ -431,7 +432,7 @@ abstract class BaseMethod implements PythonBase {
431432
// We need to build up a list of properties, which are mandatory, these are the
432433
// ones we will specifiy to start with in our dictionary literal.
433434
const mandatoryPropMembers: string[] = [];
434-
for (const prop of this.liftedProp!.properties || []) {
435+
for (const prop of this.getLiftedProperties(resolver)) {
435436
if (prop.type.optional) {
436437
continue;
437438
}
@@ -443,7 +444,7 @@ abstract class BaseMethod implements PythonBase {
443444

444445
// Now we'll go through our optional properties, and if they haven't been set
445446
// we'll add them to our dictionary.
446-
for (const prop of this.liftedProp!.properties || []) {
447+
for (const prop of this.getLiftedProperties(resolver)) {
447448
if (!prop.type.optional) {
448449
continue;
449450
}
@@ -476,6 +477,29 @@ abstract class BaseMethod implements PythonBase {
476477

477478
code.line(`${methodPrefix}jsii.${this.jsiiMethod}(${jsiiMethodParams.join(", ")}, [${paramNames.join(", ")}])`);
478479
}
480+
481+
private getLiftedProperties(resolver: TypeResolver): spec.Property[] {
482+
const liftedProperties: spec.Property[] = [];
483+
484+
const stack = [this.liftedProp];
485+
let current = stack.shift();
486+
while (current !== undefined) {
487+
// Add any interfaces that this interface depends on, to the list.
488+
if (current.interfaces !== undefined) {
489+
stack.push(...current.interfaces.map(ifc => resolver.dereference(ifc) as spec.InterfaceType));
490+
}
491+
492+
// Add all of the properties of this interface to our list of properties.
493+
if (current.properties !== undefined) {
494+
liftedProperties.push(...current.properties);
495+
}
496+
497+
// Finally, grab our next item.
498+
current = stack.shift();
499+
}
500+
501+
return liftedProperties;
502+
}
479503
}
480504

481505
interface BasePropertyOpts {
@@ -1139,6 +1163,7 @@ class Package {
11391163
}
11401164

11411165
type FindModuleCallback = (fqn: string) => spec.Assembly | spec.PackageVersion;
1166+
type FindTypeCallback = (fqn: string) => spec.Type;
11421167

11431168
interface TypeResolverOpts {
11441169
forwardReferences?: boolean;
@@ -1154,10 +1179,16 @@ class TypeResolver {
11541179
private readonly moduleName?: string;
11551180
private readonly moduleRe: RegExp;
11561181
private readonly findModule: FindModuleCallback;
1182+
private readonly findType: FindTypeCallback;
11571183

1158-
constructor(types: Map<string, PythonType>, findModule: FindModuleCallback, boundTo?: string, moduleName?: string) {
1184+
constructor(types: Map<string, PythonType>,
1185+
findModule: FindModuleCallback,
1186+
findType: FindTypeCallback,
1187+
boundTo?: string,
1188+
moduleName?: string) {
11591189
this.types = types;
11601190
this.findModule = findModule;
1191+
this.findType = findType;
11611192
this.moduleName = moduleName;
11621193
this.boundTo = boundTo !== undefined ? this.toPythonFQN(boundTo) : boundTo;
11631194

@@ -1174,6 +1205,7 @@ class TypeResolver {
11741205
return new TypeResolver(
11751206
this.types,
11761207
this.findModule,
1208+
this.findType,
11771209
fqn,
11781210
moduleName !== undefined ? moduleName : this.moduleName,
11791211
);
@@ -1211,6 +1243,10 @@ class TypeResolver {
12111243
return type;
12121244
}
12131245

1246+
public dereference(typeRef: spec.NamedTypeReference): spec.Type {
1247+
return this.findType(typeRef.fqn);
1248+
}
1249+
12141250
public resolve(
12151251
typeRef: spec.TypeReference,
12161252
opts: TypeResolverOpts = { forwardReferences: true, ignoreOptional: false }): string {
@@ -1381,7 +1417,11 @@ class PythonGenerator extends Generator {
13811417
}
13821418

13831419
protected onEndAssembly(_assm: spec.Assembly, _fingerprint: boolean) {
1384-
const resolver = new TypeResolver(this.types, (fqn: string) => this.findModule(fqn));
1420+
const resolver = new TypeResolver(
1421+
this.types,
1422+
(fqn: string) => this.findModule(fqn),
1423+
(fqn: string) => this.findType(fqn),
1424+
);
13851425
this.package.write(this.code, resolver);
13861426
}
13871427

packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,8 @@ def __init__(self) -> None:
559559
jsii.create(GiveMeStructs, self, [])
560560

561561
@jsii.member(jsii_name="derivedToFirst")
562-
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:
563-
derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive}
562+
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:
563+
derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive, "anumber": anumber, "astring": astring}
564564

565565
if another_optional is not None:
566566
derived["anotherOptional"] = another_optional
@@ -571,11 +571,14 @@ def derived_to_first(self, *, another_required: datetime.datetime, bool: bool, n
571571
if optional_array is not None:
572572
derived["optionalArray"] = optional_array
573573

574+
if first_optional is not None:
575+
derived["firstOptional"] = first_optional
576+
574577
return jsii.invoke(self, "derivedToFirst", [derived])
575578

576579
@jsii.member(jsii_name="readDerivedNonPrimitive")
577-
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":
578-
derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive}
580+
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":
581+
derived: DerivedStruct = {"anotherRequired": another_required, "bool": bool, "nonPrimitive": non_primitive, "anumber": anumber, "astring": astring}
579582

580583
if another_optional is not None:
581584
derived["anotherOptional"] = another_optional
@@ -586,6 +589,9 @@ def read_derived_non_primitive(self, *, another_required: datetime.datetime, boo
586589
if optional_array is not None:
587590
derived["optionalArray"] = optional_array
588591

592+
if first_optional is not None:
593+
derived["firstOptional"] = first_optional
594+
589595
return jsii.invoke(self, "readDerivedNonPrimitive", [derived])
590596

591597
@jsii.member(jsii_name="readFirstNumber")

0 commit comments

Comments
 (0)