Refactor to 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.
This commit is contained in:
@@ -3,21 +3,22 @@ import { Application } from '@/domain/Application';
|
||||
import { OperatingSystem } from '@/domain/OperatingSystem';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { ProjectInformationStub } from '@tests/unit/shared/Stubs/ProjectInformationStub';
|
||||
import { getAbsentObjectTestCases, getAbsentCollectionTestCases, itEachAbsentObjectValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
import { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
import { getAbsentCollectionTestCases } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
|
||||
describe('Application', () => {
|
||||
describe('getCollection', () => {
|
||||
it('returns undefined if not found', () => {
|
||||
it('throws if not found', () => {
|
||||
// arrange
|
||||
const expected = undefined;
|
||||
const missingOs = OperatingSystem.Android;
|
||||
const expectedError = `Operating system "${OperatingSystem[missingOs]}" is not defined in application`;
|
||||
const info = new ProjectInformationStub();
|
||||
const collections = [new CategoryCollectionStub().withOs(OperatingSystem.Windows)];
|
||||
// act
|
||||
const sut = new Application(info, collections);
|
||||
const actual = sut.getCollection(OperatingSystem.Android);
|
||||
const act = () => sut.getCollection(missingOs);
|
||||
// assert
|
||||
expect(actual).to.equals(expected);
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
it('returns expected when multiple collections exist', () => {
|
||||
// arrange
|
||||
@@ -34,18 +35,6 @@ describe('Application', () => {
|
||||
});
|
||||
describe('ctor', () => {
|
||||
describe('info', () => {
|
||||
describe('throws if missing', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing project information';
|
||||
const info = absentValue;
|
||||
const collections = [new CategoryCollectionStub()];
|
||||
// act
|
||||
const act = () => new Application(info, collections);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
it('sets as expected', () => {
|
||||
// arrange
|
||||
const expected = new ProjectInformationStub();
|
||||
@@ -60,20 +49,20 @@ describe('Application', () => {
|
||||
describe('throws on invalid value', () => {
|
||||
// arrange
|
||||
const testCases: readonly {
|
||||
name: string,
|
||||
expectedError: string,
|
||||
value: readonly ICategoryCollection[],
|
||||
readonly name: string,
|
||||
readonly expectedError: string,
|
||||
readonly value: readonly ICategoryCollection[],
|
||||
}[] = [
|
||||
...getAbsentCollectionTestCases<ICategoryCollection>().map((testCase) => ({
|
||||
name: testCase.valueName,
|
||||
...getAbsentCollectionTestCases<ICategoryCollection>(
|
||||
{
|
||||
excludeUndefined: true,
|
||||
excludeNull: true,
|
||||
},
|
||||
).map((testCase) => ({
|
||||
name: `empty collection: ${testCase.valueName}`,
|
||||
expectedError: 'missing collections',
|
||||
value: testCase.absentValue,
|
||||
})),
|
||||
...getAbsentObjectTestCases().map((testCase) => ({
|
||||
name: `${testCase.valueName} value in list`,
|
||||
expectedError: 'missing collection in the list',
|
||||
value: [new CategoryCollectionStub(), testCase.absentValue],
|
||||
})),
|
||||
{
|
||||
name: 'two collections with same OS',
|
||||
expectedError: 'multiple collections with same os: windows',
|
||||
|
||||
@@ -15,7 +15,7 @@ describe('Category', () => {
|
||||
const construct = () => new Category(5, name, [], [new CategoryStub(5)], []);
|
||||
// assert
|
||||
expect(construct).to.throw(expectedError);
|
||||
});
|
||||
}, { excludeNull: true, excludeUndefined: true });
|
||||
});
|
||||
it('throws when has no children', () => {
|
||||
const expectedError = 'A category must have at least one sub-category or script';
|
||||
|
||||
@@ -9,7 +9,6 @@ import { CategoryCollection } from '@/domain/CategoryCollection';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { EnumRangeTestRunner } from '@tests/unit/application/Common/EnumRangeTestRunner';
|
||||
import { itEachAbsentObjectValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
|
||||
describe('CategoryCollection', () => {
|
||||
describe('getScriptsByLevel', () => {
|
||||
@@ -81,7 +80,6 @@ describe('CategoryCollection', () => {
|
||||
sut.getScriptsByLevel(level);
|
||||
})
|
||||
// assert
|
||||
.testAbsentValueThrows()
|
||||
.testOutOfRangeThrows()
|
||||
.testValidValueDoesNotThrow(RecommendationLevel.Standard);
|
||||
});
|
||||
@@ -214,8 +212,7 @@ describe('CategoryCollection', () => {
|
||||
.construct();
|
||||
// assert
|
||||
new EnumRangeTestRunner(act)
|
||||
.testOutOfRangeThrows()
|
||||
.testAbsentValueThrows();
|
||||
.testOutOfRangeThrows();
|
||||
});
|
||||
});
|
||||
describe('scriptingDefinition', () => {
|
||||
@@ -229,20 +226,60 @@ describe('CategoryCollection', () => {
|
||||
// assert
|
||||
expect(sut.scripting).to.deep.equal(expected);
|
||||
});
|
||||
describe('cannot construct without initial script', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing scripting definition';
|
||||
const scriptingDefinition = absentValue;
|
||||
// act
|
||||
function construct() {
|
||||
return new CategoryCollectionBuilder()
|
||||
.withScripting(scriptingDefinition)
|
||||
.construct();
|
||||
}
|
||||
// assert
|
||||
expect(construct).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
describe('getCategory', () => {
|
||||
it('throws if category is not found', () => {
|
||||
// arrange
|
||||
const categoryId = 123;
|
||||
const expectedError = `Missing category with ID: "${categoryId}"`;
|
||||
const collection = new CategoryCollectionBuilder()
|
||||
.withActions([new CategoryStub(456).withMandatoryScripts()])
|
||||
.construct();
|
||||
// act
|
||||
const act = () => collection.getCategory(categoryId);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
it('finds correct category', () => {
|
||||
// arrange
|
||||
const categoryId = 123;
|
||||
const expectedCategory = new CategoryStub(categoryId).withMandatoryScripts();
|
||||
const collection = new CategoryCollectionBuilder()
|
||||
.withActions([expectedCategory])
|
||||
.construct();
|
||||
// act
|
||||
const actualCategory = collection.getCategory(categoryId);
|
||||
// assert
|
||||
expect(actualCategory).to.equal(expectedCategory);
|
||||
});
|
||||
});
|
||||
describe('getScript', () => {
|
||||
it('throws if script is not found', () => {
|
||||
// arrange
|
||||
const scriptId = 'missingScript';
|
||||
const expectedError = `missing script: ${scriptId}`;
|
||||
const collection = new CategoryCollectionBuilder()
|
||||
.withActions([new CategoryStub(456).withMandatoryScripts()])
|
||||
.construct();
|
||||
// act
|
||||
const act = () => collection.getScript(scriptId);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
it('finds correct script', () => {
|
||||
// arrange
|
||||
const scriptId = 'existingScript';
|
||||
const expectedScript = new ScriptStub(scriptId);
|
||||
const parentCategory = new CategoryStub(123)
|
||||
.withMandatoryScripts()
|
||||
.withScript(expectedScript);
|
||||
const collection = new CategoryCollectionBuilder()
|
||||
.withActions([parentCategory])
|
||||
.construct();
|
||||
// act
|
||||
const actualScript = collection.getScript(scriptId);
|
||||
// assert
|
||||
expect(actualScript).to.equal(expectedScript);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -260,30 +297,27 @@ class CategoryCollectionBuilder {
|
||||
private os = OperatingSystem.Windows;
|
||||
|
||||
private actions: readonly ICategory[] = [
|
||||
new CategoryStub(1).withScripts(
|
||||
new ScriptStub('S1').withLevel(RecommendationLevel.Standard),
|
||||
new ScriptStub('S2').withLevel(RecommendationLevel.Strict),
|
||||
),
|
||||
new CategoryStub(1).withMandatoryScripts(),
|
||||
];
|
||||
|
||||
private script: IScriptingDefinition = getValidScriptingDefinition();
|
||||
private scriptingDefinition: IScriptingDefinition = getValidScriptingDefinition();
|
||||
|
||||
public withOs(os: OperatingSystem): CategoryCollectionBuilder {
|
||||
public withOs(os: OperatingSystem): this {
|
||||
this.os = os;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withActions(actions: readonly ICategory[]): CategoryCollectionBuilder {
|
||||
public withActions(actions: readonly ICategory[]): this {
|
||||
this.actions = actions;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withScripting(script: IScriptingDefinition): CategoryCollectionBuilder {
|
||||
this.script = script;
|
||||
public withScripting(scriptingDefinition: IScriptingDefinition): this {
|
||||
this.scriptingDefinition = scriptingDefinition;
|
||||
return this;
|
||||
}
|
||||
|
||||
public construct(): CategoryCollection {
|
||||
return new CategoryCollection(this.os, this.actions, this.script);
|
||||
return new CategoryCollection(this.os, this.actions, this.scriptingDefinition);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -159,7 +159,7 @@ describe('ProjectInformation', () => {
|
||||
expect(actual).to.equal(expected);
|
||||
});
|
||||
}
|
||||
it('should throw an error when provided with an invalid operating system', () => {
|
||||
describe('should throw an error when provided with an invalid operating system', () => {
|
||||
// arrange
|
||||
const sut = new ProjectInformationBuilder()
|
||||
.build();
|
||||
@@ -168,7 +168,6 @@ describe('ProjectInformation', () => {
|
||||
// assert
|
||||
new EnumRangeTestRunner(act)
|
||||
.testOutOfRangeThrows()
|
||||
.testAbsentValueThrows()
|
||||
.testInvalidValueThrows(OperatingSystem.KaiOS, `Unsupported os: ${OperatingSystem[OperatingSystem.KaiOS]}`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,7 +4,6 @@ import { Script } from '@/domain/Script';
|
||||
import { RecommendationLevel } from '@/domain/RecommendationLevel';
|
||||
import { IScriptCode } from '@/domain/IScriptCode';
|
||||
import { ScriptCodeStub } from '@tests/unit/shared/Stubs/ScriptCodeStub';
|
||||
import { itEachAbsentObjectValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
|
||||
describe('Script', () => {
|
||||
describe('ctor', () => {
|
||||
@@ -20,19 +19,6 @@ describe('Script', () => {
|
||||
// assert
|
||||
expect(actual).to.deep.equal(expected);
|
||||
});
|
||||
describe('throws when missing', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing code';
|
||||
const code: IScriptCode = absentValue;
|
||||
// act
|
||||
const construct = () => new ScriptBuilder()
|
||||
.withCode(code)
|
||||
.build();
|
||||
// assert
|
||||
expect(construct).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
});
|
||||
describe('canRevert', () => {
|
||||
it('returns false without revert code', () => {
|
||||
@@ -112,34 +98,34 @@ class ScriptBuilder {
|
||||
|
||||
private code: IScriptCode = new ScriptCodeStub();
|
||||
|
||||
private level = RecommendationLevel.Standard;
|
||||
private level? = RecommendationLevel.Standard;
|
||||
|
||||
private docs: readonly string[] = undefined;
|
||||
private docs: readonly string[] = [];
|
||||
|
||||
public withCodes(code: string, revertCode = ''): ScriptBuilder {
|
||||
public withCodes(code: string, revertCode = ''): this {
|
||||
this.code = new ScriptCodeStub()
|
||||
.withExecute(code)
|
||||
.withRevert(revertCode);
|
||||
return this;
|
||||
}
|
||||
|
||||
public withCode(code: IScriptCode): ScriptBuilder {
|
||||
public withCode(code: IScriptCode): this {
|
||||
this.code = code;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withName(name: string): ScriptBuilder {
|
||||
public withName(name: string): this {
|
||||
this.name = name;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withRecommendationLevel(level: RecommendationLevel): ScriptBuilder {
|
||||
public withRecommendationLevel(level: RecommendationLevel | undefined): this {
|
||||
this.level = level;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withDocs(urls: readonly string[]): ScriptBuilder {
|
||||
this.docs = urls;
|
||||
public withDocs(docs: readonly string[]): this {
|
||||
this.docs = docs;
|
||||
return this;
|
||||
}
|
||||
|
||||
|
||||
@@ -6,26 +6,34 @@ describe('ScriptCode', () => {
|
||||
describe('code', () => {
|
||||
describe('throws with invalid code', () => {
|
||||
// arrange
|
||||
const testCases = [
|
||||
const testScenarios: ReadonlyArray<{
|
||||
readonly description: string;
|
||||
readonly code: {
|
||||
readonly execute: string;
|
||||
readonly revert?: string;
|
||||
},
|
||||
readonly expectedError: string;
|
||||
}> = [
|
||||
{
|
||||
name: 'throws when "execute" and "revert" are same',
|
||||
description: 'throws when "execute" and "revert" are same',
|
||||
code: {
|
||||
execute: 'same',
|
||||
revert: 'same',
|
||||
},
|
||||
expectedError: '(revert): Code itself and its reverting code cannot be the same',
|
||||
},
|
||||
...getAbsentStringTestCases().map((testCase) => ({
|
||||
name: `cannot construct with ${testCase.valueName} "execute"`,
|
||||
code: {
|
||||
execute: testCase.absentValue,
|
||||
revert: 'code',
|
||||
},
|
||||
expectedError: 'missing code',
|
||||
})),
|
||||
...getAbsentStringTestCases({ excludeNull: true, excludeUndefined: true })
|
||||
.map((testCase) => ({
|
||||
description: `cannot construct with ${testCase.valueName} "execute"`,
|
||||
code: {
|
||||
execute: testCase.absentValue,
|
||||
revert: 'code',
|
||||
},
|
||||
expectedError: 'missing code',
|
||||
})),
|
||||
];
|
||||
for (const testCase of testCases) {
|
||||
it(testCase.name, () => {
|
||||
for (const testCase of testScenarios) {
|
||||
it(testCase.description, () => {
|
||||
// act
|
||||
const act = () => new ScriptCodeBuilder()
|
||||
.withExecute(testCase.code.execute)
|
||||
@@ -38,21 +46,27 @@ describe('ScriptCode', () => {
|
||||
});
|
||||
describe('sets as expected with valid "execute" or "revert"', () => {
|
||||
// arrange
|
||||
const testCases = [
|
||||
const testScenarios: ReadonlyArray<{
|
||||
readonly description: string;
|
||||
readonly code: string;
|
||||
readonly revertCode: string | undefined;
|
||||
}> = [
|
||||
{
|
||||
testName: 'code and revert code is given',
|
||||
description: 'code and revert code is given',
|
||||
code: 'valid code',
|
||||
revertCode: 'valid revert-code',
|
||||
},
|
||||
{
|
||||
testName: 'only code is given but not revert code',
|
||||
code: 'valid code',
|
||||
revertCode: undefined,
|
||||
},
|
||||
...getAbsentStringTestCases({ excludeNull: true })
|
||||
.map((testCase) => ({
|
||||
description: `only code is given but not revert code (given ${testCase.valueName})`,
|
||||
code: 'valid code',
|
||||
revertCode: testCase.absentValue,
|
||||
expectedError: 'missing code',
|
||||
})),
|
||||
];
|
||||
// assert
|
||||
for (const testCase of testCases) {
|
||||
it(testCase.testName, () => {
|
||||
for (const testCase of testScenarios) {
|
||||
it(testCase.description, () => {
|
||||
// act
|
||||
const sut = new ScriptCodeBuilder()
|
||||
.withExecute(testCase.code)
|
||||
@@ -70,14 +84,14 @@ describe('ScriptCode', () => {
|
||||
class ScriptCodeBuilder {
|
||||
public execute = 'default-execute-code';
|
||||
|
||||
public revert = '';
|
||||
public revert: string | undefined = '';
|
||||
|
||||
public withExecute(execute: string) {
|
||||
this.execute = execute;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withRevert(revert: string) {
|
||||
public withRevert(revert: string | undefined) {
|
||||
this.revert = revert;
|
||||
return this;
|
||||
}
|
||||
|
||||
@@ -68,14 +68,13 @@ describe('ScriptingDefinition', () => {
|
||||
itEachAbsentStringValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing start code';
|
||||
const undefinedValue = absentValue;
|
||||
// act
|
||||
const act = () => new ScriptingDefinitionBuilder()
|
||||
.withStartCode(undefinedValue)
|
||||
.withStartCode(absentValue)
|
||||
.build();
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
}, { excludeNull: true, excludeUndefined: true });
|
||||
});
|
||||
});
|
||||
describe('endCode', () => {
|
||||
@@ -89,18 +88,17 @@ describe('ScriptingDefinition', () => {
|
||||
// assert
|
||||
expect(sut.endCode).to.equal(expected);
|
||||
});
|
||||
describe('throws when undefined', () => {
|
||||
describe('throws when absent', () => {
|
||||
itEachAbsentStringValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing end code';
|
||||
const undefinedValue = absentValue;
|
||||
// act
|
||||
const act = () => new ScriptingDefinitionBuilder()
|
||||
.withEndCode(undefinedValue)
|
||||
.withEndCode(absentValue)
|
||||
.build();
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
}, { excludeNull: true, excludeUndefined: true });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -108,21 +106,21 @@ describe('ScriptingDefinition', () => {
|
||||
class ScriptingDefinitionBuilder {
|
||||
private language = ScriptingLanguage.shellscript;
|
||||
|
||||
private startCode = 'REM start-code';
|
||||
private startCode = `# [${ScriptingDefinitionBuilder.name}]: start-code`;
|
||||
|
||||
private endCode = 'REM end-code';
|
||||
private endCode = `# [${ScriptingDefinitionBuilder.name}]: end-code`;
|
||||
|
||||
public withLanguage(language: ScriptingLanguage): ScriptingDefinitionBuilder {
|
||||
public withLanguage(language: ScriptingLanguage): this {
|
||||
this.language = language;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withStartCode(startCode: string): ScriptingDefinitionBuilder {
|
||||
public withStartCode(startCode: string): this {
|
||||
this.startCode = startCode;
|
||||
return this;
|
||||
}
|
||||
|
||||
public withEndCode(endCode: string): ScriptingDefinitionBuilder {
|
||||
public withEndCode(endCode: string): this {
|
||||
this.endCode = endCode;
|
||||
return this;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { Version } from '@/domain/Version';
|
||||
import { itEachAbsentStringValue } from '../shared/TestCases/AbsentTests';
|
||||
|
||||
describe('Version', () => {
|
||||
describe('invalid versions', () => {
|
||||
@@ -19,20 +20,15 @@ describe('Version', () => {
|
||||
}
|
||||
});
|
||||
describe('throws with empty string', () => {
|
||||
// arrange
|
||||
const expectedError = 'empty version';
|
||||
const testCases = [
|
||||
{ name: 'empty', value: '' },
|
||||
{ name: 'undefined', value: undefined },
|
||||
];
|
||||
for (const testCase of testCases) {
|
||||
it(`given ${testCase.name}`, () => {
|
||||
// act
|
||||
const act = () => new Version(testCase.value);
|
||||
//
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
}
|
||||
itEachAbsentStringValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'empty version';
|
||||
const value = absentValue;
|
||||
// act
|
||||
const act = () => new Version(value);
|
||||
//
|
||||
expect(act).to.throw(expectedError);
|
||||
}, { excludeNull: true, excludeUndefined: true });
|
||||
});
|
||||
});
|
||||
describe('valid versions', () => {
|
||||
|
||||
Reference in New Issue
Block a user