Refactor code with and enforce strictNullChecks

This commit applies `strictNullChecks` to the entire codebase to improve
maintainability and type safety. Key changes include:

- Remove some explicit null-checks where unnecessary.
- Add necessary null-checks.
- Refactor static factory functions for a more functional approach.
- Improve some test names and contexts for better debugging.
- Add unit tests for any additional logic introduced.
- Refactor `createPositionFromRegexFullMatch` to its own function as the
  logic is reused.
- Prefer `find` prefix on functions that may return `undefined` and
  `get` prefix for those that always return a value.

cypress.config.ts

@@ -11,8 +11,16 @@ export default defineConfig({
   videosFolder: `${CYPRESS_BASE_DIR}/videos`,
 
   e2e: {
-    baseUrl: `http://localhost:${ViteConfig.server.port}/`,
+    baseUrl: `http://localhost:${getApplicationPort()}/`,
     specPattern: `${CYPRESS_BASE_DIR}/**/*.cy.{js,jsx,ts,tsx}`, // Default: cypress/e2e/**/*.cy.{js,jsx,ts,tsx}
     supportFile: `${CYPRESS_BASE_DIR}/support/e2e.ts`,
   },
 });
+
+function getApplicationPort(): number {
+  const port = ViteConfig.server?.port;
+  if (port === undefined) {
+    throw new Error('Unknown application port');
+  }
+  return port;
+}

docs/tests.md

@@ -68,13 +68,13 @@ These checks validate various qualities like runtime execution, building process
 - [`./src/`](./../src/): Contains the code subject to testing.
 - [`./tests/shared/`](./../tests/shared/): Contains code shared by different test categories.
   - [`bootstrap/setup.ts`](./../tests/shared/bootstrap/setup.ts): Initializes unit and integration tests.
+  - [`Assertions/`](./../tests/shared/Assertions/): Contains common assertion functions, prefixed with `expect`.
 - [`./tests/unit/`](./../tests/unit/)
   - Stores unit test code.
   - The directory structure mirrors [`./src/`](./../src).
     - E.g., tests for [`./src/application/ApplicationFactory.ts`](./../src/application/ApplicationFactory.ts) reside in [`./tests/unit/application/ApplicationFactory.spec.ts`](./../tests/unit/application/ApplicationFactory.spec.ts).
   - [`shared/`](./../tests/unit/shared/)
     - Contains shared unit test functionalities.
-    - [`Assertions/`](./../tests/unit/shared/Assertions): Contains common assertion functions, prefixed with `expect`.
     - [`TestCases/`](./../tests/unit/shared/TestCases/)
       - Shared test cases.
       - Functions that calls `it()` from [Vitest](https://vitest.dev/) should have `it` prefix.

src/application/ApplicationFactory.ts

@@ -12,9 +12,6 @@ export class ApplicationFactory implements IApplicationFactory {
   private readonly getter: AsyncLazy<IApplication>;
 
   protected constructor(costlyGetter: ApplicationGetterType) {
-    if (!costlyGetter) {
-      throw new Error('missing getter');
-    }
     this.getter = new AsyncLazy<IApplication>(() => Promise.resolve(costlyGetter()));
   }
 

src/application/Common/Array.ts

@@ -1,7 +1,5 @@
 // Compares to Array<T> objects for equality, ignoring order
 export function scrambledEqual<T>(array1: readonly T[], array2: readonly T[]) {
-  if (!array1) { throw new Error('missing first array'); }
-  if (!array2) { throw new Error('missing second array'); }
   const sortedArray1 = sort(array1);
   const sortedArray2 = sort(array2);
   return sequenceEqual(sortedArray1, sortedArray2);
@@ -12,8 +10,6 @@ export function scrambledEqual<T>(array1: readonly T[], array2: readonly T[]) {
 
 // Compares to Array<T> objects for equality in same order
 export function sequenceEqual<T>(array1: readonly T[], array2: readonly T[]) {
-  if (!array1) { throw new Error('missing first array'); }
-  if (!array2) { throw new Error('missing second array'); }
   if (array1.length !== array2.length) {
     return false;
   }

src/application/Common/CustomError.ts

@@ -20,31 +20,38 @@ export abstract class CustomError extends Error {
   }
 }
 
-export const Environment = {
+interface ErrorPrototypeManipulation {
+  getSetPrototypeOf: () => (typeof Object.setPrototypeOf | undefined);
+  getCaptureStackTrace: () => (typeof Error.captureStackTrace | undefined);
+}
+
+export const PlatformErrorPrototypeManipulation: ErrorPrototypeManipulation = {
   getSetPrototypeOf: () => Object.setPrototypeOf,
   getCaptureStackTrace: () => Error.captureStackTrace,
 };
 
 function fixPrototype(target: Error, prototype: CustomError) {
-  // https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget
-  const setPrototypeOf = Environment.getSetPrototypeOf();
-  if (!functionExists(setPrototypeOf)) {
+  // This is recommended by TypeScript guidelines.
+  //  Source: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget
+  //  Snapshots: https://web.archive.org/web/20231111234849/https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget, https://archive.ph/tr7cX#support-for-newtarget
+  const setPrototypeOf = PlatformErrorPrototypeManipulation.getSetPrototypeOf();
+  if (!isFunction(setPrototypeOf)) {
     return;
   }
   setPrototypeOf(target, prototype);
 }
 
 function ensureStackTrace(target: Error) {
-  const captureStackTrace = Environment.getCaptureStackTrace();
-  if (!functionExists(captureStackTrace)) {
+  const captureStackTrace = PlatformErrorPrototypeManipulation.getCaptureStackTrace();
+  if (!isFunction(captureStackTrace)) {
     // captureStackTrace is only available on V8, if it's not available
     // modern JS engines will usually generate a stack trace on error objects when they're thrown.
     return;
   }
   captureStackTrace(target, target.constructor);
 }
 
-function functionExists(func: unknown): boolean {
-  // Not doing truthy/falsy check i.e. if(func) as most values are truthy in JS for robustness
+// eslint-disable-next-line @typescript-eslint/ban-types
+function isFunction(func: unknown): func is Function {
   return typeof func === 'function';
 }

src/application/Common/Enum.ts

@@ -54,9 +54,6 @@ export function assertInRange<T extends EnumType, TEnumValue extends EnumType>(
   value: TEnumValue,
   enumVariable: EnumVariable<T, TEnumValue>,
 ) {
-  if (value === undefined || value === null) {
-    throw new Error('absent enum value');
-  }
   if (!(value in enumVariable)) {
     throw new RangeError(`enum value "${value}" is out of range`);
   }

src/application/Common/ScriptingLanguage/ScriptingLanguageFactory.ts

@@ -9,19 +9,16 @@ export abstract class ScriptingLanguageFactory<T> implements IScriptingLanguageF
 
   public create(language: ScriptingLanguage): T {
     assertInRange(language, ScriptingLanguage);
-    if (!this.getters.has(language)) {
+    const getter = this.getters.get(language);
+    if (!getter) {
       throw new RangeError(`unknown language: "${ScriptingLanguage[language]}"`);
     }
-    const getter = this.getters.get(language);
     const instance = getter();
     return instance;
   }
 
   protected registerGetter(language: ScriptingLanguage, getter: Getter<T>) {
     assertInRange(language, ScriptingLanguage);
-    if (!getter) {
-      throw new Error('missing getter');
-    }
     if (this.getters.has(language)) {
       throw new Error(`${ScriptingLanguage[language]} is already registered`);
     }

src/application/Context/ApplicationContext.ts

@@ -26,7 +26,6 @@ export class ApplicationContext implements IApplicationContext {
     public readonly app: IApplication,
     initialContext: OperatingSystem,
   ) {
-    validateApp(app);
     this.states = initializeStates(app);
     this.changeContext(initialContext);
   }
@@ -36,10 +35,8 @@ export class ApplicationContext implements IApplicationContext {
     if (this.currentOs === os) {
       return;
     }
-    this.collection = this.app.getCollection(os);
-    if (!this.collection) {
-      throw new Error(`os "${OperatingSystem[os]}" is not defined in application`);
-    }
+    const collection = this.app.getCollection(os);
+    this.collection = collection;
     const event: IApplicationContextChangedEvent = {
       newState: this.states[os],
       oldState: this.states[this.currentOs],
@@ -49,12 +46,6 @@ export class ApplicationContext implements IApplicationContext {
   }
 }
 
-function validateApp(app: IApplication) {
-  if (!app) {
-    throw new Error('missing app');
-  }
-}
-
 function initializeStates(app: IApplication): StateMachine {
   const machine = new Map<OperatingSystem, ICategoryCollectionState>();
   for (const collection of app.collections) {

src/application/Context/ApplicationContextFactory.ts

@@ -10,20 +10,25 @@ export async function buildContext(
   factory: IApplicationFactory = ApplicationFactory.Current,
   environment = RuntimeEnvironment.CurrentEnvironment,
 ): Promise<IApplicationContext> {
-  if (!factory) { throw new Error('missing factory'); }
-  if (!environment) { throw new Error('missing environment'); }
   const app = await factory.getApp();
   const os = getInitialOs(app, environment.os);
   return new ApplicationContext(app, os);
 }
 
-function getInitialOs(app: IApplication, currentOs: OperatingSystem): OperatingSystem {
+function getInitialOs(
+  app: IApplication,
+  currentOs: OperatingSystem | undefined,
+): OperatingSystem {
   const supportedOsList = app.getSupportedOsList();
-  if (supportedOsList.includes(currentOs)) {
+  if (currentOs !== undefined && supportedOsList.includes(currentOs)) {
     return currentOs;
   }
+  return getMostSupportedOs(supportedOsList, app);
+}
+
+function getMostSupportedOs(supportedOsList: OperatingSystem[], app: IApplication) {
   supportedOsList.sort((os1, os2) => {
-    const getPriority = (os: OperatingSystem) => app.getCollection(os).totalScripts;
+    const getPriority = (os: OperatingSystem) => app.getCollection(os)?.totalScripts ?? 0;
     return getPriority(os2) - getPriority(os1);
   });
   return supportedOsList[0];

src/application/Context/State/Code/ApplicationCode.ts

@@ -21,9 +21,6 @@ export class ApplicationCode implements IApplicationCode {
     private readonly scriptingDefinition: IScriptingDefinition,
     private readonly generator: IUserScriptGenerator = new UserScriptGenerator(),
   ) {
-    if (!userSelection) { throw new Error('missing userSelection'); }
-    if (!scriptingDefinition) { throw new Error('missing scriptingDefinition'); }
-    if (!generator) { throw new Error('missing generator'); }
     this.setCode(userSelection.selectedScripts);
     userSelection.changed.on((scripts) => {
       this.setCode(scripts);

src/application/Context/State/Code/Event/CodeChangedEvent.ts

@@ -36,7 +36,11 @@ export class CodeChangedEvent implements ICodeChangedEvent {
   }
 
   public getScriptPositionInCode(script: IScript): ICodePosition {
-    return this.scripts.get(script);
+    const position = this.scripts.get(script);
+    if (!position) {
+      throw new Error('Unknown script: Position could not be found for the script');
+    }
+    return position;
   }
 }
 

src/application/Context/State/Code/Generation/CodeBuilder.ts

@@ -16,7 +16,9 @@ export abstract class CodeBuilder implements ICodeBuilder {
       return this;
     }
     const lines = code.match(/[^\r\n]+/g);
-    this.lines.push(...lines);
+    if (lines) {
+      this.lines.push(...lines);
+    }
     return this;
   }
 

src/application/Context/State/Code/Generation/UserScriptGenerator.ts

@@ -17,8 +17,6 @@ export class UserScriptGenerator implements IUserScriptGenerator {
     selectedScripts: ReadonlyArray<SelectedScript>,
     scriptingDefinition: IScriptingDefinition,
   ): IUserScript {
-    if (!selectedScripts) { throw new Error('missing scripts'); }
-    if (!scriptingDefinition) { throw new Error('missing definition'); }
     if (!selectedScripts.length) {
       return { code: '', scriptPositions: new Map<SelectedScript, ICodePosition>() };
     }
@@ -68,7 +66,7 @@ function appendSelection(
 function appendCode(selection: SelectedScript, builder: ICodeBuilder): ICodeBuilder {
   const { script } = selection;
   const name = selection.revert ? `${script.name} (revert)` : script.name;
-  const scriptCode = selection.revert ? script.code.revert : script.code.execute;
+  const scriptCode = selection.revert ? (script.code.revert ?? '') : script.code.execute;
   return builder
     .appendLine()
     .appendFunction(name, scriptCode);

src/application/Context/State/Filter/Event/FilterChange.ts

@@ -1,37 +1,37 @@
 import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
 import { FilterActionType } from './FilterActionType';
-import { IFilterChangeDetails, IFilterChangeDetailsVisitor } from './IFilterChangeDetails';
+import {
+  IFilterChangeDetails, IFilterChangeDetailsVisitor,
+  ApplyFilterAction, ClearFilterAction,
+} from './IFilterChangeDetails';
 
 export class FilterChange implements IFilterChangeDetails {
-  public static forApply(filter: IFilterResult) {
-    if (!filter) {
-      throw new Error('missing filter');
-    }
-    return new FilterChange(FilterActionType.Apply, filter);
+  public static forApply(
+    filter: IFilterResult,
+  ): IFilterChangeDetails {
+    return new FilterChange({ type: FilterActionType.Apply, filter });
   }
 
-  public static forClear() {
-    return new FilterChange(FilterActionType.Clear);
+  public static forClear(): IFilterChangeDetails {
+    return new FilterChange({ type: FilterActionType.Clear });
   }
 
-  private constructor(
-    public readonly actionType: FilterActionType,
-    public readonly filter?: IFilterResult,
-  ) { }
+  private constructor(public readonly action: ApplyFilterAction | ClearFilterAction) { }
 
   public visit(visitor: IFilterChangeDetailsVisitor): void {
-    if (!visitor) {
-      throw new Error('missing visitor');
-    }
-    switch (this.actionType) {
+    switch (this.action.type) {
       case FilterActionType.Apply:
-        visitor.onApply(this.filter);
+        if (visitor.onApply) {
+          visitor.onApply(this.action.filter);
+        }
         break;
       case FilterActionType.Clear:
-        visitor.onClear();
+        if (visitor.onClear) {
+          visitor.onClear();
+        }
         break;
       default:
-        throw new Error(`Unknown action type: ${this.actionType}`);
+        throw new Error(`Unknown action: ${this.action}`);
     }
   }
 }

src/application/Context/State/Filter/Event/IFilterChangeDetails.ts

@@ -2,13 +2,22 @@ import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult'
 import { FilterActionType } from './FilterActionType';
 
 export interface IFilterChangeDetails {
-  readonly actionType: FilterActionType;
-  readonly filter?: IFilterResult;
-
+  readonly action: FilterAction;
   visit(visitor: IFilterChangeDetailsVisitor): void;
 }
 
 export interface IFilterChangeDetailsVisitor {
-  onClear(): void;
-  onApply(filter: IFilterResult): void;
+  readonly onClear?: () => void;
+  readonly onApply?: (filter: IFilterResult) => void;
 }
+
+export type ApplyFilterAction = {
+  readonly type: FilterActionType.Apply,
+  readonly filter: IFilterResult;
+};
+
+export type ClearFilterAction = {
+  readonly type: FilterActionType.Clear,
+};
+
+export type FilterAction = ApplyFilterAction | ClearFilterAction;

src/application/Context/State/Selection/UserSelection.ts

@@ -43,7 +43,7 @@ export class UserSelection implements IUserSelection {
   }
 
   public removeAllInCategory(categoryId: number): void {
-    const category = this.collection.findCategory(categoryId);
+    const category = this.collection.getCategory(categoryId);
     const scriptsToRemove = category.getAllScriptsRecursively()
       .filter((script) => this.scripts.exists(script.id));
     if (!scriptsToRemove.length) {
@@ -57,7 +57,7 @@ export class UserSelection implements IUserSelection {
 
   public addOrUpdateAllInCategory(categoryId: number, revert = false): void {
     const scriptsToAddOrUpdate = this.collection
-      .findCategory(categoryId)
+      .getCategory(categoryId)
       .getAllScriptsRecursively()
       .filter(
         (script) => !this.scripts.exists(script.id)
@@ -74,7 +74,7 @@ export class UserSelection implements IUserSelection {
   }
 
   public addSelectedScript(scriptId: string, revert: boolean): void {
-    const script = this.collection.findScript(scriptId);
+    const script = this.collection.getScript(scriptId);
     if (!script) {
       throw new Error(`Cannot add (id: ${scriptId}) as it is unknown`);
     }
@@ -84,7 +84,7 @@ export class UserSelection implements IUserSelection {
   }
 
   public addOrUpdateSelectedScript(scriptId: string, revert: boolean): void {
-    const script = this.collection.findScript(scriptId);
+    const script = this.collection.getScript(scriptId);
     const selectedScript = new SelectedScript(script, revert);
     this.scripts.addOrUpdateItem(selectedScript);
     this.changed.notify(this.scripts.getItems());

src/application/Parser/ApplicationParser.ts

@@ -32,10 +32,7 @@ const PreParsedCollections: readonly CollectionData [] = [
 ];
 
 function validateCollectionsData(collections: readonly CollectionData[]) {
-  if (!collections?.length) {
+  if (!collections.length) {
     throw new Error('missing collections');
   }
-  if (collections.some((collection) => !collection)) {
-    throw new Error('missing collection provided');
-  }
 }