diff --git a/common/web/types/src/kvk/kvks-file-reader.ts b/common/web/types/src/kvk/kvks-file-reader.ts index c12b7036e4..184c162210 100644 --- a/common/web/types/src/kvk/kvks-file-reader.ts +++ b/common/web/types/src/kvk/kvks-file-reader.ts @@ -7,12 +7,6 @@ import { VisualKeyboard, VisualKeyboardHeaderFlags, VisualKeyboardKey, VisualKey import { USVirtualKeyCodes } from '../consts/virtual-key-constants.js'; import { BUILDER_KVK_HEADER_VERSION } from './kvk-file.js'; -export enum KVKSParseErrorType { invalidVkey }; -export class KVKSParseError extends Error { - public type: KVKSParseErrorType; - public vkey: string; -}; - export default class KVKSFileReader { public read(file: Uint8Array): KVKSourceFile { let source: KVKSourceFile; @@ -33,10 +27,12 @@ export default class KVKSFileReader { // rather than using the version tagged on npmjs.com. }); - parser.parseString(file, (e: unknown, r: unknown) => { source = r as KVKSourceFile }); - source = this.boxArrays(source); - this.cleanupFlags(source); - this.cleanupUnderscore('visualkeyboard', source.visualkeyboard); + parser.parseString(file, (e: unknown, r: unknown) => { if(e) { throw e }; source = r as KVKSourceFile }); + if(source) { + source = this.boxArrays(source); + this.cleanupFlags(source); + this.cleanupUnderscore('visualkeyboard', source.visualkeyboard); + } return source; } @@ -80,7 +76,7 @@ export default class KVKSFileReader { } } - public transform(source: KVKSourceFile, errors?: KVKSParseError[]): VisualKeyboard { + public transform(source: KVKSourceFile, invalidVkeys?: string[]): VisualKeyboard { // NOTE: at this point, the xml should have been validated // and matched the schema result so we can assume properties exist let result: VisualKeyboard = { @@ -118,11 +114,8 @@ export default class KVKSFileReader { for(let sourceKey of layer.key) { let vkey = (USVirtualKeyCodes as any)[sourceKey.$?.vkey]; if(!vkey) { - if(errors) { - let e = new KVKSParseError(); - e.type = KVKSParseErrorType.invalidVkey; - e.vkey = sourceKey.$?.vkey; - errors.push(e); + if(typeof invalidVkeys !== 'undefined') { + invalidVkeys.push(sourceKey.$?.vkey); } continue; } diff --git a/common/web/types/test/kvk/test-kvk-round-trip.ts b/common/web/types/test/kvk/test-kvk-round-trip.ts index 41dbe2dbc0..031ad60b9e 100644 --- a/common/web/types/test/kvk/test-kvk-round-trip.ts +++ b/common/web/types/test/kvk/test-kvk-round-trip.ts @@ -5,7 +5,7 @@ import Hexy from 'hexy'; import gitDiff from 'git-diff'; const { hexy } = Hexy; import { loadSchema, makePathToFixture } from '../helpers/index.js'; -import KvksFileReader, { KVKSParseError } from "../../src/kvk/kvks-file-reader.js"; +import KvksFileReader from "../../src/kvk/kvks-file-reader.js"; import KvkFileReader from "../../src/kvk/kvk-file-reader.js"; import KvkFileWriter from "../../src/kvk/kvk-file-writer.js"; import KvksFileWriter from "../../src/kvk/kvks-file-writer.js"; @@ -53,9 +53,9 @@ describe('kvks-file-reader', function () { assert.doesNotThrow(() => { reader.validate(kvks, loadSchema('kvks')); }); - const errors: KVKSParseError[] = []; - const vk = reader.transform(kvks, errors); - assert.isEmpty(errors); + const invalidVkeys: string[] = []; + const vk = reader.transform(kvks, invalidVkeys); + assert.isEmpty(invalidVkeys); const writer = new KvkFileWriter(); const output = writer.write(vk); assertBufferMatch(Buffer.from(output), compiled); @@ -76,9 +76,9 @@ describe('kvks-file-reader', function () { assert.doesNotThrow(() => { kvksReader.validate(kvks, loadSchema('kvks')); }); - const errors: KVKSParseError[] = []; - const vk2 = kvksReader.transform(kvks, errors); - assert.isEmpty(errors); + const invalidVkeys: string[] = []; + const vk2 = kvksReader.transform(kvks, invalidVkeys); + assert.isEmpty(invalidVkeys); // make sure the binary is the same assert.deepEqual(vk2, vk); diff --git a/common/web/types/test/kvk/test-kvks-file.ts b/common/web/types/test/kvk/test-kvks-file.ts index 523bfaf54d..4cbb299a00 100644 --- a/common/web/types/test/kvk/test-kvks-file.ts +++ b/common/web/types/test/kvk/test-kvks-file.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import 'mocha'; import { loadSchema, makePathToFixture } from '../helpers/index.js'; -import KvksFileReader, { KVKSParseError } from "../../src/kvk/kvks-file-reader.js"; +import KvksFileReader from "../../src/kvk/kvks-file-reader.js"; import KvksFileWriter from "../../src/kvk/kvks-file-writer.js"; import { verify_khmer_angkor } from './test-kvk-utils.js'; import { assert } from 'chai'; @@ -16,9 +16,9 @@ describe('kvks-file-reader', function() { assert.doesNotThrow(() => { reader.validate(kvks, loadSchema('kvks')); }); - const errors: KVKSParseError[] = []; - const vk = reader.transform(kvks, errors); - assert.isEmpty(errors); + const invalidVkeys: string[] = []; + const vk = reader.transform(kvks, invalidVkeys); + assert.isEmpty(invalidVkeys); verify_khmer_angkor(vk); }); }); @@ -30,9 +30,9 @@ describe('kvks-file-writer', function() { const reader = new KvksFileReader(); const kvksExpected = reader.read(input); - const errors: KVKSParseError[] = []; - const vk = reader.transform(kvksExpected, errors); - assert.isEmpty(errors); + const invalidVkeys: string[] = []; + const vk = reader.transform(kvksExpected, invalidVkeys); + assert.isEmpty(invalidVkeys); const writer = new KvksFileWriter(); const output = writer.write(vk); diff --git a/developer/src/kmc-kmn/src/compiler/compiler.ts b/developer/src/kmc-kmn/src/compiler/compiler.ts index 3c6df80e7e..4761fc5c3b 100644 --- a/developer/src/kmc-kmn/src/compiler/compiler.ts +++ b/developer/src/kmc-kmn/src/compiler/compiler.ts @@ -88,18 +88,7 @@ export class KmnCompiler { } public run(infile: string, outfile: string, options?: CompilerOptions): boolean { - if(!this.verifyInitialized()) { - /* c8 ignore next 2 */ - return false; - } - - options = {...baseOptions, ...options}; - (globalThis as any)[this.callbackID] = { - message: this.compilerMessageCallback, - loadFile: this.loadFileCallback - }; let result = this.runCompiler(infile, outfile, options); - delete (globalThis as any)[this.callbackID]; if(result) { if(result.kmx) { this.callbacks.fs.writeFileSync(result.kmx.filename, result.kmx.data); @@ -133,9 +122,8 @@ export class KmnCompiler { } if(bufferSize != data.byteLength) { - // TODO: consider chucking a wobbly because this is a bug #8885 /* c8 ignore next 2 */ - return 0; + throw new Error(`Second call, expected file size ${bufferSize} == ${data.byteLength}`); } this.Module.HEAP8.set(data, buffer); @@ -143,7 +131,19 @@ export class KmnCompiler { return 1; } - private runCompiler(infile: string, outfile: string, options: CompilerOptions): CompilerResult { + public runCompiler(infile: string, outfile: string, options: CompilerOptions): CompilerResult { + if(!this.verifyInitialized()) { + /* c8 ignore next 2 */ + return null; + } + + options = {...baseOptions, ...options}; + + (globalThis as any)[this.callbackID] = { + message: this.compilerMessageCallback, + loadFile: this.loadFileCallback + }; + let result: CompilerResult = {}; let wasm_interface = new this.Module.CompilerInterface(); let wasm_options = new this.Module.CompilerOptions(); @@ -183,6 +183,7 @@ export class KmnCompiler { } wasm_interface.delete(); wasm_options.delete(); + delete (globalThis as any)[this.callbackID]; } } @@ -190,22 +191,19 @@ export class KmnCompiler { // The compiler detected a .kvks file, which needs to be captured let reader = new KvksFileReader(); kvksFilename = this.callbacks.resolveFilename(kmnFilename, kvksFilename); - let kvks = reader.read(this.callbacks.loadFile(kvksFilename)); + let filename = this.callbacks.path.basename(kvksFilename); + let kvks = null; try { + kvks = reader.read(this.callbacks.loadFile(kvksFilename)); reader.validate(kvks, this.callbacks.loadSchema('kvks')); } catch(e) { - console.log(e); - // TODO: also unit test #8886 - // TODO: this.callbacks.reportMessage(CompilerMessages.Error_InvalidKvksFile({e})); + this.callbacks.reportMessage(CompilerMessages.Error_InvalidKvksFile({filename, e})); return null; } - let errors: any = []; //TODO: KVKSParseError[]; - let vk = reader.transform(kvks, errors); - if(!vk || errors.length) { - console.dir(errors); - // TODO: also unit test #8886 - // TODO: this.callbacks.reportMessage(CompilerMessages.Error_InvalidKvksFile({e})); - return null; + let invalidVkeys: string[] = []; + let vk = reader.transform(kvks, invalidVkeys); + for(let invalidVkey of invalidVkeys) { + this.callbacks.reportMessage(CompilerMessages.Warn_InvalidVkeyInKvksFile({filename, invalidVkey})); } let writer = new KvkFileWriter(); return { diff --git a/developer/src/kmc-kmn/src/compiler/messages.ts b/developer/src/kmc-kmn/src/compiler/messages.ts index 10578870d7..0769096e07 100644 --- a/developer/src/kmc-kmn/src/compiler/messages.ts +++ b/developer/src/kmc-kmn/src/compiler/messages.ts @@ -67,6 +67,14 @@ export class CompilerMessages { static Error_UnicodeSetSyntaxError = () => m(this.ERROR_UnicodeSetSyntaxError, `UnicodeSet had a Syntax Error while parsing`); static ERROR_UnicodeSetSyntaxError = SevError | 0x1007; + + static Error_InvalidKvksFile = (o:{filename: string, e: any}) => m(this.ERROR_InvalidKvksFile, + `Error encountered parsing ${o.filename}: ${o.e}`); + static ERROR_InvalidKvksFile = SevError | 0x1008; + + static Warn_InvalidVkeyInKvksFile = (o:{filename: string, invalidVkey: string}) => m(this.WARN_InvalidVkeyInKvksFile, + `Invalid virtual key ${o.invalidVkey} found in ${o.filename}`); + static WARN_InvalidVkeyInKvksFile = SevWarn | 0x1009; } export function mapErrorFromKmcmplib(line: number, code: number, msg: string): CompilerEvent { diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kmn new file mode 100644 index 0000000000..f37d6397c5 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kmn @@ -0,0 +1,12 @@ +c Description: Verifies that kmc throws an error with an invalid .kvks file + +store(&NAME) 'error_invalid_kvks_file' +store(&VERSION) '9.0' +store(&VISUALKEYBOARD) 'error_invalid_kvks_file.kvks' + +begin unicode > use(main) + +group(main) using keys + ++ [K_A] > 'a' +'a' + [K_B] > 'ខ្មែរ' diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kvks b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kvks new file mode 100644 index 0000000000..c681064f5b --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_invalid_kvks_file.kvks @@ -0,0 +1,9 @@ + + +
+ 10.0 + caps_lock_layer_3620 + + +
+ diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kmn new file mode 100644 index 0000000000..18a88973e9 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kmn @@ -0,0 +1,12 @@ +c Description: Verifies that kmc reports a warning with an invalid vkey in the kvks + +store(&NAME) 'warn_invalid_vkey_in_kvks_file' +store(&VERSION) '9.0' +store(&VISUALKEYBOARD) 'warn_invalid_vkey_in_kvks_file.kvks' + +begin unicode > use(main) + +group(main) using keys + ++ [K_A] > 'a' +'a' + [K_B] > 'ខ្មែរ' diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kvks b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kvks new file mode 100644 index 0000000000..f7b247db34 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_invalid_vkey_in_kvks_file.kvks @@ -0,0 +1,14 @@ + + +
+ 10.0 + caps_lock_layer_3620 + +
+ + + + ឞ + + + diff --git a/developer/src/kmc-kmn/test/helpers/index.ts b/developer/src/kmc-kmn/test/helpers/index.ts new file mode 100644 index 0000000000..8250dfd101 --- /dev/null +++ b/developer/src/kmc-kmn/test/helpers/index.ts @@ -0,0 +1,17 @@ +/** + * Helpers and utilities for the Mocha tests. + */ +import * as path from 'path'; +import { fileURLToPath } from 'url'; + +/** + * Builds a path to the fixture with the given path components. + * + * e.g., makePathToFixture('example.qaa.trivial') + * e.g., makePathToFixture('example.qaa.trivial', 'model.ts') + * + * @param components One or more path components. + */ + export function makePathToFixture(...components: string[]): string { + return fileURLToPath(new URL(path.join('..', '..', '..', 'test', 'fixtures', ...components), import.meta.url)); +} diff --git a/developer/src/kmc-kmn/test/test-messages.ts b/developer/src/kmc-kmn/test/test-messages.ts index 0dee53f121..dce0c2477d 100644 --- a/developer/src/kmc-kmn/test/test-messages.ts +++ b/developer/src/kmc-kmn/test/test-messages.ts @@ -1,9 +1,54 @@ import 'mocha'; +import { assert } from 'chai'; import { CompilerMessages } from '../src/compiler/messages.js'; -import { verifyCompilerMessagesObject } from '@keymanapp/developer-test-helpers'; +import { TestCompilerCallbacks, verifyCompilerMessagesObject } from '@keymanapp/developer-test-helpers'; +import { makePathToFixture } from './helpers/index.js'; +import { KmnCompiler } from '../src/main.js'; describe('CompilerMessages', function () { + const callbacks = new TestCompilerCallbacks(); + it('should have a valid CompilerMessages object', function() { return verifyCompilerMessagesObject(CompilerMessages); }); + + // + // Message tests + // + + async function testForMessage(context: Mocha.Context, fixture: string[], messageId?: number) { + context.timeout(10000); + + callbacks.clear(); + + const compiler = new KmnCompiler(); + assert(await compiler.init(callbacks)); + assert(compiler.verifyInitialized()); + + const kmnPath = makePathToFixture(...fixture); + const outfile = callbacks.path.basename(kmnPath, '.kmn') + '.kmx'; + + // Note: throwing away compile results (just to memory) + compiler.runCompiler(kmnPath, outfile, {saveDebug: true, shouldAddCompilerVersion: false}); + + if(messageId) { + assert.isTrue(callbacks.hasMessage(messageId), `messageId ${messageId.toString(16)} not generated, instead got: `+JSON.stringify(callbacks.messages,null,2)); + assert.lengthOf(callbacks.messages, 1); + } else { + assert.lengthOf(callbacks.messages, 0, `messages should be empty, but instead got: `+JSON.stringify(callbacks.messages,null,2)); + } + } + + // ERROR_InvalidKvksFile + + it('should generate ERROR_InvalidKvksFile if the kvks is not valid XML', async function() { + await testForMessage(this, ['invalid-keyboards', 'error_invalid_kvks_file.kmn'], CompilerMessages.ERROR_InvalidKvksFile); + }); + + // WARN_InvalidVkeyInKvksFile + + it('should generate WARN_InvalidVkeyInKvksFile if the kvks contains an invalid virtual key', async function() { + await testForMessage(this, ['invalid-keyboards', 'warn_invalid_vkey_in_kvks_file.kmn'], CompilerMessages.WARN_InvalidVkeyInKvksFile); + }); + }); diff --git a/developer/src/kmc-kmn/test/tsconfig.json b/developer/src/kmc-kmn/test/tsconfig.json index 115412f57f..f61df15d4a 100644 --- a/developer/src/kmc-kmn/test/tsconfig.json +++ b/developer/src/kmc-kmn/test/tsconfig.json @@ -13,7 +13,8 @@ }, }, "include": [ - "**/test-*.ts" + "**/test-*.ts", + "./helpers/index.ts" ], "references": [ { "path": "../../../../common/web/keyman-version/tsconfig.esm.json" },