Skip to content

Commit

Permalink
fix(python): duplicated kwargs when field is multi-inherited (#2654)
Browse files Browse the repository at this point in the history
When a struct field is inherited from more than one parent (this could
be twice from the same type - in the case of a diamond shape; or from
two distinct parents that declare the same field), the lifted keyword
arguments in python would be duplicated for this field.

This is because the method that performs the keyword argument lifting
did not perform name-based de-duplication, and operates directly on the
"raw" assembly (whereby it must traverse the inheritance tree itself,
as opposed to the Go generator which uses `jsii-reflect` and has the
field collection done by that library).

Fixes #2653
  • Loading branch information
RomainMuller authored Mar 18, 2021
1 parent 7d0cfc5 commit 3cd9d19
Show file tree
Hide file tree
Showing 3 changed files with 1,468 additions and 13 deletions.
38 changes: 25 additions & 13 deletions packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,8 @@ abstract class BaseMethod implements PythonBase {
// resolved, so build up a list of all of the prop names so we can check against
// them later.
const liftedPropNames = new Set<string>();
if (this.liftedProp?.properties?.length ?? 0 >= 1) {
for (const prop of this.liftedProp!.properties!) {
if (this.liftedProp?.properties != null) {
for (const prop of this.liftedProp.properties) {
liftedPropNames.add(toPythonParameterName(prop.name));
}
}
Expand Down Expand Up @@ -751,24 +751,36 @@ abstract class BaseMethod implements PythonBase {
const liftedProperties: spec.Property[] = [];

const stack = [this.liftedProp];
let current = stack.shift();
while (current !== undefined) {
const knownIfaces = new Set<string>();
const knownProps = new Set<string>();
for (
let current = stack.shift();
current != null;
current = stack.shift()
) {
knownIfaces.add(current.fqn);

// 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,
),
);
for (const iface of current.interfaces) {
if (knownIfaces.has(iface)) {
continue;
}
stack.push(resolver.dereference(iface) as spec.InterfaceType);
knownIfaces.add(iface);
}
}

// Add all of the properties of this interface to our list of properties.
if (current.properties !== undefined) {
liftedProperties.push(...current.properties);
for (const prop of current.properties) {
if (knownProps.has(prop.name)) {
continue;
}
liftedProperties.push(prop);
knownProps.add(prop.name);
}
}

// Finally, grab our next item.
current = stack.shift();
}

return liftedProperties;
Expand Down
Loading

0 comments on commit 3cd9d19

Please sign in to comment.