Improve compiler output for line validation

This commit improves feedback when a line is too long to enhance
developer/maintainer productivity.

- Trim long lines in output to 500 characters
- Show character count exceeding max line length
- Refactor line formatting for better readability
This commit is contained in:
undergroundwires
2024-09-16 12:39:52 +02:00
parent 98e8dc0a67
commit 8b6067f83f
4 changed files with 197 additions and 140 deletions

View File

@@ -12,7 +12,7 @@ export const analyzeTooLongLines: CodeValidationAnalyzer = (
lineNumber: line.lineNumber, lineNumber: line.lineNumber,
error: [ error: [
`Line is too long (${line.text.length}).`, `Line is too long (${line.text.length}).`,
`It exceed maximum allowed length ${maxLineLength}.`, `It exceed maximum allowed length ${maxLineLength} by ${line.text.length - maxLineLength} characters.`,
'This may cause bugs due to unintended trimming by operating system, shells or terminal emulators.', 'This may cause bugs due to unintended trimming by operating system, shells or terminal emulators.',
].join(' '), ].join(' '),
})); }));
@@ -39,6 +39,6 @@ function getMaxAllowedLineLength(language: ScriptingLanguage): number {
*/ */
return 1048576; // Minimum value for reliability return 1048576; // Minimum value for reliability
default: default:
throw new Error(`Unsupported language: ${language}`); throw new Error(`Unsupported language: ${ScriptingLanguage[language]} (${language})`);
} }
} }

View File

@@ -46,9 +46,33 @@ function formatLines(
): string { ): string {
return lines.map((line) => { return lines.map((line) => {
const badLine = invalidLines.find((invalidLine) => invalidLine.lineNumber === line.lineNumber); const badLine = invalidLines.find((invalidLine) => invalidLine.lineNumber === line.lineNumber);
if (!badLine) { return formatLine({
return `[${line.lineNumber}] ✅ ${line.text}`; lineNumber: line.lineNumber,
} text: line.text,
return `[${badLine.lineNumber}] ❌ ${line.text}\n\t⟶ ${badLine.error}`; error: badLine?.error,
});
}).join('\n'); }).join('\n');
} }
function formatLine(
line: {
readonly lineNumber: number;
readonly text: string;
readonly error?: string;
},
): string {
let text = `[${line.lineNumber}] `;
text += line.error ? '❌' : '✅';
text += ` ${trimLine(line.text)}`;
if (line.error) {
text += `\n\t⟶ ${line.error}`;
}
return text;
}
function trimLine(line: string) {
const maxLength = 500;
if (line.length > maxLength) {
line = `${line.substring(0, maxLength)}... [Rest of the line trimmed]`;
}
return line;
}

View File

@@ -3,21 +3,20 @@ import type { CodeLine, InvalidCodeLine } from '@/application/Parser/Executable/
import { ScriptingLanguage } from '@/domain/ScriptingLanguage'; import { ScriptingLanguage } from '@/domain/ScriptingLanguage';
import { analyzeTooLongLines } from '@/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeTooLongLines'; import { analyzeTooLongLines } from '@/application/Parser/Executable/Script/Validation/Analyzers/AnalyzeTooLongLines';
import { createCodeLines } from './CreateCodeLines'; import { createCodeLines } from './CreateCodeLines';
import { expectSameInvalidCodeLines } from './ExpectSameInvalidCodeLines'; import { expectInvalidCodeLines, expectSameInvalidCodeLines } from './ExpectSameInvalidCodeLines';
describe('AnalyzeTooLongLines', () => { describe('AnalyzeTooLongLines', () => {
describe('analyzeTooLongLines', () => { describe('analyzeTooLongLines', () => {
describe('batchfile', () => { describe('returns no results for lines within the maximum length', () => {
const MAX_BATCHFILE_LENGTH = 8191; createScriptLanguageScenarios().forEach((scenario) => {
it(scenario.description, () => {
it('returns no results for lines within the maximum length', () => {
// arrange // arrange
const expected: InvalidCodeLine[] = []; const expected: InvalidCodeLine[] = [];
const context = new TestContext() const context = new TestContext()
.withLanguage(ScriptingLanguage.batchfile) .withLanguage(scenario.language)
.withLines([ .withLines([
'A'.repeat(MAX_BATCHFILE_LENGTH), 'A'.repeat(scenario.maxLength),
'B'.repeat(8000), 'B'.repeat(scenario.maxLength - 1),
'C'.repeat(100), 'C'.repeat(100),
]); ]);
// act // act
@@ -25,139 +24,152 @@ describe('AnalyzeTooLongLines', () => {
// assert // assert
expectSameInvalidCodeLines(actual, expected); expectSameInvalidCodeLines(actual, expected);
}); });
});
});
it('identifies a single line exceeding maximum length', () => { describe('identifies a single line exceeding maximum length', () => {
createScriptLanguageScenarios().forEach((scenario) => {
it(scenario.description, () => {
// arrange // arrange
const expectedLength = MAX_BATCHFILE_LENGTH + 1; const expectedLength = scenario.maxLength + 1;
const expected: InvalidCodeLine[] = [{ const expectedLineNumber = 2;
lineNumber: 2,
error: createTooLongLineError(expectedLength, MAX_BATCHFILE_LENGTH),
}];
const context = new TestContext() const context = new TestContext()
.withLanguage(ScriptingLanguage.batchfile) .withLanguage(scenario.language)
.withLines([ .withLines([
'A'.repeat(MAX_BATCHFILE_LENGTH), 'A'.repeat(scenario.maxLength),
'B'.repeat(expectedLength), 'B'.repeat(expectedLength),
'C'.repeat(100), 'C'.repeat(100),
]); ]);
// act // act
const actual = context.analyze(); const actual = context.analyze();
// assert // assert
expectSameInvalidCodeLines(actual, expected); expectInvalidCodeLines(actual, [expectedLineNumber]);
});
});
}); });
it('identifies multiple lines exceeding maximum length', () => { describe('identifies multiple lines exceeding maximum length', () => {
createScriptLanguageScenarios().forEach((scenario) => {
it(scenario.description, () => {
// arrange // arrange
const expectedLength1 = MAX_BATCHFILE_LENGTH + 1; const expectedInvalidLines: readonly {
const expectedLength2 = MAX_BATCHFILE_LENGTH + 2; readonly lineNumber: number,
const expected: InvalidCodeLine[] = [ readonly length: number,
{ }[] = [
lineNumber: 1, { lineNumber: 1, length: scenario.maxLength + 1 },
error: createTooLongLineError(expectedLength1, MAX_BATCHFILE_LENGTH), { lineNumber: 3, length: scenario.maxLength + 2 },
},
{
lineNumber: 3,
error: createTooLongLineError(expectedLength2, MAX_BATCHFILE_LENGTH),
},
]; ];
const context = new TestContext() const context = new TestContext()
.withLanguage(ScriptingLanguage.batchfile) .withLanguage(scenario.language)
.withLines([ .withLines([
'A'.repeat(expectedLength1), 'A'.repeat(expectedInvalidLines[0].length),
'B'.repeat(MAX_BATCHFILE_LENGTH), 'B'.repeat(scenario.maxLength),
'C'.repeat(expectedLength2), 'C'.repeat(expectedInvalidLines[1].length),
]); ]);
// act // act
const actual = context.analyze(); const actual = context.analyze();
// assert // assert
expectSameInvalidCodeLines(actual, expected); expectInvalidCodeLines(actual, expectedInvalidLines.map((l) => l.lineNumber));
});
}); });
}); });
describe('shellscript', () => { describe('error construction', () => {
const MAX_SHELLSCRIPT_LENGTH = 1048576; describe('outputs actual character length', () => {
createScriptLanguageScenarios().forEach((scenario) => {
it('returns no results for lines within the maximum length', () => { it(scenario.description, () => {
// arrange // arrange
const expected: InvalidCodeLine[] = []; const expectedLength = scenario.maxLength + 1;
const expectedErrorPart = `Line is too long (${expectedLength}).`;
const context = new TestContext() const context = new TestContext()
.withLanguage(ScriptingLanguage.shellscript) .withLanguage(scenario.language)
.withLines([ .withLines([
'A'.repeat(MAX_SHELLSCRIPT_LENGTH),
'B'.repeat(1000000),
'C'.repeat(100),
]);
// act
const actual = context.analyze();
// assert
expectSameInvalidCodeLines(actual, expected);
});
it('identifies a single line exceeding maximum length', () => {
// arrange
const expectedLength = MAX_SHELLSCRIPT_LENGTH + 1;
const expected: InvalidCodeLine[] = [{
lineNumber: 2,
error: createTooLongLineError(expectedLength, MAX_SHELLSCRIPT_LENGTH),
}];
const context = new TestContext()
.withLanguage(ScriptingLanguage.shellscript)
.withLines([
'A'.repeat(MAX_SHELLSCRIPT_LENGTH),
'B'.repeat(expectedLength), 'B'.repeat(expectedLength),
'C'.repeat(100),
]); ]);
// act // act
const actual = context.analyze(); const actual = context.analyze();
// assert // assert
expectSameInvalidCodeLines(actual, expected); expect(actual).to.have.lengthOf(1);
const actualError = actual[0].error;
expect(actualError).to.include(expectedErrorPart);
}); });
});
it('identifies multiple lines exceeding maximum length', () => { });
describe('outputs maximum allowed character length', () => {
createScriptLanguageScenarios().forEach((scenario) => {
it(scenario.description, () => {
// arrange // arrange
const expectedLength1 = MAX_SHELLSCRIPT_LENGTH + 1; const expectedLength = scenario.maxLength + 1;
const expectedLength2 = MAX_SHELLSCRIPT_LENGTH + 2; const expectedErrorPart = `It exceed maximum allowed length ${scenario.maxLength}`;
const expected: InvalidCodeLine[] = [
{
lineNumber: 1,
error: createTooLongLineError(expectedLength1, MAX_SHELLSCRIPT_LENGTH),
},
{
lineNumber: 3,
error: createTooLongLineError(expectedLength2, MAX_SHELLSCRIPT_LENGTH),
},
];
const context = new TestContext() const context = new TestContext()
.withLanguage(ScriptingLanguage.shellscript) .withLanguage(scenario.language)
.withLines([ .withLines([
'A'.repeat(expectedLength1), 'B'.repeat(expectedLength),
'B'.repeat(MAX_SHELLSCRIPT_LENGTH),
'C'.repeat(expectedLength2),
]); ]);
// act // act
const actual = context.analyze(); const actual = context.analyze();
// assert // assert
expectSameInvalidCodeLines(actual, expected); expect(actual).to.have.lengthOf(1);
const actualError = actual[0].error;
expect(actualError).to.include(expectedErrorPart);
});
});
});
describe('outputs total exceeding characters', () => {
createScriptLanguageScenarios().forEach((scenario) => {
it(scenario.description, () => {
// arrange
const expectedExceedingCharacters = 5;
const expectedErrorPart = `by ${expectedExceedingCharacters} characters.`;
const lineLength = scenario.maxLength + expectedExceedingCharacters;
const context = new TestContext()
.withLanguage(scenario.language)
.withLines([
'B'.repeat(lineLength),
]);
// act
const actual = context.analyze();
// assert
expect(actual).to.have.lengthOf(1);
const actualError = actual[0].error;
expect(actualError).to.include(expectedErrorPart);
});
});
}); });
}); });
it('throws an error for unsupported language', () => { it('throws an error for unsupported language', () => {
// arrange // arrange
const unsupportedLanguage = 'unsupported' as unknown as ScriptingLanguage;
const expectedError = `Unsupported language: ${ScriptingLanguage[unsupportedLanguage]} (${unsupportedLanguage})`;
const context = new TestContext() const context = new TestContext()
.withLanguage('unsupported' as unknown as ScriptingLanguage) .withLanguage('unsupported' as unknown as ScriptingLanguage)
.withLines(['A', 'B', 'C']); .withLines(['A', 'B', 'C']);
// act & assert // act
expect(() => context.analyze()).to.throw('Unsupported language: unsupported'); const act = () => context.analyze();
// assert
expect(act).to.throw(expectedError);
}); });
}); });
}); });
function createTooLongLineError(actualLength: number, maxAllowedLength: number): string { interface ScriptLanguageScenario {
return [ readonly description: string;
`Line is too long (${actualLength}).`, readonly maxLength: number;
`It exceed maximum allowed length ${maxAllowedLength}.`, readonly language: ScriptingLanguage;
'This may cause bugs due to unintended trimming by operating system, shells or terminal emulators.', }
].join(' ');
function createScriptLanguageScenarios(): readonly ScriptLanguageScenario[] {
const maxLengths: Record< // `Record` catches missing entries at compile-time
ScriptingLanguage, number> = {
[ScriptingLanguage.batchfile]: 8191,
[ScriptingLanguage.shellscript]: 1048576,
};
return Object.entries(maxLengths).map(([language, length]): ScriptLanguageScenario => ({
description: `${ScriptingLanguage[language]} (max: ${length})`,
language: Number.parseInt(language, 10) as ScriptingLanguage,
maxLength: length,
}));
} }
class TestContext { class TestContext {

View File

@@ -1,11 +1,32 @@
import { expect } from 'vitest'; import { expect } from 'vitest';
import type { InvalidCodeLine } from '@/application/Parser/Executable/Script/Validation/Analyzers/CodeValidationAnalyzer'; import type { InvalidCodeLine } from '@/application/Parser/Executable/Script/Validation/Analyzers/CodeValidationAnalyzer';
import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage';
export function expectSameInvalidCodeLines( export function expectSameInvalidCodeLines(
expected: readonly InvalidCodeLine[], expected: readonly InvalidCodeLine[],
actual: readonly InvalidCodeLine[], actual: readonly InvalidCodeLine[],
) { ) {
expect(sort(expected)).to.deep.equal(sort(actual)); const sortedExpected = sort(expected);
const sortedActual = sort(actual);
expect(sortedActual).to.deep.equal(sortedExpected, formatAssertionMessage([
'Invalid code lines do not match',
`Expected: ${JSON.stringify(sortedExpected, null, 2)}`,
`Actual: ${JSON.stringify(sortedActual, null, 2)}`,
]));
}
export function expectInvalidCodeLines(
actual: readonly InvalidCodeLine[],
expectedInvalidLineNumbers: readonly number[],
) {
const sortedActualLineNumbers = actual.map((a) => a.lineNumber).sort();
const sortedExpectedLineNumbers = [...expectedInvalidLineNumbers].sort();
expect(sortedActualLineNumbers).to.deep.equal(sortedExpectedLineNumbers, formatAssertionMessage([
'Invalid line numbers do not match.',
`Expected (${sortedExpectedLineNumbers.length}): ${sortedExpectedLineNumbers.join(', ')}`,
`Actual (${sortedActualLineNumbers.length}): ${sortedActualLineNumbers.join(', ')}`,
]));
} }
function sort(lines: readonly InvalidCodeLine[]) { // To ignore order function sort(lines: readonly InvalidCodeLine[]) { // To ignore order