From 21a768027831b390a6e4d6d836279351b82ca16a Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Thu, 21 Dec 2023 22:14:54 -0600 Subject: [PATCH] =?UTF-8?q?fix(developer):=20quell=20internal=20error=20wh?= =?UTF-8?q?en=20=20a=20section=20fails=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - when a required section is missing, only throw an internal error IF we haven't already flagged a problem. for example, if 'vars' fails to compile because of a problem, and we already have an error on file, then don't throw an internal error that 'bksp' depends on uninitialized 'vars'. - mimic this flow in the test helpers. - add a test case for this in the test helpers, in test-tran - add a strictError flag to say that no additional errs are allowed. Fixes: #10291 --- .../web/types/src/util/compiler-interfaces.ts | 13 +++++++++++ .../src/common/web/test-helpers/index.ts | 7 +++++- .../src/kmc-ldml/src/compiler/compiler.ts | 15 ++++++++++-- developer/src/kmc-ldml/test/helpers/index.ts | 23 +++++++++++++++---- developer/src/kmc-ldml/test/test-tran.ts | 16 +++++++++++++ 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/common/web/types/src/util/compiler-interfaces.ts b/common/web/types/src/util/compiler-interfaces.ts index b27a170ab34..79072c6f30e 100644 --- a/common/web/types/src/util/compiler-interfaces.ts +++ b/common/web/types/src/util/compiler-interfaces.ts @@ -53,6 +53,19 @@ export class CompilerError { static formatSeverity(code: number): string { return errorSeverityName[CompilerError.severity(code)] ?? 'UNKNOWN'; } + /** true if events has at least one message of the atLeast severity */ + static hasSeverity(events: CompilerEvent[], atLeast: CompilerErrorSeverity): boolean { + for (const { code } of events) { + if (CompilerError.severity(code) >= atLeast) { + return true; + } + } + return false; + } + /** true if events has at least one Error or worse */ + static hasError(events: CompilerEvent[]): boolean { + return CompilerError.hasSeverity(events, CompilerErrorSeverity.Error); + } /** * Format an error code number. The error code number does not include * the severity mask, as this is reported in text form separately; see diff --git a/developer/src/common/web/test-helpers/index.ts b/developer/src/common/web/test-helpers/index.ts index 0ccbc7ff7bd..9a9db0d1ce8 100644 --- a/developer/src/common/web/test-helpers/index.ts +++ b/developer/src/common/web/test-helpers/index.ts @@ -23,6 +23,11 @@ export class TestCompilerCallbacks implements CompilerCallbacks { return this.messages.find((item) => item.code == code) === undefined ? false : true; } + /** true of at least one error */ + hasError(): boolean { + return CompilerError.hasError(this.messages); + } + /* CompilerCallbacks */ loadFile(filename: string): Uint8Array { @@ -76,4 +81,4 @@ export class TestCompilerCallbacks implements CompilerCallbacks { debug(msg: string) { console.debug(msg); } -}; \ No newline at end of file +}; diff --git a/developer/src/kmc-ldml/src/compiler/compiler.ts b/developer/src/kmc-ldml/src/compiler/compiler.ts index b6f51f553b2..b7a4cb4bc7b 100644 --- a/developer/src/kmc-ldml/src/compiler/compiler.ts +++ b/developer/src/kmc-ldml/src/compiler/compiler.ts @@ -176,19 +176,30 @@ export class LdmlKeyboardCompiler { // pre-initialize the usetparser globalSections.usetparser = await this.getUsetParser(); const dependencies = section.dependencies; + let dependencyProblem = false; Object.keys(constants.section).forEach((sectstr : string) => { const sectid : SectionIdent = constants.section[sectstr]; if (dependencies.has(sectid)) { /* c8 ignore next 4 */ if (!kmx.kmxplus[sectid]) { - // Internal error useful during section bring-up - throw new Error(`Internal error: section ${section.id} depends on uninitialized dependency ${sectid}, check ordering`); + if (passed) { + // Internal error useful during section bring-up + throw new Error(`Internal error: section ${section.id} depends on uninitialized dependency ${sectid}, check ordering`); + } else { + dependencyProblem = true; + return; // Already failed to validate, so no need for the layering message. + } } } else { // delete dependencies that aren't referenced delete globalSections[sectid]; } }); + if (dependencyProblem && !passed) { + // Some layering problem, but we've already noted an error (!passed). + // Just skip this section. + continue; + } const sect = section.compile(globalSections); /* c8 ignore next 7 */ diff --git a/developer/src/kmc-ldml/test/helpers/index.ts b/developer/src/kmc-ldml/test/helpers/index.ts index 8f6d73ef439..44bbe8cece3 100644 --- a/developer/src/kmc-ldml/test/helpers/index.ts +++ b/developer/src/kmc-ldml/test/helpers/index.ts @@ -79,6 +79,10 @@ export async function loadSectionFixture(compilerClass: SectionCompilerNew, file // load dependencies first await loadDepsFor(sections, compiler, source, callbacks, dependencies); + if (callbacks.hasError()) { + // break out if there's an error + return null; + } // make sure all dependencies are loaded compiler.dependencies.forEach(dep => assert.ok(sections[dep], `Required dependency '${dep}' for '${compiler.id}' was not supplied: Check the 'dependencies' argument to loadSectionFixture or testCompilationCases`)); @@ -101,14 +105,18 @@ async function loadDepsFor(sections: DependencySections, parentCompiler: Section for (const dep of dependencies) { const compiler = new dep(source, callbacks); assert.notEqual(compiler.id, parentId, `${parentId} depends on itself`); - assert.ok(compiler.validate(), `while setting up ${parentId}: ${compiler.id} failed validate()`); + const didValidate = compiler.validate(); + if (!callbacks.hasError()) { + // only go down this path if there isn't already a noted error + assert.ok(didValidate, `while setting up ${parentId}: ${compiler.id} failed validate()`); - const sect = compiler.compile(sections); + const sect = compiler.compile(sections); - assert.ok(sect, `while setting up ${parentId}: ${compiler.id} failed compile()`); - assert.notOk(sections[compiler.id], `while setting up ${parentId}: ${compiler.id} was already in the sections[] table, probably a bad dependency`); + assert.ok(sect, `while setting up ${parentId}: ${compiler.id} failed compile()`); + assert.notOk(sections[compiler.id], `while setting up ${parentId}: ${compiler.id} was already in the sections[] table, probably a bad dependency`); - sections[compiler.id] = sect as any; + sections[compiler.id] = sect as any; + } } } @@ -175,6 +183,8 @@ export function checkMessages() { } export interface CompilationCase { + /** if true, expect no further errors than what's in errors. */ + strictErrors?: boolean; /** * path to xml, such as 'sections/layr/invalid-case.xml' */ @@ -236,6 +246,9 @@ export function testCompilationCases(compiler: SectionCompilerNew, cases : Compi if (testcase.errors && testcase.errors !== true) { assert.includeDeepMembers(callbacks.messages, testcase.errors, 'expected errors to be included'); } + if (testcase.errors && testcase.strictErrors) { + assert.sameDeepMembers(callbacks.messages, testcase.errors, 'expected same errors to be included'); + } if (testcase.warnings) { assert.includeDeepMembers(callbacks.messages, testcase.warnings, 'expected warnings to be included'); } else if (!expectFailure) { diff --git a/developer/src/kmc-ldml/test/test-tran.ts b/developer/src/kmc-ldml/test/test-tran.ts index ec77ab02f2d..4922facb632 100644 --- a/developer/src/kmc-ldml/test/test-tran.ts +++ b/developer/src/kmc-ldml/test/test-tran.ts @@ -209,6 +209,22 @@ describe('bksp', function () { assert.strictEqual(transforms[0].to.value, ""); } }, + { + // this would fail with a dependency issue if + // we tried to initialize the bksp compiler, because + // vars isn't initialized. + subpath: 'sections/vars/fail-markers-badref-0.xml', + strictErrors: true, + errors: [ + CompilerMessages.Error_MissingMarkers({ + ids: [ + 'doesnt_exist_1', + 'doesnt_exist_2', + 'doesnt_exist_3', + ] + }), + ], + }, ], bkspDependencies); });