From 09a1ad7b69bf7133c702bbfe760bdb604bcd9740 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 24 Jul 2024 07:35:31 +0700 Subject: [PATCH] feat(developer): add hint when index() store is longer than any() store While it does not cause any problems to have a index() store that is longer than its corresponding any() store, it often indicates a mistake, as the trailing characters in the store are ignored. Thus, adding as a hint (which can be disabled via message suppression) rather than as a warning or error. Example code: store(abc) 'abc' store(defg) 'defg' any(abc) + 'x' > index(defg, 1) c generates HINT_IndexStoreLong Fixes: #10666 --- .../src/common/include/kmn_compiler_errors.h | 16 +++++++++------- .../src/compiler/kmn-compiler-messages.ts | 10 ++++++++-- .../keyboards/hint_index_store_long.kmn | 11 +++++++++++ .../keyboards/hint_index_store_long_key.kmn | 11 +++++++++++ .../keyboards/warn_index_store_short.kmn | 11 +++++++++++ .../keyboards/warn_index_store_short_key.kmn | 11 +++++++++++ developer/src/kmc-kmn/test/test-messages.ts | 18 +++++++++++++++++- developer/src/kmcmplib/src/CompMsg.cpp | 1 + developer/src/kmcmplib/src/Compiler.cpp | 9 +++++++-- 9 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long.kmn create mode 100644 developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long_key.kmn create mode 100644 developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short.kmn create mode 100644 developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short_key.kmn diff --git a/developer/src/common/include/kmn_compiler_errors.h b/developer/src/common/include/kmn_compiler_errors.h index 87ee9c081c..062cce291e 100644 --- a/developer/src/common/include/kmn_compiler_errors.h +++ b/developer/src/common/include/kmn_compiler_errors.h @@ -49,13 +49,13 @@ #define CERR_None 0x00000000 #define CERR_EndOfFile 0x00000001 -#define CERR_BadCallParams 0x00008002 -#define CERR_CannotAllocateMemory 0x00008004 +#define CERR_BadCallParams 0x00008002 // TODO: rename to CFATAL_ +#define CERR_CannotAllocateMemory 0x00008004 // TODO: rename to CFATAL_ #define CERR_InfileNotExist 0x00004005 // #10678: reduced from fatal to error in 17.0 // #define CERR_CannotCreateOutfile 0x00004006 // #10678: reduced from fatal to error in 17.0, but unused -#define CERR_UnableToWriteFully 0x00008007 +#define CERR_UnableToWriteFully 0x00008007 // TODO: rename to CFATAL_ #define CERR_CannotReadInfile 0x00004008 // #10678: reduced from fatal to error in 17.0 -#define CERR_SomewhereIGotItWrong 0x00008009 +#define CERR_SomewhereIGotItWrong 0x00008009 // TODO: rename to CFATAL_ #define CERR_InvalidToken 0x0000400A #define CERR_InvalidBegin 0x0000400B @@ -129,7 +129,7 @@ #define CERR_InvalidEthnologueCode 0x0000404D -#define CERR_CannotCreateTempfile 0x0000804E +#define CERR_CannotCreateTempfile 0x0000804E // TODO: rename to CFATAL_ #define CERR_90FeatureOnly_IfSystemStores 0x0000404F #define CERR_IfSystemStore_NotFound 0x00004050 @@ -252,7 +252,9 @@ #define CWARN_VirtualKeyInOutput 0x000020AF -#define CERR_BufferOverflow 0x000080C0 -#define CERR_Break 0x000080C1 +#define CHINT_IndexStoreLong 0x000010B0 + +#define CERR_BufferOverflow 0x000080C0 // TODO: Rename to CFATAL_... +#define CERR_Break 0x000080C1 // TODO: Rename to CFATAL_... #endif // _kmn_compiler_errors_h diff --git a/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts b/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts index d24f397c45..f8dcac0df0 100644 --- a/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts +++ b/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts @@ -735,11 +735,17 @@ export class KmnCompilerMessages { static WARN_VirtualKeyInOutput = SevWarn | 0x0AF; static Warn_VirtualKeyInOutput = () => m(this.WARN_VirtualKeyInOutput, `Virtual keys are not supported in output`); + static HINT_IndexStoreLong = SevHint | 0x0B0; + static Hint_IndexStoreLong = () => m( + this.HINT_IndexStoreLong, + `The store referenced in index() is longer than the store referenced in any()` + ); + static FATAL_BufferOverflow = SevFatal | 0x0C0; - static Fatal_BufferOverflow = () => m(this.FATAL_BufferOverflow, `The compiler memory buffer overflowed`); + static Fatal_BufferOverflow = () => m(this.FATAL_BufferOverflow, `The compiler memory buffer overflowed`); static FATAL_Break = SevFatal | 0x0C1; - static Fatal_Break = () => m(this.FATAL_Break, `Compiler interrupted by user`); + static Fatal_Break = () => m(this.FATAL_Break, `Compiler interrupted by user`); }; /** diff --git a/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long.kmn b/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long.kmn new file mode 100644 index 0000000000..37d89ea48c --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long.kmn @@ -0,0 +1,11 @@ +store(&NAME) 'warn_index_store_short' +store(&VERSION) '10.0' + +begin unicode > use(main) + +group(main) using keys + +store(abc) 'abc' +store(defg) 'defg' + +any(abc) + 'x' > index(defg, 1) diff --git a/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long_key.kmn b/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long_key.kmn new file mode 100644 index 0000000000..7926186619 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/keyboards/hint_index_store_long_key.kmn @@ -0,0 +1,11 @@ +store(&NAME) 'warn_index_store_short_key' +store(&VERSION) '10.0' + +begin unicode > use(main) + +group(main) using keys + +store(abc) 'abc' +store(defg) 'defg' + ++ any(abc) > index(defg, 1) diff --git a/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short.kmn b/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short.kmn new file mode 100644 index 0000000000..bbe208eb68 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short.kmn @@ -0,0 +1,11 @@ +store(&NAME) 'warn_index_store_short' +store(&VERSION) '10.0' + +begin unicode > use(main) + +group(main) using keys + +store(abc) 'abc' +store(defg) 'defg' + +any(defg) + 'x' > index(abc, 1) diff --git a/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short_key.kmn b/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short_key.kmn new file mode 100644 index 0000000000..1131d595e3 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/keyboards/warn_index_store_short_key.kmn @@ -0,0 +1,11 @@ +store(&NAME) 'warn_index_store_short_key' +store(&VERSION) '10.0' + +begin unicode > use(main) + +group(main) using keys + +store(abc) 'abc' +store(defg) 'defg' + ++ any(defg) > index(abc, 1) diff --git a/developer/src/kmc-kmn/test/test-messages.ts b/developer/src/kmc-kmn/test/test-messages.ts index 1ba46033d8..ee45199b13 100644 --- a/developer/src/kmc-kmn/test/test-messages.ts +++ b/developer/src/kmc-kmn/test/test-messages.ts @@ -33,7 +33,7 @@ describe('KmnCompilerMessages', function () { 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); + assert.lengthOf(callbacks.messages, 1, `messages should have 1 entry, instead has: `+JSON.stringify(callbacks.messages,null,2)); } else { assert.lengthOf(callbacks.messages, 0, `messages should be empty, but instead got: `+JSON.stringify(callbacks.messages,null,2)); } @@ -112,4 +112,20 @@ describe('KmnCompilerMessages', function () { await testForMessage(this, ['invalid-keyboards', 'error_character_range_too_long.kmn'], KmnCompilerMessages.ERROR_CharacterRangeTooLong); }); + // WARN_IndexStoreShort + + it('should generate WARN_IndexStoreShort if a store referenced in index() is shorter than the corresponding any() store', async function() { + await testForMessage(this, ['keyboards', 'warn_index_store_short.kmn'], KmnCompilerMessages.WARN_IndexStoreShort); + callbacks.clear(); + await testForMessage(this, ['keyboards', 'warn_index_store_short_key.kmn'], KmnCompilerMessages.WARN_IndexStoreShort); + }); + + // HINT_IndexStoreLong + + it('should generate HINT_IndexStoreLong if a store referenced in index() is longer than the corresponding any() store', async function() { + await testForMessage(this, ['keyboards', 'hint_index_store_long.kmn'], KmnCompilerMessages.HINT_IndexStoreLong); + callbacks.clear(); + await testForMessage(this, ['keyboards', 'hint_index_store_long_key.kmn'], KmnCompilerMessages.HINT_IndexStoreLong); + }); + }); diff --git a/developer/src/kmcmplib/src/CompMsg.cpp b/developer/src/kmcmplib/src/CompMsg.cpp index 3313da9104..b41409b0db 100644 --- a/developer/src/kmcmplib/src/CompMsg.cpp +++ b/developer/src/kmcmplib/src/CompMsg.cpp @@ -115,6 +115,7 @@ std::map CompilerErrorMap = { { CHINT_UnreachableRule , "This rule will never be matched as another rule takes precedence"}, { CHINT_NonUnicodeFile , "Keyman Developer has detected that the file has ANSI encoding. Consider converting this file to UTF-8"}, + { CHINT_IndexStoreLong , "The store referenced in index() is longer than the store referenced in any()"}, { CWARN_TooManyWarnings , "Too many warnings or errors"}, { CWARN_OldVersion , "The keyboard file is an old version"}, diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index f11534ab2b..be83f55ac5 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -1300,8 +1300,13 @@ KMX_DWORD CheckStatementOffsets(PFILE_KEYBOARD fk, PFILE_GROUP gp, PKMX_WCHAR co int anyStore = *(q + 2) - 1; - if (xstrlen(fk->dpStoreArray[indexStore].dpString) < xstrlen(fk->dpStoreArray[anyStore].dpString)) { - AddWarning(CWARN_IndexStoreShort); //TODO: if this fails, then we return FALSE instead of an error + const int anyLength = xstrlen(fk->dpStoreArray[anyStore].dpString); + const int indexLength = xstrlen(fk->dpStoreArray[indexStore].dpString); + + if (indexLength < anyLength) { + AddWarning(CWARN_IndexStoreShort); + } else if(indexLength > anyLength) { + AddWarning(CHINT_IndexStoreLong); } } else if (*(p + 1) == CODE_CONTEXTEX) { int contextOffset = *(p + 2);