Relax and improve code validation
Rework code validation to be bound to a context and not context-independent. It means that the generated code is validated based on different phases during the compilation. This is done by moving validation from `ScriptCode` constructor to a different callable function. It removes duplicate detection for function calls once a call is fully compiled, but still checks for duplicates inside each function body that has inline code. This allows for having duplicates in final scripts (thus relaxing the duplicate detection), e.g., when multiple calls to the same function is made. It fixes non-duplicates (when using common syntax) being misrepresented as duplicate lines. It improves the output of errors, such as printing valid lines, to give more context. This improvement also fixes empty line validation not showing the right empty lines in the error output. Empty line validation shows tabs and whitespaces more clearly. Finally, it adds more tests including tests for existing logic, such as singleton factories.
This commit is contained in:
undergroundwires
@@ -0,0 +1,169 @@
|
||||
import 'mocha';
|
||||
import { expect } from 'chai';
|
||||
import { CodeValidator } from '@/application/Parser/Script/Validation/CodeValidator';
|
||||
import { CodeValidationRuleStub } from '@tests/unit/shared/Stubs/CodeValidationRuleStub';
|
||||
import { itEachAbsentCollectionValue, itEachAbsentStringValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
import { itIsSingleton } from '@tests/unit/shared/TestCases/SingletonTests';
|
||||
import { ICodeLine } from '@/application/Parser/Script/Validation/ICodeLine';
|
||||
import { IInvalidCodeLine } from '@/application/Parser/Script/Validation/ICodeValidationRule';
|
||||
|
||||
describe('CodeValidator', () => {
|
||||
describe('instance', () => {
|
||||
itIsSingleton({
|
||||
getter: () => CodeValidator.instance,
|
||||
expectedType: CodeValidator,
|
||||
});
|
||||
});
|
||||
describe('throwIfInvalid', () => {
|
||||
describe('does not throw if code is absent', () => {
|
||||
itEachAbsentStringValue((absentValue) => {
|
||||
// arrange
|
||||
const code = absentValue;
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
const act = () => sut.throwIfInvalid(code, [new CodeValidationRuleStub()]);
|
||||
// assert
|
||||
expect(act).to.not.throw();
|
||||
});
|
||||
});
|
||||
describe('throws if rules are empty', () => {
|
||||
itEachAbsentCollectionValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing rules';
|
||||
const rules = absentValue;
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
const act = () => sut.throwIfInvalid('code', rules);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
describe('splits lines as expected', () => {
|
||||
it('supports all line separators', () => {
|
||||
// arrange
|
||||
const expectedLineTexts = ['line1', 'line2', 'line3', 'line4'];
|
||||
const code = 'line1\r\nline2\rline3\nline4';
|
||||
const spy = new CodeValidationRuleStub();
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
sut.throwIfInvalid(code, [spy]);
|
||||
// expect
|
||||
expect(spy.receivedLines).has.lengthOf(1);
|
||||
const actualLineTexts = spy.receivedLines[0].map((line) => line.text);
|
||||
expect(actualLineTexts).to.deep.equal(expectedLineTexts);
|
||||
});
|
||||
it('uses 1-indexed line numbering', () => {
|
||||
// arrange
|
||||
const expectedIndexes = [1, 2, 3];
|
||||
const code = ['line1', 'line2', 'line3'].join('\n');
|
||||
const spy = new CodeValidationRuleStub();
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
sut.throwIfInvalid(code, [spy]);
|
||||
// expect
|
||||
expect(spy.receivedLines).has.lengthOf(1);
|
||||
const actualLineIndexes = spy.receivedLines[0].map((line) => line.index);
|
||||
expect(actualLineIndexes).to.deep.equal(expectedIndexes);
|
||||
});
|
||||
it('matches texts with indexes as expected', () => {
|
||||
// arrange
|
||||
const expected: readonly ICodeLine[] = [
|
||||
{ index: 1, text: 'first' },
|
||||
{ index: 2, text: 'second' },
|
||||
];
|
||||
const code = expected.map((line) => line.text).join('\n');
|
||||
const spy = new CodeValidationRuleStub();
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
sut.throwIfInvalid(code, [spy]);
|
||||
// expect
|
||||
expect(spy.receivedLines).has.lengthOf(1);
|
||||
expect(spy.receivedLines[0]).to.deep.equal(expected);
|
||||
});
|
||||
});
|
||||
describe('throws invalid lines as expected', () => {
|
||||
it('throws with invalid line from single rule', () => {
|
||||
// arrange
|
||||
const errorText = 'error';
|
||||
const expectedError = new ExpectedErrorBuilder()
|
||||
.withOkLine('line1')
|
||||
.withErrorLine('line2', errorText)
|
||||
.withOkLine('line3')
|
||||
.withOkLine('line4')
|
||||
.buildError();
|
||||
const code = ['line1', 'line2', 'line3', 'line4'].join('\n');
|
||||
const invalidLines: readonly IInvalidCodeLine[] = [
|
||||
{ index: 2, error: errorText },
|
||||
];
|
||||
const rule = new CodeValidationRuleStub()
|
||||
.withReturnValue(invalidLines);
|
||||
const noopRule = new CodeValidationRuleStub()
|
||||
.withReturnValue([]);
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
const act = () => sut.throwIfInvalid(code, [rule, noopRule]);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
it('throws with combined invalid lines from multiple rules', () => {
|
||||
// arrange
|
||||
const firstError = 'firstError';
|
||||
const secondError = 'firstError';
|
||||
const expectedError = new ExpectedErrorBuilder()
|
||||
.withOkLine('line1')
|
||||
.withErrorLine('line2', firstError)
|
||||
.withOkLine('line3')
|
||||
.withErrorLine('line4', secondError)
|
||||
.buildError();
|
||||
const code = ['line1', 'line2', 'line3', 'line4'].join('\n');
|
||||
const firstRuleError: readonly IInvalidCodeLine[] = [
|
||||
{ index: 2, error: firstError },
|
||||
];
|
||||
const secondRuleError: readonly IInvalidCodeLine[] = [
|
||||
{ index: 4, error: secondError },
|
||||
];
|
||||
const firstRule = new CodeValidationRuleStub().withReturnValue(firstRuleError);
|
||||
const secondRule = new CodeValidationRuleStub().withReturnValue(secondRuleError);
|
||||
const sut = new CodeValidator();
|
||||
// act
|
||||
const act = () => sut.throwIfInvalid(code, [firstRule, secondRule]);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
class ExpectedErrorBuilder {
|
||||
private lineCount = 0;
|
||||
|
||||
private outputLines = new Array<string>();
|
||||
|
||||
public withOkLine(text: string) {
|
||||
return this.withNumberedLine(`✅ ${text}`);
|
||||
}
|
||||
|
||||
public withErrorLine(text: string, error: string) {
|
||||
return this
|
||||
.withNumberedLine(`❌ ${text}`)
|
||||
.withLine(`\t⟶ ${error}`);
|
||||
}
|
||||
|
||||
public buildError(): string {
|
||||
return [
|
||||
'Errors with the code.',
|
||||
...this.outputLines,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
private withLine(line: string) {
|
||||
this.outputLines.push(line);
|
||||
return this;
|
||||
}
|
||||
|
||||
private withNumberedLine(text: string) {
|
||||
this.lineCount += 1;
|
||||
const lineNumber = `[${this.lineCount}]`;
|
||||
return this.withLine(`${lineNumber} ${text}`);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,37 @@
|
||||
import 'mocha';
|
||||
import { expect } from 'chai';
|
||||
import { ICodeValidationRule, IInvalidCodeLine } from '@/application/Parser/Script/Validation/ICodeValidationRule';
|
||||
import { ICodeLine } from '@/application/Parser/Script/Validation/ICodeLine';
|
||||
|
||||
interface ICodeValidationRuleTestCase {
|
||||
testName: string;
|
||||
codeLines: readonly string[];
|
||||
expected: readonly IInvalidCodeLine[];
|
||||
sut: ICodeValidationRule;
|
||||
}
|
||||
|
||||
export function testCodeValidationRule(testCases: readonly ICodeValidationRuleTestCase[]) {
|
||||
for (const testCase of testCases) {
|
||||
it(testCase.testName, () => {
|
||||
// arrange
|
||||
const { sut } = testCase;
|
||||
const codeLines = createCodeLines(testCase.codeLines);
|
||||
// act
|
||||
const actual = sut.analyze(codeLines);
|
||||
// assert
|
||||
function sort(lines: readonly IInvalidCodeLine[]) { // To ignore order
|
||||
return Array.from(lines).sort((a, b) => a.index - b.index);
|
||||
}
|
||||
expect(sort(actual)).to.deep.equal(sort(testCase.expected));
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
function createCodeLines(lines: readonly string[]): ICodeLine[] {
|
||||
return lines.map((lineText, index): ICodeLine => (
|
||||
{
|
||||
index: index + 1,
|
||||
text: lineText,
|
||||
}
|
||||
));
|
||||
}
|
||||
@@ -0,0 +1,93 @@
|
||||
import 'mocha';
|
||||
import { expect } from 'chai';
|
||||
import { itEachAbsentObjectValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
import { NoDuplicatedLines } from '@/application/Parser/Script/Validation/Rules/NoDuplicatedLines';
|
||||
import { LanguageSyntaxStub } from '@tests/unit/shared/Stubs/LanguageSyntaxStub';
|
||||
import { IInvalidCodeLine } from '@/application/Parser/Script/Validation/ICodeValidationRule';
|
||||
import { testCodeValidationRule } from './CodeValidationRuleTestRunner';
|
||||
|
||||
describe('NoDuplicatedLines', () => {
|
||||
describe('ctor', () => {
|
||||
describe('throws if syntax is missing', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing syntax';
|
||||
const syntax = absentValue;
|
||||
// act
|
||||
const act = () => new NoDuplicatedLines(syntax);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
});
|
||||
describe('analyze', () => {
|
||||
testCodeValidationRule([
|
||||
{
|
||||
testName: 'no results when code is valid',
|
||||
codeLines: ['unique1', 'unique2', 'unique3', 'unique4'],
|
||||
expected: [],
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()),
|
||||
},
|
||||
{
|
||||
testName: 'detects single duplicated line as expected',
|
||||
codeLines: ['duplicate', 'duplicate', 'unique', 'duplicate'],
|
||||
expected: expectInvalidCodeLines([1, 2, 4]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()),
|
||||
},
|
||||
{
|
||||
testName: 'detects multiple duplicated lines as expected',
|
||||
codeLines: ['duplicate1', 'duplicate2', 'unique', 'duplicate1', 'unique2', 'duplicate2'],
|
||||
expected: expectInvalidCodeLines([1, 4], [2, 6]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()),
|
||||
},
|
||||
{
|
||||
testName: 'common code parts: does not detect multiple common code part usages as duplicates',
|
||||
codeLines: ['good', 'good', 'bad', 'bad', 'good', 'also-good', 'also-good', 'unique'],
|
||||
expected: expectInvalidCodeLines([3, 4]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()
|
||||
.withCommonCodeParts('good', 'also-good')),
|
||||
},
|
||||
{
|
||||
testName: 'common code parts: does not detect multiple common code part used in same code line as duplicates',
|
||||
codeLines: ['bad', 'bad', 'good1 good2', 'good1 good2', 'good2 good1', 'good2 good1'],
|
||||
expected: expectInvalidCodeLines([1, 2]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()
|
||||
.withCommonCodeParts('good2', 'good1')),
|
||||
},
|
||||
{
|
||||
testName: 'common code parts: detects when common code parts used in conjunction with unique words',
|
||||
codeLines: [
|
||||
'common-part1', 'common-part1', 'common-part1 common-part2', 'common-part1 unique', 'common-part1 unique',
|
||||
'common-part2', 'common-part2 common-part1', 'unique common-part2', 'unique common-part2',
|
||||
],
|
||||
expected: expectInvalidCodeLines([4, 5], [8, 9]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()
|
||||
.withCommonCodeParts('common-part1', 'common-part2')),
|
||||
},
|
||||
{
|
||||
testName: 'comments: does not when lines start with comment',
|
||||
codeLines: ['#abc', '#abc', 'abc', 'unique', 'abc', '//abc', '//abc', '//unique', '#unique'],
|
||||
expected: expectInvalidCodeLines([3, 5]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()
|
||||
.withCommentDelimiters('#', '//')),
|
||||
},
|
||||
{
|
||||
testName: 'comments: does when comments come after lien start',
|
||||
codeLines: ['test #comment', 'test #comment', 'test2 # comment', 'test2 # comment'],
|
||||
expected: expectInvalidCodeLines([1, 2], [3, 4]),
|
||||
sut: new NoDuplicatedLines(new LanguageSyntaxStub()
|
||||
.withCommentDelimiters('#')),
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
function expectInvalidCodeLines(
|
||||
...lines: readonly ReadonlyArray<number>[]
|
||||
): IInvalidCodeLine[] {
|
||||
return lines.flatMap((occurrenceIndices): readonly IInvalidCodeLine[] => occurrenceIndices
|
||||
.map((index): IInvalidCodeLine => ({
|
||||
index,
|
||||
error: `Line is duplicated at line numbers ${occurrenceIndices.join(',')}.`,
|
||||
})));
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
import { NoEmptyLines } from '@/application/Parser/Script/Validation/Rules/NoEmptyLines';
|
||||
import { testCodeValidationRule } from './CodeValidationRuleTestRunner';
|
||||
|
||||
describe('NoEmptyLines', () => {
|
||||
describe('analyze', () => {
|
||||
testCodeValidationRule([
|
||||
{
|
||||
testName: 'no results when code is valid',
|
||||
codeLines: ['non-empty-line1', 'none-empty-line2'],
|
||||
expected: [],
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for empty line',
|
||||
codeLines: ['first line', '', 'third line'],
|
||||
expected: [{ index: 2, error: 'Empty line' }],
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for multiple empty lines',
|
||||
codeLines: ['first line', '', 'third line', ''],
|
||||
expected: [2, 4].map((index) => ({ index, error: 'Empty line' })),
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for undefined and null lines',
|
||||
codeLines: ['first line', undefined, 'third line', null],
|
||||
expected: [2, 4].map((index) => ({ index, error: 'Empty line' })),
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for whitespace-only lines',
|
||||
codeLines: ['first line', ' ', 'third line'],
|
||||
expected: [{ index: 2, error: 'Empty line: "{whitespace}{whitespace}"' }],
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for tab-only lines',
|
||||
codeLines: ['first line', '\t\t', 'third line'],
|
||||
expected: [{ index: 2, error: 'Empty line: "{tab}{tab}"' }],
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
{
|
||||
testName: 'shows error for lines that consists of whitespace and tabs',
|
||||
codeLines: ['first line', '\t \t', 'third line', ' \t '],
|
||||
expected: [{ index: 2, error: 'Empty line: "{tab}{whitespace}{tab}"' }, { index: 4, error: 'Empty line: "{whitespace}{tab}{whitespace}"' }],
|
||||
sut: new NoEmptyLines(),
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,32 @@
|
||||
import 'mocha';
|
||||
import { expect } from 'chai';
|
||||
import { ShellScriptSyntax } from '@/application/Parser/Script/Validation/Syntax/ShellScriptSyntax';
|
||||
import { ILanguageSyntax } from '@/application/Parser/Script/Validation/Syntax/ILanguageSyntax';
|
||||
import { BatchFileSyntax } from '@/application/Parser/Script/Validation/Syntax/BatchFileSyntax';
|
||||
|
||||
function getSystemsUnderTest(): ILanguageSyntax[] {
|
||||
return [new BatchFileSyntax(), new ShellScriptSyntax()];
|
||||
}
|
||||
|
||||
describe('ConcreteSyntaxes', () => {
|
||||
describe('commentDelimiters', () => {
|
||||
for (const sut of getSystemsUnderTest()) {
|
||||
it(`${sut.constructor.name} returns defined value`, () => {
|
||||
// act
|
||||
const value = sut.commentDelimiters;
|
||||
// assert
|
||||
expect(value);
|
||||
});
|
||||
}
|
||||
});
|
||||
describe('commonCodeParts', () => {
|
||||
for (const sut of getSystemsUnderTest()) {
|
||||
it(`${sut.constructor.name} returns defined value`, () => {
|
||||
// act
|
||||
const value = sut.commonCodeParts;
|
||||
// assert
|
||||
expect(value);
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,14 @@
|
||||
import 'mocha';
|
||||
import { SyntaxFactory } from '@/application/Parser/Script/Validation/Syntax/SyntaxFactory';
|
||||
import { ScriptingLanguage } from '@/domain/ScriptingLanguage';
|
||||
import { ShellScriptSyntax } from '@/application/Parser/Script/Validation/Syntax/ShellScriptSyntax';
|
||||
import { ScriptingLanguageFactoryTestRunner } from '@tests/unit/application/Common/ScriptingLanguage/ScriptingLanguageFactoryTestRunner';
|
||||
import { BatchFileSyntax } from '@/application/Parser/Script/Validation/Syntax/BatchFileSyntax';
|
||||
|
||||
describe('SyntaxFactory', () => {
|
||||
const sut = new SyntaxFactory();
|
||||
const runner = new ScriptingLanguageFactoryTestRunner()
|
||||
.expect(ScriptingLanguage.shellscript, ShellScriptSyntax)
|
||||
.expect(ScriptingLanguage.batchfile, BatchFileSyntax);
|
||||
runner.testCreateMethod(sut);
|
||||
});
|
||||
Reference in New Issue
Block a user