Refactor to improve iterations

- Use function abstractions (such as map, reduce, filter etc.) over
  for-of loops to gain benefits of having less side effects and easier
  readability.
- Enable `downLevelIterations` for writing modern code with lazy evaluation.
- Refactor for of loops to named abstractions to clearly express their
  intentions without needing to analyse the loop itself.
- Add missing cases for changes that had no tests.
This commit is contained in:
undergroundwires
2022-01-04 21:45:22 +01:00
parent bd23faa28f
commit 31f70913a2
35 changed files with 342 additions and 343 deletions

View File

@@ -25,7 +25,7 @@ function testExpectedInstanceTypes<T>(
) {
describe('create returns expected instances', () => {
// arrange
for (const language of Array.from(expectedTypes.keys())) {
for (const language of expectedTypes.keys()) {
it(ScriptingLanguage[language], () => {
// act
const expected = expectedTypes.get(language);

View File

@@ -197,7 +197,7 @@ class UserScriptGeneratorMock implements IUserScriptGenerator {
selectedScripts: readonly SelectedScript[],
scriptingDefinition: IScriptingDefinition,
): IUserScript {
for (const [parameters, result] of Array.from(this.prePlanned)) {
for (const [parameters, result] of this.prePlanned) {
if (selectedScripts === parameters.scripts
&& scriptingDefinition === parameters.definition) {
return result;

View File

@@ -13,16 +13,23 @@ describe('CodeChangedEvent', () => {
it('throws when code position is out of range', () => {
// arrange
const code = 'singleline code';
const nonExistingLine1 = 2;
const nonExistingLine2 = 31;
const newScripts = new Map<SelectedScript, ICodePosition>([
[new SelectedScriptStub('1'), new CodePosition(0, 2 /* nonexisting line */)],
[new SelectedScriptStub('1'), new CodePosition(0, nonExistingLine1)],
[new SelectedScriptStub('2'), new CodePosition(0, nonExistingLine2)],
]);
// act
const act = () => new CodeChangedEventBuilder()
.withCode(code)
.withNewScripts(newScripts)
.build();
let errorText = '';
try {
new CodeChangedEventBuilder()
.withCode(code)
.withNewScripts(newScripts)
.build();
} catch (error) { errorText = error.message; }
// assert
expect(act).to.throw();
expect(errorText).to.include(nonExistingLine1);
expect(errorText).to.include(nonExistingLine2);
});
describe('does not throw with valid code position', () => {
// arrange

View File

@@ -151,6 +151,14 @@ describe('ExpressionsCompiler', () => {
.withArgument('parameter2', 'value'),
expectedError: 'parameter value(s) not provided for: "parameter1", "parameter3" but used in code',
},
{
name: 'parameter names are not repeated in error message',
expressions: [
new ExpressionStub().withParameterNames(['parameter1', 'parameter1', 'parameter2', 'parameter2'], false),
],
args: new FunctionCallArgumentCollectionStub(),
expectedError: 'parameter value(s) not provided for: "parameter1", "parameter2" but used in code',
},
];
for (const testCase of testCases) {
it(testCase.name, () => {

View File

@@ -31,6 +31,31 @@ describe('RegexParser', () => {
});
}
});
it('throws when position is invalid', () => {
// arrange
const regexMatchingEmpty = /^/gm; /* expressions cannot be empty */
const code = 'unimportant';
const expectedErrorParts = [
`[${RegexParserConcrete.constructor.name}]`,
'invalid script position',
`Regex: ${regexMatchingEmpty}`,
`Code: ${code}`,
];
const sut = new RegexParserConcrete(regexMatchingEmpty);
// act
let error: string;
try {
sut.findExpressions(code);
} catch (err) {
error = err.message;
}
// assert
expect(
expectedErrorParts.every((part) => error.includes(part)),
`Expected parts: ${expectedErrorParts.join(', ')}`
+ `Actual error: ${error}`,
);
});
describe('matches regex as expected', () => {
// arrange
const testCases = [

View File

@@ -70,7 +70,7 @@ describe('FunctionCallCompiler', () => {
},
{
name: 'provided: an unexpected parameter, when: none required',
functionParameters: undefined,
functionParameters: [],
callParameters: ['unexpected-call-parameter'],
expectedError:
`Function "${functionName}" has unexpected parameter(s) provided: "unexpected-call-parameter"`
@@ -90,10 +90,10 @@ describe('FunctionCallCompiler', () => {
const func = new SharedFunctionStub(FunctionBodyType.Code)
.withName('test-function-name')
.withParameterNames(...testCase.functionParameters);
let params: FunctionCallParametersData = {};
for (const parameter of testCase.callParameters) {
params = { ...params, [parameter]: 'defined-parameter-value ' };
}
const params = testCase.callParameters
.reduce((result, parameter) => {
return { ...result, [parameter]: 'defined-parameter-value ' };
}, {} as FunctionCallParametersData);
const call = new FunctionCallStub()
.withFunctionName(func.name)
.withArguments(params);

View File

@@ -49,15 +49,11 @@ async function findBadLineNumbers(fileContent: string): Promise<number[]> {
function findLineNumbersEndingWith(content: string, ending: string): number[] {
sanityCheck(content, ending);
const lines = content.split(/\r\n|\r|\n/);
const results = new Array<number>();
for (let i = 0; i < lines.length; i++) {
const line = lines[i];
if (line.trim().endsWith(ending)) {
results.push((i + 1 /* first line is 1 not 0 */));
}
}
return results;
return content
.split(/\r\n|\r|\n/)
.map((line, index) => ({ text: line, index }))
.filter((line) => line.text.trim().endsWith(ending))
.map((line) => line.index + 1 /* first line is 1, not 0 */);
}
function sanityCheck(content: string, ending: string): void {

View File

@@ -121,24 +121,45 @@ describe('CategoryCollection', () => {
expect(construct).to.throw('must consist of at least one script');
});
describe('cannot construct without any recommended scripts', () => {
// arrange
const recommendationLevels = getEnumValues(RecommendationLevel);
for (const missingLevel of recommendationLevels) {
it(`when "${RecommendationLevel[missingLevel]}" is missing`, () => {
const expectedError = `none of the scripts are recommended as ${RecommendationLevel[missingLevel]}`;
const otherLevels = recommendationLevels.filter((level) => level !== missingLevel);
const categories = otherLevels.map(
(level, index) => new CategoryStub(index)
.withScript(new ScriptStub(`Script${index}`).withLevel(level)),
);
// act
const construct = () => new CategoryCollectionBuilder()
.withActions(categories)
.construct();
// assert
expect(construct).to.throw(expectedError);
});
}
describe('single missing', () => {
// arrange
const recommendationLevels = getEnumValues(RecommendationLevel);
for (const missingLevel of recommendationLevels) {
it(`when "${RecommendationLevel[missingLevel]}" is missing`, () => {
const expectedError = `none of the scripts are recommended as "${RecommendationLevel[missingLevel]}".`;
const otherLevels = recommendationLevels.filter((level) => level !== missingLevel);
const categories = otherLevels.map(
(level, index) => new CategoryStub(index)
.withScript(
new ScriptStub(`Script${index}`).withLevel(level),
),
);
// act
const construct = () => new CategoryCollectionBuilder()
.withActions(categories)
.construct();
// assert
expect(construct).to.throw(expectedError);
});
}
});
it('multiple are missing', () => {
// arrange
const expectedError = 'none of the scripts are recommended as '
+ `"${RecommendationLevel[RecommendationLevel.Standard]}, "${RecommendationLevel[RecommendationLevel.Strict]}".`;
const categories = [
new CategoryStub(0)
.withScript(
new ScriptStub(`Script${0}`).withLevel(undefined),
),
];
// act
const construct = () => new CategoryCollectionBuilder()
.withActions(categories)
.construct();
// assert
expect(construct).to.throw(expectedError);
});
});
});
describe('totalScripts', () => {

View File

@@ -61,25 +61,24 @@ describe('ScriptCode', () => {
},
];
// act
const actions = [];
for (const testCase of testCases) {
actions.push(...[
{
act: () => new ScriptCodeBuilder()
.withExecute(testCase.code)
.build(),
testName: `execute: ${testCase.testName}`,
expectedMessage: testCase.expectedMessage,
},
{
act: () => new ScriptCodeBuilder()
.withRevert(testCase.code)
.build(),
testName: `revert: ${testCase.testName}`,
expectedMessage: `(revert): ${testCase.expectedMessage}`,
},
]);
}
const actions = testCases.flatMap((testCase) => ([
{
act: () => new ScriptCodeBuilder()
.withExecute(testCase.code)
.build(),
testName: `execute: ${testCase.testName}`,
expectedMessage: testCase.expectedMessage,
code: testCase.code,
},
{
act: () => new ScriptCodeBuilder()
.withRevert(testCase.code)
.build(),
testName: `revert: ${testCase.testName}`,
expectedMessage: `(revert): ${testCase.expectedMessage}`,
code: testCase.code,
},
]));
// assert
for (const action of actions) {
it(action.testName, () => {
@@ -115,27 +114,24 @@ describe('ScriptCode', () => {
},
];
// act
const actions = [];
for (const testCase of testCases) {
actions.push(...[
{
testName: `execute: ${testCase.testName}`,
act: () => new ScriptCodeBuilder()
.withSyntax(syntax)
.withExecute(testCase.code)
.build(),
expect: (sut: IScriptCode) => sut.execute === testCase.code,
},
{
testName: `revert: ${testCase.testName}`,
act: () => new ScriptCodeBuilder()
.withSyntax(syntax)
.withRevert(testCase.code)
.build(),
expect: (sut: IScriptCode) => sut.revert === testCase.code,
},
]);
}
const actions = testCases.flatMap((testCase) => ([
{
testName: `execute: ${testCase.testName}`,
act: () => new ScriptCodeBuilder()
.withSyntax(syntax)
.withExecute(testCase.code)
.build(),
expect: (sut: IScriptCode) => sut.execute === testCase.code,
},
{
testName: `revert: ${testCase.testName}`,
act: () => new ScriptCodeBuilder()
.withSyntax(syntax)
.withRevert(testCase.code)
.build(),
expect: (sut: IScriptCode) => sut.revert === testCase.code,
},
]));
// assert
for (const action of actions) {
it(action.testName, () => {

View File

@@ -39,7 +39,7 @@ describe('ScriptingDefinition', () => {
[ScriptingLanguage.batchfile, 'bat'],
[ScriptingLanguage.shellscript, 'sh'],
]);
Array.from(testCases.entries()).forEach((test) => {
for (const test of testCases.entries()) {
const language = test[0];
const expectedExtension = test[1];
it(`${ScriptingLanguage[language]} has ${expectedExtension}`, () => {
@@ -50,7 +50,7 @@ describe('ScriptingDefinition', () => {
// assert
expect(sut.fileExtension, expectedExtension);
});
});
}
});
});
describe('startCode', () => {

View File

@@ -52,14 +52,12 @@ class SchedulerMock {
public tickNext(ms: number) {
const newTime = this.currentTime + ms;
let newActions = this.scheduledActions;
for (const action of this.scheduledActions) {
if (newTime >= action.time) {
newActions = newActions.filter((a) => a !== action);
action.action();
}
const dueActions = this.scheduledActions
.filter((action) => newTime >= action.time);
for (const action of dueActions) {
action.action();
}
this.scheduledActions = newActions;
this.scheduledActions = this.scheduledActions.filter((action) => !dueActions.includes(action));
}
}

View File

@@ -63,7 +63,6 @@ describe('ScriptNodeParser', () => {
expectSameScript(nodes[2], scripts[2]);
});
});
it('parseAllCategories parses as expected', () => {
// arrange
const collection = new CategoryCollectionStub()

View File

@@ -61,46 +61,27 @@ export class CategoryCollectionStub implements ICategoryCollection {
}
public getAllScripts(): ReadonlyArray<IScript> {
const scripts = [];
for (const category of this.actions) {
const categoryScripts = getScriptsRecursively(category);
scripts.push(...categoryScripts);
}
return scripts;
return this.actions.flatMap((category) => getScriptsRecursively(category));
}
public getAllCategories(): ReadonlyArray<ICategory> {
const categories = [];
categories.push(...this.actions);
for (const category of this.actions) {
const subCategories = getSubCategoriesRecursively(category);
categories.push(...subCategories);
}
return categories;
return this.actions.flatMap(
(category) => [category, ...getSubCategoriesRecursively(category)],
);
}
}
function getSubCategoriesRecursively(category: ICategory): ReadonlyArray<ICategory> {
const subCategories = [];
if (category.subCategories) {
for (const subCategory of category.subCategories) {
subCategories.push(subCategory);
subCategories.push(...getSubCategoriesRecursively(subCategory));
}
}
return subCategories;
return (category.subCategories || []).flatMap(
(subCategory) => [subCategory, ...getSubCategoriesRecursively(subCategory)],
);
}
function getScriptsRecursively(category: ICategory): ReadonlyArray<IScript> {
const categoryScripts = [];
if (category.scripts) {
categoryScripts.push(...category.scripts);
}
if (category.subCategories) {
for (const subCategory of category.subCategories) {
const subCategoryScripts = getScriptsRecursively(subCategory);
categoryScripts.push(...subCategoryScripts);
}
}
return categoryScripts;
return [
...(category.scripts || []),
...(category.subCategories || []).flatMap(
(subCategory) => getScriptsRecursively(subCategory),
),
];
}

View File

@@ -27,10 +27,9 @@ export class CategoryStub extends BaseEntity<number> implements ICategory {
}
public withScriptIds(...scriptIds: string[]): CategoryStub {
for (const scriptId of scriptIds) {
this.withScript(new ScriptStub(scriptId));
}
return this;
return this.withScripts(
...scriptIds.map((id) => new ScriptStub(id)),
);
}
public withScripts(...scripts: IScript[]): CategoryStub {

View File

@@ -58,12 +58,9 @@ function deepEqual(
if (!scrambledEqual(expectedParameterNames, actualParameterNames)) {
return false;
}
for (const parameterName of expectedParameterNames) {
return expectedParameterNames.every((parameterName) => {
const expectedValue = expected.getArgument(parameterName).argumentValue;
const actualValue = actual.getArgument(parameterName).argumentValue;
if (expectedValue !== actualValue) {
return false;
}
}
return true;
return expectedValue === actualValue;
});
}

View File

@@ -14,9 +14,8 @@ export class FunctionCallArgumentCollectionStub implements IFunctionCallArgument
}
public withArguments(args: { readonly [index: string]: string }) {
for (const parameterName of Object.keys(args)) {
const parameterValue = args[parameterName];
this.withArgument(parameterName, parameterValue);
for (const [name, value] of Object.entries(args)) {
this.withArgument(name, value);
}
return this;
}

View File

@@ -13,7 +13,7 @@ export class ScriptStub extends BaseEntity<string> implements IScript {
public readonly documentationUrls = new Array<string>();
public level = RecommendationLevel.Standard;
public level? = RecommendationLevel.Standard;
constructor(public readonly id: string) {
super(id);
@@ -23,7 +23,7 @@ export class ScriptStub extends BaseEntity<string> implements IScript {
return Boolean(this.code.revert);
}
public withLevel(value: RecommendationLevel): ScriptStub {
public withLevel(value?: RecommendationLevel): ScriptStub {
this.level = value;
return this;
}