Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: ESLint strict-boolean-expressions #759

Merged
merged 3 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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 *****
Expand Down
1 change: 1 addition & 0 deletions src/modules/argumentsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/modules/candidateGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/modules/datScanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/polyfill/arrayPoly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/types/datStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/types/dats/game.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? '';
Expand Down
6 changes: 3 additions & 3 deletions src/types/dats/logiqx/logiqxDat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/types/dats/mame/machine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/types/dats/mame/mameDat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/types/files/archives/tar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down
4 changes: 2 additions & 2 deletions src/types/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}
Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions src/types/outputFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion test/modules/candidatePreferer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion test/modules/datFilter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Loading