Skip to content

Commit

Permalink
refactor(developer): move kmcmplib message construction to kmc-kmn
Browse files Browse the repository at this point in the history
* Remove compiler message definitions from kmcmplib
* Add parameterization to compiler message structures
* Translate parameters for existing parameterized messages (except for
  `ERROR_InvalidToken`, which requires a bigger refactor of
  `GetXStringImpl()` and many friends)
* Add columnNumber to message structures (not yet used in kmc-kmn)
* Add filename to message structures (not yet used in kmcmplib)
* Rename `INFO_Info` to `INFO_MinimumCoreEngineVersion` and
  `INFO_MinimumEngineVersion` to `INFO_MinimumWebEngineVersion`

Relates-to: #10866
  • Loading branch information
mcdurdin committed Jul 30, 2024
1 parent 59addc5 commit 8cd97e5
Show file tree
Hide file tree
Showing 21 changed files with 134 additions and 370 deletions.
8 changes: 6 additions & 2 deletions developer/src/common/include/kmn_compiler_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ namespace CompilerErrorSeverity {
};
};

#define MESSAGE_SEVERITY_MASK 0xF00000
#define MESSAGE_SEVERITY_MASK 0x00F00000 // includes reserved bits, 16 possible severity levels
#define MESSAGE_ERROR_MASK 0x000FFFFF // error | namespace
#define MESSAGE_NAMESPACE_MASK 0x000FF000 // 256 possible namespaces
#define MESSAGE_BASEERROR_MASK 0x00000FFF // error code, 2,048 possible error codes per namespace
#define MESSAGE_RESERVED_MASK 0xFF000000 // do not use these error values at this time

#define MESSAGE_NAMESPACE_KmnCompiler 0x2000

Expand Down Expand Up @@ -208,7 +212,7 @@ namespace KmnCompilerMessages {
WARN_UnicodeSurrogateUsed = SevWarn | 0x088,
WARN_ReservedCharacter = SevWarn | 0x089,

INFO_Info = SevInfo | 0x08A,
INFO_MinimumCoreEngineVersion = SevInfo | 0x08A, // renamed from INFO_Info in 18.0-alpha

WARN_VirtualKeyWithMnemonicLayout = SevWarn | 0x08B,
WARN_VirtualCharKeyWithPositionalLayout = SevWarn | 0x08C,
Expand Down
31 changes: 29 additions & 2 deletions developer/src/kmc-kmn/src/compiler/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ export interface KmnCompilerResultExtra {

/** @internal */
export interface KmnCompilerResultMessage {
message: string;
errorCode: number;
lineNumber: number;
columnNumber: number;
filename: string;
parameters: string[];
}

/**
Expand Down Expand Up @@ -250,6 +252,31 @@ export class KmnCompiler implements KeymanCompiler, UnicodeSetParser {
return new Uint8Array(new Uint8Array(Module.HEAP8.buffer, offset, size));
}

private toTitleCase = (s: string) => s.substring(0, 1).toUpperCase() + s.substring(1).toLowerCase();

private generateKmcmpLibMessage(filename: string, line: number, code: number, parameters: string[]) {
const keys = Object.keys(KmnCompilerMessages);
const m = KmnCompilerMessages as Record<string,any>;
const key = keys.find(key => m[key] === code);
if(!key) {
return `Unknown message ${code.toString(16)} -- message identifier not found`;
}

const o = /^(INFO|HINT|WARN|ERROR|FATAL)_([A-Za-z0-9_]+)$/.exec(key);
if(!o) {
return `Unknown message ${code.toString(16)} -- message identifier is not a valid format`;
}

const generator = this.toTitleCase(o[1])+'_'+o[2];
if(!generator || typeof m[generator] != 'function') {
return `Unknown message ${code.toString(16)} -- generator function not found`;
}
const result = m[generator]({p:parameters});
result.filename = filename;
result.line = line;
return result;
}

/**
* Compiles a .kmn file to .kmx, .kvk, and/or .js files. Returns an object
* containing binary artifacts on success. The files are passed in by name,
Expand All @@ -276,7 +303,7 @@ export class KmnCompiler implements KeymanCompiler, UnicodeSetParser {
const compiler = this;
const wasm_callbacks = Module.WasmCallbackInterface.implement({
message: function(message: KmnCompilerResultMessage) {
compiler.callbacks.reportMessage({line:message.lineNumber, code: message.errorCode, message: message.message});
compiler.callbacks.reportMessage(compiler.generateKmcmpLibMessage(message.filename, message.lineNumber, message.errorCode, message.parameters));
},
loadFile: function(filename: string, baseFilename: string): number[] {
const data: Uint8Array = compiler.callbacks.loadFile(compiler.callbacks.resolveFilename(baseFilename, filename));
Expand Down
31 changes: 23 additions & 8 deletions developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const enum KmnCompilerMessageRanges {
RANGE_CompilerMessage_Max = 0xFFF, // Highest available error code for kmc-kmn
}

type KmcmpLibMessageParameters = {p:string[]};

/**
* @internal
*
Expand Down Expand Up @@ -522,10 +524,16 @@ export class KmnCompilerMessages {
static Error_PostKeystrokeGroupMustBeReadonly = () => m(this.ERROR_PostKeystrokeGroupMustBeReadonly, `Group used in begin postKeystroke must be readonly`);

static ERROR_DuplicateGroup = SevError | 0x071;
static Error_DuplicateGroup = () => m(this.ERROR_DuplicateGroup, `A group with this name has already been defined.`);
static Error_DuplicateGroup = (o:KmcmpLibMessageParameters) => m(
this.ERROR_DuplicateGroup,
`A group with the name '${o.p?.[0]}' has already been defined on line ${o.p?.[1]}.`
);

static ERROR_DuplicateStore = SevError | 0x072;
static Error_DuplicateStore = () => m(this.ERROR_DuplicateStore, `A store with this name has already been defined.`);
static Error_DuplicateStore = (o:KmcmpLibMessageParameters) => m(
this.ERROR_DuplicateStore,
`A store with the name '${o.p?.[0]}' has already been defined on line ${o.p?.[1]}.`
);

static ERROR_RepeatedBegin = SevError | 0x073;
static Error_RepeatedBegin = () => m(this.ERROR_RepeatedBegin, `Begin has already been set`);
Expand All @@ -549,7 +557,10 @@ export class KmnCompilerMessages {
static Error_NonBMPCharactersNotSupportedInKeySection = () => m(this.ERROR_NonBMPCharactersNotSupportedInKeySection, `Characters with codepoints over U+FFFF are not supported in the key part of the rule`);

static ERROR_InvalidTarget = SevError | 0x07A;
static Error_InvalidTarget = () => m(this.ERROR_InvalidTarget, `Unrecognized compile target`);
static Error_InvalidTarget = (o:KmcmpLibMessageParameters) => m(
this.ERROR_InvalidTarget,
`Unrecognized compile target '${o.p?.[0]}'`
);

static ERROR_NoTargetsSpecified = SevError | 0x07B;
static Error_NoTargetsSpecified = () => m(this.ERROR_NoTargetsSpecified, `At least one compile target must be specified`);
Expand Down Expand Up @@ -584,10 +595,11 @@ export class KmnCompilerMessages {
static WARN_ReservedCharacter = SevWarn | 0x089;
static Warn_ReservedCharacter = () => m(this.WARN_ReservedCharacter, `A Unicode character was found that should not be used`);

// Note: INFO_Info is called CWARN_Info in kmn_compiler_errors.h, but should have an "info" severity
static INFO_Info = SevInfo | 0x08A;
static Info_Info = () => m(this.INFO_Info, `Information`);

static INFO_MinimumCoreEngineVersion = SevInfo | 0x08A;
static Info_MinimumCoreEngineVersion = (o:KmcmpLibMessageParameters) => m(
this.INFO_MinimumCoreEngineVersion,
`The compiler has assigned a minimum engine version of ${o.p?.[0]}.${o.p?.[1]} based on features used in this keyboard`
);

static WARN_VirtualKeyWithMnemonicLayout = SevWarn | 0x08B;
static Warn_VirtualKeyWithMnemonicLayout = () => m(this.WARN_VirtualKeyWithMnemonicLayout, `Virtual key used instead of virtual character key with a mnemonic layout`);
Expand Down Expand Up @@ -704,7 +716,10 @@ export class KmnCompilerMessages {
static Warn_KeyShouldIncludeNCaps = () => m(this.WARN_KeyShouldIncludeNCaps, `Other rules which reference this key include CAPS or NCAPS modifiers, so this rule must include NCAPS modifier to avoid inconsistent matches`);

static HINT_UnreachableRule = SevHint | 0x0AE;
static Hint_UnreachableRule = () => m(this.HINT_UnreachableRule, `This rule will never be matched as another rule takes precedence`);
static Hint_UnreachableRule = (o:KmcmpLibMessageParameters) => m(
this.HINT_UnreachableRule,
`This rule will never be matched as the rule on line ${o.p?.[0]} takes precedence`
);

static WARN_VirtualKeyInOutput = SevWarn | 0x0AF;
static Warn_VirtualKeyInOutput = () => m(this.WARN_VirtualKeyInOutput, `Virtual keys are not supported in output`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ export class KmwCompilerMessages extends KmnCompilerMessages {
static Hint_TouchLayoutUsesUnsupportedGesturesDownlevel = (o:{keyId:string}) => m(this.HINT_TouchLayoutUsesUnsupportedGesturesDownlevel,
`The touch layout uses a flick or multi-tap gesture on key ${def(o.keyId)}, which is only available on version 17.0+ of Keyman`);

static INFO_MinimumEngineVersion = SevInfo | 0x0006;
static Info_MinimumEngineVersion = (o:{version:string}) => m(this.INFO_MinimumEngineVersion,
`The compiler has assigned a minimum web engine version of ${o.version} based on features used in this keyboard`);
static INFO_MinimumWebEngineVersion = SevInfo | 0x0006;
static Info_MinimumWebEngineVersion = (o:{version:string}) => m(
this.INFO_MinimumWebEngineVersion,
`The compiler has assigned a minimum web engine version of ${o.version} based on features used in this keyboard`
);
};
2 changes: 1 addition & 1 deletion developer/src/kmc-kmn/src/kmw-compiler/kmw-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ export function WriteCompiledKeyboard(
if((fk.flags & KMX.KMXFile.KF_AUTOMATICVERSION) == KMX.KMXFile.KF_AUTOMATICVERSION) {
// Note: the KeymanWeb compiler is responsible for reporting minimum
// version for the web targets
callbacks.reportMessage(KmwCompilerMessages.Info_MinimumEngineVersion({version:minVer}));
callbacks.reportMessage(KmwCompilerMessages.Info_MinimumWebEngineVersion({version:minVer}));
}

return resultPrefix + resultMinVer + result;
Expand Down
50 changes: 20 additions & 30 deletions developer/src/kmc-kmn/test/kmw/test-kmw-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,9 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
// The min version message from the .kmn compiler is generic 208A INFO_Info;
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
// we expect only 1 of the info messages -- for the .kmx target (not 2)
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_Info).length, 1);
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_MinimumCoreEngineVersion).length, 1);

const data = new TextDecoder('utf-8').decode(result.artifacts.js.data);
assert.match(data, /KMINVER="15.0"/, `Could not find expected 'KMINVER="15.0"'`);
Expand All @@ -101,9 +100,8 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNull(result);
// The min version message from the .kmn compiler is generic 208A INFO_Info
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_Info));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_MinimumCoreEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.ERROR_TouchLayoutIdentifierRequires15));
});

Expand All @@ -113,9 +111,8 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result); // only a warning, so output is generated
// The min version message from the .kmn compiler is generic 208A INFO_Info
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_Info));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_MinimumCoreEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.WARN_ExtendedShiftFlagsNotSupportedInKeymanWeb));
});

Expand All @@ -125,10 +122,9 @@ describe('KeymanWeb Compiler', function() {
let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);

// The min version message from the .kmn compiler is generic 208A INFO_Info;
// we expect only 1 of the info messages -- for the .kmx target (not 2)
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_Info).length, 1);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_MinimumCoreEngineVersion).length, 1);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.WARN_ExtendedShiftFlagsNotSupportedInKeymanWeb));

const data = new TextDecoder('utf-8').decode(result.artifacts.js.data);
Expand All @@ -142,10 +138,9 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
// The min version message from the .kmn compiler is generic 208A INFO_Info;
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
// we expect only 1 of the info messages -- for the .kmx target (not 2)
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_Info).length, 1);
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_MinimumCoreEngineVersion).length, 1);

const data = new TextDecoder('utf-8').decode(result.artifacts.js.data);
assert.match(data, /KMINVER="14.0"/, `Could not find expected 'KMINVER="14.0"'`);
Expand All @@ -157,9 +152,8 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNull(result);
// The min version message from the .kmn compiler is generic 208A INFO_Info
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_Info));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_MinimumCoreEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.ERROR_140FeatureOnlyContextAndNotAnyWeb));
});

Expand All @@ -168,10 +162,9 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
// The min version message from the .kmn compiler is generic 208A INFO_Info;
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
// we expect only 1 of the info messages -- for the .kmx target (not 2)
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_Info).length, 1);
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_MinimumCoreEngineVersion).length, 1);

const data = new TextDecoder('utf-8').decode(result.artifacts.js.data);
assert.match(data, /KMINVER="14.0"/, `Could not find expected 'KMINVER="14.0"'`);
Expand All @@ -182,9 +175,8 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
// The min version message from the .kmn compiler is generic 208A INFO_Info
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_Info));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_MinimumCoreEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.WARN_TouchLayoutSpecialLabelOnNormalKey));
});

Expand All @@ -193,10 +185,9 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
// The min version message from the .kmn compiler is generic 208A INFO_Info;
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
// we expect only 1 of the info messages -- for the .kmx target (not 2)
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_Info).length, 1);
assert.equal(callbacks.messages.filter(item => item.code == KmnCompilerMessages.INFO_MinimumCoreEngineVersion).length, 1);

const data = new TextDecoder('utf-8').decode(result.artifacts.js.data);
assert.match(data, /KMINVER="17.0"/, `Could not find expected 'KMINVER="17.0"'`);
Expand All @@ -207,9 +198,8 @@ describe('KeymanWeb Compiler', function() {

let result = await kmnCompiler.run(filenames.source, null);
assert.isNotNull(result);
// The min version message from the .kmn compiler is generic 208A INFO_Info
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_Info));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumEngineVersion));
assert.isFalse(callbacks.hasMessage(KmnCompilerMessages.INFO_MinimumCoreEngineVersion));
assert.isFalse(callbacks.hasMessage(KmwCompilerMessages.INFO_MinimumWebEngineVersion));
assert.isTrue(callbacks.hasMessage(KmwCompilerMessages.HINT_TouchLayoutUsesUnsupportedGesturesDownlevel));
});

Expand Down
6 changes: 3 additions & 3 deletions developer/src/kmcmplib/include/kmcmplibapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ struct KMCMP_COMPILER_RESULT_EXTRA_GROUP {
};

struct KMCMP_COMPILER_RESULT_MESSAGE {
std::string message; // TODO: move to kmc-kmn, utf-8
unsigned int errorCode;
int lineNumber;
// TODO add filename
// TODO add parameters
int columnNumber;
std::string filename;
std::vector<std::string> parameters;
};

#define COMPILETARGETS_KMX 0x01
Expand Down
12 changes: 8 additions & 4 deletions developer/src/kmcmplib/src/CheckForDuplicates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ KMX_BOOL CheckForDuplicateGroup(PFILE_KEYBOARD fk, PFILE_GROUP gp) noexcept {
continue;
}
if (u16icmp(gp0->szName, gp->szName) == 0) {
snprintf(ErrExtraLIB, ERR_EXTRA_LIB_LEN, " Group '%s' declared on line %d", string_from_u16string(gp->szName).c_str(), gp0->Line);
AddCompileError(KmnCompilerMessages::ERROR_DuplicateGroup);
AddCompileError(KmnCompilerMessages::ERROR_DuplicateGroup, {
/*groupName*/ string_from_u16string(gp->szName),
/*lineNumber*/ std::to_string(gp0->Line)
});
return FALSE;
}
}
Expand All @@ -39,8 +41,10 @@ KMX_BOOL CheckForDuplicateStore(PFILE_KEYBOARD fk, PFILE_STORE sp) noexcept {
continue;
}
if (u16icmp(sp0->szName, sp->szName) == 0) {
snprintf(ErrExtraLIB, ERR_EXTRA_LIB_LEN, " Store '%s' declared on line %d", string_from_u16string(sp0->szName).c_str(), sp0->line);
AddCompileError(KmnCompilerMessages::ERROR_DuplicateStore);
AddCompileError(KmnCompilerMessages::ERROR_DuplicateStore, {
/*storeName*/ string_from_u16string(sp0->szName),
/*lineNumber*/ std::to_string(sp0->line)
});
return FALSE;
}
}
Expand Down
Loading

0 comments on commit 8cd97e5

Please sign in to comment.