fix(jsii): Validate overriding does not affect optionality (#549)

ptionality must remain unchancged when overriding methods and
properties, such that types are pure with respects to Liskov's
substitution.

Author: RomainMuller

packages/jsii/lib/validator.ts

@@ -265,7 +265,12 @@ function _defaultValidations(): ValidationFunction[] {
                 if (expParam.variadic !== actParam.variadic) {
                     diagnostic(ts.DiagnosticCategory.Error,
                                // tslint:disable-next-line:max-line-length
-                               `${label} changes the variadicity of parameter ${actParam.name} when ${action} (expected ${expParam.variadic}, found ${actParam.variadic})`);
+                               `${label} changes the variadicity of parameter ${actParam.name} when ${action} (expected ${!!expParam.variadic}, found ${!!actParam.variadic})`);
+                }
+                if (expParam.optional !== actParam.optional) {
+                    diagnostic(ts.DiagnosticCategory.Error,
+                               // tslint:disable-next-line: max-line-length
+                               `${label} changes the optionality of paramerter ${actParam.name} when ${action} (expected ${!!expParam.optional}, found ${!!actParam.optional})`);
                 }
             }
         }
@@ -281,6 +286,10 @@ function _defaultValidations(): ValidationFunction[] {
                 diagnostic(ts.DiagnosticCategory.Error,
                            `${label} changes immutability of property when ${action}`);
             }
+            if (expected.optional !== actual.optional) {
+                diagnostic(ts.DiagnosticCategory.Error,
+                           `${label} changes optionality of property when ${action}`);
+            }
         }
     }
 }

packages/jsii/test/negatives/neg.implementing-method-changes-optionality.1.ts

@@ -0,0 +1,12 @@
+///!MATCH_ERROR: jsii.Implementor#method changes the optionality of paramerter _optional when overriding jsii.AbstractClass (expected true, found false)
+
+// Attempt to change optionality of method parameter
+export abstract class AbstractClass {
+  public abstract method(required: string, optional?: number): void;
+}
+
+export class Implementor extends AbstractClass {
+  public method(_required: string, _optional: number): void {
+    // ...
+  }
+}

packages/jsii/test/negatives/neg.implementing-method-changes-optionality.2.ts

@@ -0,0 +1,14 @@
+///!MATCH_ERROR: jsii.Implementor#method changes the optionality of paramerter _optional when overriding jsii.ParentClass (expected true, found false)
+
+// Attempt to change optionality of method parameter
+export class ParentClass {
+  public method(_required: string, _optional?: number): void {
+    // ...
+  }
+}
+
+export class Implementor extends ParentClass {
+  public method(_required: string, _optional: number): void {
+    // ...
+  }
+}

packages/jsii/test/negatives/neg.implementing-method-changes-optionality.ts

@@ -0,0 +1,12 @@
+///!MATCH_ERROR: jsii.Implementor#method changes the optionality of paramerter _optional when implementing jsii.IInterface (expected true, found false)
+
+// Attempt to change optionality of method parameter
+export interface IInterface {
+  method(required: string, optional?: number): void;
+}
+
+export class Implementor implements IInterface {
+  public method(_required: string, _optional: number): void {
+    // ...
+  }
+}

packages/jsii/test/negatives/neg.implementing-property-changes-optionality.1.ts

@@ -0,0 +1,15 @@
+///!MATCH_ERROR: jsii.Implementor#property changes optionality of property when overriding jsii.AbstractClass
+
+// Attempt to change optionality of method parameter
+export abstract class AbstractClass {
+  public abstract property?: string;
+}
+
+export class Implementor extends AbstractClass {
+  public property: string;
+
+  constructor() {
+    super();
+    this.property = 'Bazinga!';
+  }
+}

packages/jsii/test/negatives/neg.implementing-property-changes-optionality.2.ts

@@ -0,0 +1,15 @@
+///!MATCH_ERROR: jsii.Implementor#property changes optionality of property when overriding jsii.ParentClass
+
+// Attempt to change optionality of method parameter
+export class ParentClass {
+  public property?: string = undefined;
+}
+
+export class Implementor extends ParentClass {
+  public property: string;
+
+  constructor() {
+    super();
+    this.property = 'Bazinga!';
+  }
+}

packages/jsii/test/negatives/neg.implementing-property-changes-optionality.ts

@@ -0,0 +1,14 @@
+///!MATCH_ERROR: jsii.Implementor#property changes optionality of property when implementing jsii.IInterface
+
+// Attempt to change optionality of method parameter
+export interface IInterface {
+  property?: string;
+}
+
+export class Implementor implements IInterface {
+  public property: string;
+
+  constructor() {
+    this.property = 'Bazinga!';
+  }
+}