fix(jsii-diff): correctly handle assignability of type unions (#500)

packages/jsii-diff/lib/type-analysis.ts

@@ -4,7 +4,7 @@ import { flatMap } from './util';
 /**
  * Check whether A is a supertype of B
  *
- * Put differently: whether an value of type B would be assignable to a
+ * Put differently: whether any value of type B would be assignable to a
  * variable of type A.
  *
  * We always check the relationship in the NEW (latest, updated) typesystem.
@@ -33,16 +33,7 @@ export function isSuperType(a: reflect.TypeReference, b: reflect.TypeReference,
       `${b} is not assignable to ${a}`);
   }
 
-  // Any element of A should accept all of B
-  if (a.unionOfTypes !== undefined) {
-    const analyses = a.unionOfTypes.map(aaa => isSuperType(aaa, b, updatedSystem));
-    if (analyses.some(x => x.success)) { return { success: true }; }
-    return failure(
-      `none of ${b} are assignable to ${a}`,
-      ...flatMap(analyses, x => x.success ? [] : x.reasons)
-    );
-  }
-  // All potential elements of B should go into A
+  // Every element of B can be assigned to A
   if (b.unionOfTypes !== undefined) {
     const analyses = b.unionOfTypes.map(bbb => isSuperType(a, bbb, updatedSystem));
     if (analyses.every(x => x.success)) { return { success: true }; }
@@ -51,6 +42,15 @@ export function isSuperType(a: reflect.TypeReference, b: reflect.TypeReference,
       ...flatMap(analyses, x => x.success ? [] : x.reasons)
     );
   }
+  // There should be an element of A which can accept all of B
+  if (a.unionOfTypes !== undefined) {
+    const analyses = a.unionOfTypes.map(aaa => isSuperType(aaa, b, updatedSystem));
+    if (analyses.some(x => x.success)) { return { success: true }; }
+    return failure(
+      `none of ${b} are assignable to ${a}`,
+      ...flatMap(analyses, x => x.success ? [] : x.reasons)
+    );
+  }
 
   // For named types, we'll always do a nominal typing relationship.
   // That is, if in the updated typesystem someone were to use the type name

packages/jsii-diff/package.json

@@ -35,6 +35,11 @@
         "typescript": "^3.4.3",
         "yargs": "^13.2.2"
     },
+    "repository": {
+        "type": "git",
+        "url": "https://github.com/awslabs/jsii.git",
+        "directory": "packages/jsii-diff"
+    },
     "nyc": {
         "reporter": [
             "lcov",

packages/jsii-diff/test/test.type-unions.ts

@@ -0,0 +1,145 @@
+import { Test } from 'nodeunit';
+import { expectError, expectNoError } from './util';
+
+export = {
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in return structs can be the same'(test: Test) {
+    await expectNoError(test,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `);
+
+    test.done();
+  },
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in return structs can be narrowed'(test: Test) {
+    await expectNoError(test,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `);
+
+    test.done();
+  },
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in return structs can not be widened'(test: Test) {
+    await expectError(test,
+      /some of string \| number \| boolean are not assignable to string \| number/,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string | number | boolean;
+      }
+      export class Actions {
+        returnHenk(): Henk { return { henk: 'henk' }; }
+      }
+    `);
+
+    test.done();
+  },
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in input structs can be the same'(test: Test) {
+    await expectNoError(test,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `);
+
+    test.done();
+  },
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in input structs can be widened'(test: Test) {
+    await expectNoError(test,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string | number | boolean;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `);
+
+    test.done();
+  },
+
+  // ----------------------------------------------------------------------
+
+  async 'type unions in input structs can not be narrowed'(test: Test) {
+    await expectError(test,
+      /string \| number is not assignable to string/,
+      `
+      export interface Henk {
+        readonly henk: string | number;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `, `
+      export interface Henk {
+        readonly henk: string;
+      }
+      export class Actions {
+        takeHenk(_henk: Henk): void { }
+      }
+    `);
+
+    test.done();
+  },
+};