Skip to content

Commit

Permalink
fix(python): Lift the entire data class hierarchy (#408)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@dstufft @RomainMuller
dstufft authored and RomainMuller committed Mar 28, 2019
1 parent 66789e8 commit f813620
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 10 deletions.
52 changes: 46 additions & 6 deletions packages/jsii-pacmak/lib/targets/python.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" : "";
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1139,6 +1163,7 @@ class Package {
}

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

interface TypeResolverOpts {
forwardReferences?: boolean;
Expand All @@ -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<string, PythonType>, findModule: FindModuleCallback, boundTo?: string, moduleName?: string) {
constructor(types: Map<string, PythonType>,
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;

Expand All @@ -1174,6 +1205,7 @@ class TypeResolver {
return new TypeResolver(
this.types,
this.findModule,
this.findType,
fqn,
moduleName !== undefined ? moduleName : this.moduleName,
);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
14 changes: 10 additions & 4 deletions packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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")
Expand Down

0 comments on commit f813620

Please sign in to comment.