Refactor: ESLint strict-boolean-expressions (#759)

.eslintrc

@@ -81,8 +81,19 @@
     // ***** Operands *****
     "@typescript-eslint/prefer-nullish-coalescing": "error",
 
+    // ***** Conditionals *****
+    // Don't allow unnecessary conditional checks, such as when a value is always true, which can also help catch cases
+    // such as accidentally checking `if([]){}` vs. `if([].length){}`
+    "@typescript-eslint/strict-boolean-expressions": ["error", {
+      "allowAny": true,
+      "allowNullableBoolean": true,
+      "allowNullableString": true
+    }],
+
     // ***** Arrays *****
+    // Try to enforce early terminations of loops, rather than statements such as `.find(x=>x)[0]`
     "unicorn/prefer-array-find": "error",
+    // TypeScript doesn't do a good job of reporting indexed values as potentially undefined, such as `[1,2,3][999]`
     "unicorn/prefer-at": "error",
 
     // ***** Numbers *****

src/modules/argumentsParser.ts

@@ -712,6 +712,7 @@ Example use cases:
         type: 'boolean',
       })
       .fail((msg, err, _yargs) => {
+        // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
         if (err) {
           throw err;
         }

src/modules/candidateGenerator.ts

@@ -231,7 +231,7 @@ export default class CandidateGenerator extends Module {
         .filter(([, roms]) => roms.length === game.getRoms().length)
         .map(([archive]) => archive);
 
-      const archiveWithEveryRom = archivesWithEveryRom[0];
+      const archiveWithEveryRom = archivesWithEveryRom.at(0);
       if (archiveWithEveryRom) {
         // An Archive was found, use that as the only possible input file
         // For each of this Game's ROMs, find the matching ArchiveEntry from this Archive

src/modules/datScanner.ts

@@ -193,7 +193,7 @@ export default class DATScanner extends Scanner {
         });
 
         proc.on('exit', (code) => {
-          if (code) {
+          if (code !== null && code > 0) {
             reject(new Error(`exit code ${code}`));
             return;
           }

src/polyfill/arrayPoly.ts

@@ -28,7 +28,7 @@ export default class ArrayPoly {
   public static filterUniqueMapped<T, V>(
     mapper: (arg: T) => V,
   ): (value: T, idx: number, values: T[]) => boolean {
-    let mappedValues: V[];
+    let mappedValues: V[] | undefined;
     return (value: T, idx: number, values: T[]): boolean => {
       if (!mappedValues) {
         mappedValues = values.map((val) => mapper(val));

src/types/datStatus.ts

@@ -245,7 +245,7 @@ export default class DATStatus {
 
         const foundReleaseCandidate = foundReleaseCandidates
           .find((rc) => rc && rc.getGame().equals(game));
-        if (foundReleaseCandidate ?? !game.getRoms().length) {
+        if (foundReleaseCandidate !== undefined || !game.getRoms().length) {
           status = GameStatus.FOUND;
         }
 

src/types/dats/game.ts

@@ -112,12 +112,12 @@ export default class Game implements GameProps {
   @Expose()
   @Type(() => Release)
   @Transform(({ value }) => value || [])
-  readonly release: Release | Release[];
+  readonly release?: Release | Release[];
 
   @Expose()
   @Type(() => ROM)
   @Transform(({ value }) => value || [])
-  readonly rom: ROM | ROM[];
+  readonly rom?: ROM | ROM[];
 
   constructor(props?: GameProps) {
     this.name = props?.name ?? '';

src/types/dats/logiqx/logiqxDat.ts

@@ -22,13 +22,13 @@ export default class LogiqxDAT extends DAT {
   @Expose()
   @Type(() => Game)
   @Transform(({ value }) => value || [])
-  private readonly game: Game | Game[];
+  private readonly game?: Game | Game[];
 
   // NOTE(cemmer): this is not Logiqx DTD-compliant, but it's what pleasuredome Datfiles use
   @Expose()
   @Type(() => Machine)
   @Transform(({ value }) => value || [])
-  private readonly machine: Machine | Machine[];
+  private readonly machine?: Machine | Machine[];
 
   constructor(header: Header, games: Game | Game[]) {
     super();
@@ -89,7 +89,7 @@ export default class LogiqxDAT extends DAT {
     }
 
     if (Array.isArray(this.machine)) {
-      if (this.machine) {
+      if (this.machine.length) {
         return this.machine;
       }
     } else if (this.machine) {

src/types/dats/mame/machine.ts

@@ -14,7 +14,7 @@ export default class Machine extends Game implements MachineProps {
   @Expose({ name: 'device_ref' })
   @Type(() => DeviceRef)
   @Transform(({ value }) => value || [])
-  readonly deviceRef: DeviceRef | DeviceRef[];
+  readonly deviceRef?: DeviceRef | DeviceRef[];
 
   constructor(props?: MachineProps) {
     super(props);

src/types/dats/mame/mameDat.ts

@@ -23,7 +23,7 @@ export default class MameDAT extends DAT {
   @Expose()
   @Type(() => Machine)
   @Transform(({ value }) => value || [])
-  private readonly machine: Machine | Machine[];
+  private readonly machine?: Machine | Machine[];
 
   constructor(machine: Machine | Machine[]) {
     super();
@@ -50,7 +50,7 @@ export default class MameDAT extends DAT {
 
   getGames(): Game[] {
     if (Array.isArray(this.machine)) {
-      if (this.machine) {
+      if (this.machine.length) {
         return this.machine;
       }
     } else if (this.machine) {

src/types/files/archives/tar.ts

@@ -27,7 +27,7 @@ export default class Tar extends Archive {
 
     // WARN(cemmer): entries in tar archives don't have headers, the entire file has to be read to
     // calculate the CRCs
-    let errorMessage;
+    let errorMessage: string | undefined;
     const writeStream = new tar.Parse({
       onwarn: (code, message): void => {
         errorMessage = `${code}: ${message}`;

src/types/options.ts

@@ -634,7 +634,7 @@ export default class Options implements OptionsProps {
     if (await fsPoly.isDirectory(inputPath)) {
       const dirPaths = (await fsPoly.walk(inputPathNormalized, walkCallback))
         .map((filePath) => path.normalize(filePath));
-      if (!dirPaths || !dirPaths.length) {
+      if (!dirPaths.length) {
         if (!requireFiles) {
           return [];
         }
@@ -652,7 +652,7 @@ export default class Options implements OptionsProps {
     // Otherwise, process it as a glob pattern
     const paths = (await fg(inputPathNormalized, { onlyFiles: true }))
       .map((filePath) => path.normalize(filePath));
-    if (!paths || !paths.length) {
+    if (!paths.length) {
       if (URLPoly.canParse(inputPath)) {
         // Allow URLs, let the scanner modules deal with them
         walkCallback(1);

src/types/outputFactory.ts

@@ -381,12 +381,11 @@ export default class OutputFactory {
       output = path.join(dir, output);
     }
 
-    if (game && (
-      (options.getDirGameSubdir() === GameSubdirMode.MULTIPLE
+    if ((options.getDirGameSubdir() === GameSubdirMode.MULTIPLE
         && game.getRoms().length > 1
         && !FileFactory.isArchive(ext))
       || options.getDirGameSubdir() === GameSubdirMode.ALWAYS
-    )) {
+    ) {
       output = path.join(game.getName(), output);
     }
 

test/modules/candidatePreferer.test.ts

@@ -60,7 +60,7 @@ async function expectPreferredCandidates(
 }
 
 function arrayCoerce<T>(val: T | T[] | undefined): T[] {
-  if (!val) {
+  if (val === undefined) {
     return [];
   }
   return Array.isArray(val) ? val : [val];

test/modules/datFilter.test.ts

@@ -22,7 +22,7 @@ async function expectFilteredDAT(
 }
 
 function arrayCoerce<T>(val: T | T[] | undefined): T[] {
-  if (!val) {
+  if (val === undefined) {
     return [];
   }
   return Array.isArray(val) ? val : [val];