Skip to content

Commit

Permalink
fix(jsii-diff): detect interfaces inherited via interfaces (#1492)
Browse files Browse the repository at this point in the history
* fix(jsii-diff): detect interfaces inherited via interfaces

`jsii-diff` is currently throwing on CDK because it thinks interfaces
are being removed from the inheritance chain, even if they've actually
been moved to base classes.

This is because the `ClassType.getInterfaces()` method of `jsii-reflect`
was only returning its direct interfaces, even when run with
`inherited=true` (as opposed to recursively returning the interfaces
all the way up the tree).

Q: Why did you not add a unit test in `jsii-reflect`?

A: The set suite of `jsii-reflect` is quite anemic, and basically only
tests that we can mostly deduce the types that are present in an
assembly. It doesn't have good testing infrastructure for actually
asserting things about arbitrary small pieces of typescript, instead it
mostly works on the generated assemblies from the `jsii-calc` etc.
examples. `jsii-diff` *does* have that testing infrastructure, and so
`jsii-diff` serves as the test suite for `jsii-reflect` at the moment...

* Satisfy eslint

Co-authored-by: Mitchell Valine <valinm@amazon.com>
  • Loading branch information
rix0rrr and MrArnoldPalmer committed Apr 7, 2020
1 parent 44724fb commit e03a638
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
52 changes: 52 additions & 0 deletions packages/jsii-diff/test/classes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,58 @@ describe('implement base types need to be present in updated type system', () =>
`)
);

test('change direct implementation to inherited implementation via interface', () =>
expectNoError(
`
export interface IPapa {
readonly pipe: string;
}
export class Bebe implements IPapa {
readonly pipe: string = 'pff';
readonly pacifier: string = 'mmm';
}
`, `
export interface IPapa {
readonly pipe: string;
}
export interface IInbetween extends IPapa {
}
export class Bebe implements IInbetween {
readonly pipe: string = 'pff';
readonly pacifier: string = 'mmm';
}
`)
);

test('change direct implementation to inherited implementation via base class', () =>
expectNoError(
`
export interface IPapa {
readonly pipe: string;
}
export class Bebe implements IPapa {
readonly pipe: string = 'pff';
readonly pacifier: string = 'mmm';
}
`, `
export interface IPapa {
readonly pipe: string;
}
export class Inbetween implements IPapa {
readonly pipe: string = 'pff';
}
export class Bebe extends Inbetween {
readonly pacifier: string = 'mmm';
}
`)
);

});

// ----------------------------------------------------------------------
Expand Down
9 changes: 8 additions & 1 deletion packages/jsii-reflect/lib/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ export class ClassType extends ReferenceType {
out.push(...this.base.getInterfaces(inherited));
}
if (this.spec.interfaces) {
out.push(...this.spec.interfaces.map(iface => this.system.findInterface(iface)));
out.push(...flatten(this.spec.interfaces
.map(fqn => this.system.findInterface(fqn))
.map(iface => [iface, ...inherited ? iface.getInterfaces(true) : []]),
));
}
return out;
}
Expand All @@ -111,3 +114,7 @@ export class ClassType extends ReferenceType {
m => m.name));
}
}

function flatten<A>(xs: A[][]): A[] {
return Array.prototype.concat([], ...xs);
}

0 comments on commit e03a638

Please sign in to comment.