From e5a23bd7db1dc43db8ad2c901def120c1d04c102 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 5 Sep 2024 21:17:38 +0800 Subject: [PATCH] commitlint: do not use empty class for None Empty classes result in strange issues that turn the TS compiler into not very helpful. E.g.: https://stackoverflow.com/questions/78953267/why-is-typescript-not-throwing-a-compile-time-error-when-constructor-is-not-used NOTE: we still mark the new methods as deprecated to give a hint that `instanceof` usage is better. These tooltips will be properly shown by IDEs and linters according to: https://stackoverflow.com/a/62991642/544947 --- commitlint/fpHelpers.ts | 30 ++++++++++++++++++++++++++++-- commitlint/tests/testHelpers.ts | 9 +++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/commitlint/fpHelpers.ts b/commitlint/fpHelpers.ts index d01c94f1..37e022c0 100644 --- a/commitlint/fpHelpers.ts +++ b/commitlint/fpHelpers.ts @@ -1,9 +1,35 @@ -export class None {} +interface IOption { + /** + * @deprecated it is better to use `if (foo instanceof None)` so that you can access the .value in the `else` case + **/ + IsNone(): boolean; + /** + * @deprecated it is better to use `if (!(foo instanceof None))` so that you can access the .value inside the `if` block + **/ + IsSome(): boolean; +} + +export class None { + public IsNone(): boolean { + return true; + } + public IsSome(): boolean { + return false; + } +} export class Some { value: T; constructor(val: T) { this.value = val; } + + public IsNone(): boolean { + return false; + } + public IsSome(): boolean { + return true; + } } -export type Option = None | Some; + +export type Option = (None | Some) & IOption; diff --git a/commitlint/tests/testHelpers.ts b/commitlint/tests/testHelpers.ts index 5d4b9392..0cd6497e 100644 --- a/commitlint/tests/testHelpers.ts +++ b/commitlint/tests/testHelpers.ts @@ -19,6 +19,15 @@ test("testing Options", () => { expect(typeGuard(bar)).toBe("4"); }); +test("testing Is methods", () => { + let foo: Option = new None(); + let bar: Option = new Some(2); + expect(foo.IsNone()).toBe(true); + expect(bar.IsNone()).toBe(false); + expect(foo.IsSome()).toBe(false); + expect(bar.IsSome()).toBe(true); +}); + export function runCommitLintOnMsg(inputMsg: string) { // FIXME: should we .lowerCase().startsWith("win") in case it starts // returning Win64 in the future? thing is, our CI doesn't like this