Merge pull request #12407 from keymanapp/fix/developer/cherry-pick/12…

…403-validate-string-use-before-definition fix(developer): check vars string usage before definition 🍒 🏠
keymanapp · Sep 16, 2024 · 69a5d99 · 69a5d99
2 parents 1acdc5b + 62af49a
commit 69a5d99
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 7 deletions.
diff --git a/common/web/types/src/kmx/kmx-plus.ts b/common/web/types/src/kmx/kmx-plus.ts
@@ -285,7 +285,7 @@ export class Vars extends Section {
     });
   }
   findStringVariableValue(id: string): string {
-    return Vars.findVariable(this.strings, id)?.value?.value; // Unwrap: Variable, StrsItem
+    return Vars.findVariable(this.strings, id)?.value?.value ?? null; // Unwrap: Variable, StrsItem
   }
   substituteSetRegex(str: string, sections: DependencySections): string {
     return str.replaceAll(VariableParser.SET_REFERENCE, (_entire, id) => {

diff --git a/developer/src/kmc-ldml/src/compiler/vars.ts b/developer/src/kmc-ldml/src/compiler/vars.ts
@@ -68,9 +68,16 @@ export class VarsCompiler extends SectionCompiler {
       // Strings
       for (const { id, value } of variables.string) {
         addId(id);
-        allStrings.add(id);
         const stringrefs = VariableParser.allStringReferences(value);
+        for(const ref of stringrefs) {
+          if(!allStrings.has(ref)) {
+            valid = false;
+            this.callbacks.reportMessage(CompilerMessages.Error_MissingStringVariable({id: ref}));
+            allStrings.add(ref); // avoids multiple reports of same missing variable
+          }
+        }
         st.string.add(SubstitutionUse.variable, stringrefs);
+        allStrings.add(id);
       }
       // Sets
       for (const { id, value } of variables.set) {

diff --git a/developer/src/kmc-ldml/test/fixtures/sections/vars/fail-badref-7.xml b/developer/src/kmc-ldml/test/fixtures/sections/vars/fail-badref-7.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info name="vars-fail"/>
+
+  <keys />
+
+  <!-- from spec -->
+  <variables>
+    <string id="y" value="${usedBeforeDefinition}" /> <!-- FAIL: reference before definition -->
+    <string id="usedBeforeDefinition" value="yes" />
+  </variables>
+
+
+</keyboard3>
diff --git a/developer/src/kmc-ldml/test/test-vars.ts b/developer/src/kmc-ldml/test/test-vars.ts
@@ -187,7 +187,14 @@ describe('vars', function () {
         CompilerMessages.Error_MissingStringVariable({id: 'missingStringInSet'})
       ],
     },
-  ], varsDependencies);
+    {
+      subpath: 'sections/vars/fail-badref-7.xml',
+      errors: [
+        CompilerMessages.Error_MissingStringVariable({id: 'usedBeforeDefinition'})
+      ],
+      strictErrors: true
+    },
+], varsDependencies);
   describe('should match some marker constants', () => {
     // neither of these live here, but, common/web/types does not import ldml-keyboard-constants otherwise.
 

diff --git a/developer/src/kmc-ldml/test/test-visual-keyboard-compiler.ts b/developer/src/kmc-ldml/test/test-visual-keyboard-compiler.ts
@@ -159,7 +159,7 @@ describe('visual-keyboard-compiler', function() {
     assert.equal(vk.keys[1].text, '\u{0e81}');
   });
 
-  it.skip('should read string variables in key.output', async function() {
+  it('should read string variables in key.output', async function() {
     const xml = stripIndent`
       <?xml version="1.0" encoding="UTF-8"?>
       <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
@@ -171,8 +171,8 @@ describe('visual-keyboard-compiler', function() {
           <layer modifiers="none"><row keys="x" /></layer>
         </layers>
         <variables>
-          <string id="one" value="\${two}" />
           <string id="two" value="2" />
+          <string id="one" value="\${two}" />
         </variables>
       </keyboard3>
     `;
@@ -183,7 +183,7 @@ describe('visual-keyboard-compiler', function() {
     assert.equal(vk.keys[0].text, '2');
   });
 
-  it.skip('should read string variables in display.display', async function() {
+  it('should read string variables in display.display', async function() {
     const xml = stripIndent`
       <?xml version="1.0" encoding="UTF-8"?>
       <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
@@ -198,8 +198,8 @@ describe('visual-keyboard-compiler', function() {
           <layer modifiers="none"><row keys="x" /></layer>
         </layers>
         <variables>
-          <string id="one" value="\${two}" />
           <string id="two" value="2" />
+          <string id="one" value="\${two}" />
         </variables>
       </keyboard3>
     `;