Skip to content

Commit 28241cd

Browse files
author
Elad Ben-Israel
authored
fix(jsii-diff): crash when changing method to a property (#521)
Famous last words: "Trust me I know what I'm doing" Fixes #520
1 parent efae447 commit 28241cd

File tree

2 files changed

+66
-6
lines changed

2 files changed

+66
-6
lines changed

packages/jsii-diff/lib/classes-ifaces.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,16 @@ export function compareReferenceType<T extends reflect.ReferenceType>(original:
2727
context.mismatches.report(original, 'has gone from @subclassable to non-@subclassable');
2828
}
2929

30-
for (const [origMethod, updatedMethod] of memberPairs(original, original.allMethods, updated, context)) {
31-
compareMethod(original, origMethod, updatedMethod, context);
30+
for (const [origMethod, updatedElement] of memberPairs(original, original.allMethods, updated, context)) {
31+
if (reflect.isMethod(origMethod) && reflect.isMethod(updatedElement)) {
32+
compareMethod(original, origMethod, updatedElement, context);
33+
}
3234
}
3335

34-
for (const [origProp, updatedProp] of memberPairs(original, original.allProperties, updated, context)) {
35-
compareProperty(original, origProp, updatedProp, context);
36+
for (const [origProp, updatedElement] of memberPairs(original, original.allProperties, updated, context)) {
37+
if (reflect.isProperty(origProp) && reflect.isProperty(updatedElement)) {
38+
compareProperty(original, origProp, updatedElement, context);
39+
}
3640
}
3741

3842
// You cannot have added abstract members to the class/interface, as they are
@@ -174,7 +178,7 @@ function compareProperty(origClass: reflect.Type, original: reflect.Property, up
174178
}
175179

176180
// tslint:disable-next-line:max-line-length
177-
function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceType>(origClass: U, xs: T[], updatedClass: U, context: ComparisonContext): IterableIterator<[T, T]> {
181+
function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceType>(origClass: U, xs: T[], updatedClass: U, context: ComparisonContext): IterableIterator<[T, reflect.TypeMember]> {
178182
for (const origMember of xs.filter(shouldInspect(context))) {
179183
LOG.trace(`${origClass.fqn}#${origMember.name}`);
180184

@@ -192,7 +196,7 @@ function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceT
192196
context.mismatches.report(origClass, `member ${origMember.name} changed from 'public' to 'protected'`);
193197
}
194198

195-
yield [origMember, updatedMember as T]; // Trust me I know what I'm doing
199+
yield [origMember, updatedMember];
196200
}
197201
}
198202

packages/jsii-diff/test/test.classes.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,4 +328,60 @@ export = {
328328

329329
test.done();
330330
},
331+
332+
// ----------------------------------------------------------------------
333+
334+
async 'change from method to property'(test: Test) {
335+
await expectError(test,
336+
/CLASS testpkg.Boom member foo changed from method to property/,
337+
`
338+
export class Boom {
339+
foo() { return 12; }
340+
}
341+
`,
342+
`
343+
export class Boom {
344+
get foo() { return 12; }
345+
}
346+
`
347+
);
348+
349+
test.done();
350+
},
351+
352+
async 'change from method with arguments to property'(test: Test) {
353+
await expectError(test,
354+
/CLASS testpkg.Boom member foo changed from method to property/,
355+
`
356+
export class Boom {
357+
foo(arg: number) { return 12 * arg; }
358+
}
359+
`,
360+
`
361+
export class Boom {
362+
get foo() { return 12; }
363+
}
364+
`
365+
);
366+
367+
test.done();
368+
},
369+
370+
async 'change from property to method'(test: Test) {
371+
await expectError(test,
372+
/CLASS testpkg.Boom member foo changed from property to method/,
373+
`
374+
export class Boom {
375+
get foo() { return 12; }
376+
}
377+
`,
378+
`
379+
export class Boom {
380+
foo() { return 12; }
381+
}
382+
`
383+
);
384+
385+
test.done();
386+
}
331387
};

0 commit comments

Comments
 (0)