fix(developer): check vars string usage before definition

Validation of vars was not properly checking for forward references to
string variables. This, coupled with a null vs undefined bug in
subsequent use, meant that forward reference variables were ending up
with a literal string value of 'undefined'.

This also fixes the test for visual-keyboard-compiler, where the fixture
was actually buggy and was the trigger for investigating this problem.

Fixes: #12403
Relates-to: #12395
This commit is contained in:
Marc Durdin 2024-09-12 09:18:06 +07:00
parent e4017a8c28
commit b7810bdd2e
5 changed files with 36 additions and 7 deletions

View file

@ -291,7 +291,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) => {

View file

@ -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(LdmlCompilerMessages.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) {

View file

@ -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>

View file

@ -187,7 +187,14 @@ describe('vars', function () {
LdmlCompilerMessages.Error_MissingStringVariable({id: 'missingStringInSet'})
],
},
], varsDependencies);
{
subpath: 'sections/vars/fail-badref-7.xml',
errors: [
LdmlCompilerMessages.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.

View file

@ -153,7 +153,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">
@ -165,8 +165,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>
`;
@ -177,7 +177,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">
@ -192,8 +192,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>
`;