Merge pull request #10300 from keymanapp/fix/developer/10291-layer-er…

…rs-epic-ldml

fix(developer): quell internal error when  a section fails 🙀

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

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);
   }
-};
+};

developer/src/kmc-ldml/src/compiler/compiler.ts

@@ -254,19 +254,30 @@ export class LdmlKeyboardCompiler implements KeymanCompiler {
       // 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[<SectionIdent>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 */

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;
+    }
   }
 }
 
@@ -178,6 +186,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'
    */
@@ -239,6 +249,9 @@ export function testCompilationCases(compiler: SectionCompilerNew, cases : Compi
       if (testcase.errors && testcase.errors !== true) {
         assert.includeDeepMembers(callbacks.messages, <CompilerEvent[]> testcase.errors, 'expected errors to be included');
       }
+      if (testcase.errors && testcase.strictErrors) {
+        assert.sameDeepMembers(callbacks.messages, <CompilerEvent[]> 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) {

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);
 });