fix(jsii-diff): crash when changing method to a property (#521)

Famous last words: "Trust me I know what I'm doing" Fixes #520
aws · Jun 6, 2019 · 28241cd · 28241cd
1 parent efae447
commit 28241cd
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 6 deletions.
diff --git a/packages/jsii-diff/lib/classes-ifaces.ts b/packages/jsii-diff/lib/classes-ifaces.ts
@@ -27,12 +27,16 @@ export function compareReferenceType<T extends reflect.ReferenceType>(original:
     context.mismatches.report(original, 'has gone from @subclassable to non-@subclassable');
   }
 
-  for (const [origMethod, updatedMethod] of memberPairs(original, original.allMethods, updated, context)) {
-    compareMethod(original, origMethod, updatedMethod, context);
+  for (const [origMethod, updatedElement] of memberPairs(original, original.allMethods, updated, context)) {
+    if (reflect.isMethod(origMethod) && reflect.isMethod(updatedElement)) {
+      compareMethod(original, origMethod, updatedElement, context);
+    }
   }
 
-  for (const [origProp, updatedProp] of memberPairs(original, original.allProperties, updated, context)) {
-    compareProperty(original, origProp, updatedProp, context);
+  for (const [origProp, updatedElement] of memberPairs(original, original.allProperties, updated, context)) {
+    if (reflect.isProperty(origProp) && reflect.isProperty(updatedElement)) {
+      compareProperty(original, origProp, updatedElement, context);
+    }
   }
 
   // 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
 }
 
 // tslint:disable-next-line:max-line-length
-function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceType>(origClass: U, xs: T[], updatedClass: U, context: ComparisonContext): IterableIterator<[T, T]> {
+function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceType>(origClass: U, xs: T[], updatedClass: U, context: ComparisonContext): IterableIterator<[T, reflect.TypeMember]> {
   for (const origMember of xs.filter(shouldInspect(context))) {
     LOG.trace(`${origClass.fqn}#${origMember.name}`);
 
@@ -192,7 +196,7 @@ function* memberPairs<T extends reflect.TypeMember, U extends reflect.ReferenceT
       context.mismatches.report(origClass, `member ${origMember.name} changed from 'public' to 'protected'`);
     }
 
-    yield [origMember, updatedMember as T]; // Trust me I know what I'm doing
+    yield [origMember, updatedMember];
   }
 }
 

diff --git a/packages/jsii-diff/test/test.classes.ts b/packages/jsii-diff/test/test.classes.ts
@@ -328,4 +328,60 @@ export = {
 
     test.done();
   },
+
+  // ----------------------------------------------------------------------
+
+  async 'change from method to property'(test: Test) {
+    await expectError(test,
+      /CLASS testpkg.Boom member foo changed from method to property/,
+      `
+      export class Boom {
+        foo() { return 12; }
+      }
+      `,
+      `
+      export class Boom {
+        get foo() { return 12; }
+      }
+      `
+    );
+
+    test.done();
+  },
+
+  async 'change from method with arguments to property'(test: Test) {
+    await expectError(test,
+      /CLASS testpkg.Boom member foo changed from method to property/,
+      `
+      export class Boom {
+        foo(arg: number) { return 12 * arg; }
+      }
+      `,
+      `
+      export class Boom {
+        get foo() { return 12; }
+      }
+      `
+    );
+
+    test.done();
+  },
+
+  async 'change from property to method'(test: Test) {
+    await expectError(test,
+      /CLASS testpkg.Boom member foo changed from property to method/,
+      `
+      export class Boom {
+        get foo() { return 12; }
+      }
+      `,
+      `
+      export class Boom {
+        foo() { return 12; }
+      }
+      `
+    );
+
+    test.done();
+  }
 };