Merge pull request #16717 from keymanapp/fix/developer/16716-warn-on-vk-in-store-index-statement

fix(developer): check `any()` and `index()` for invalid use of virtual keys

developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_in_context-any.kmn

@@ -0,0 +1,11 @@
+store(&NAME) 'ERROR_VirtualKeyInContext-any'
+store(&VERSION) '9.0'
+
+begin Unicode > use(main)
+
+group(main) using keys
+
+store(keys) [NCAPS SHIFT K_S]
+
+c This should generate warning KM02074 ERROR_VirtualKeyInContext
+any(keys) + 'X' > 'x'

developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_in_context-index.kmn

@@ -0,0 +1,12 @@
+store(&NAME) 'ERROR_VirtualKeyInContext-index'
+store(&VERSION) '9.0'
+
+begin Unicode > use(main)
+
+group(main) using keys
+
+store(keys) [NCAPS SHIFT K_S]
+store(letter) 'F'
+
+c This should generate warning KM02074 ERROR_VirtualKeyInContext
+any(letter) index(keys, 1) + 'X' > 'x'

developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_in_context-notany.kmn

@@ -0,0 +1,11 @@
+store(&NAME) 'ERROR_VirtualKeyInContext-any'
+store(&VERSION) '9.0'
+
+begin Unicode > use(main)
+
+group(main) using keys
+
+store(keys) [NCAPS SHIFT K_S]
+
+c This should generate warning KM02074 ERROR_VirtualKeyInContext
+notany(keys) + 'X' > 'x'

developer/src/kmc-kmn/test/fixtures/invalid-keyboards/warn_virtual_key_in_output-index.kmn

@@ -0,0 +1,12 @@
+store(&VERSION) '10.0'
+store(&NAME) 'WARN_VirtualKeyInOutput'
+
+begin Unicode > use(main)
+
+group(main) using keys
+
+store(keys) [NCAPS SHIFT K_S]
+store(outputs) [NCAPS SHIFT K_S]
+
+c This should generate warning KM020AF WARN_VirtualKeyInOutput
++ any(keys) > index(outputs,1)

developer/src/kmc-kmn/test/kmn-compiler-messages.tests.ts

@@ -198,6 +198,9 @@ describe('KmnCompilerMessages', function () {
 
   it('should generate ERROR_VirtualKeyInContext if a virtual key is found in the context part of a rule', async function() {
     await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_in_context.kmn'], KmnCompilerMessages.ERROR_VirtualKeyInContext);
+    await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_in_context-any.kmn'], KmnCompilerMessages.ERROR_VirtualKeyInContext);
+    await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_in_context-index.kmn'], KmnCompilerMessages.ERROR_VirtualKeyInContext);
+    await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_in_context-notany.kmn'], KmnCompilerMessages.ERROR_VirtualKeyInContext);
   });
 
   // WARN_TouchLayoutUnidentifiedKey
@@ -217,6 +220,7 @@ describe('KmnCompilerMessages', function () {
 
   it('should generate WARN_VirtualKeyInOutput if a virtual key is found in the output part of a rule', async function() {
     await testForMessage(this, ['invalid-keyboards', 'warn_virtual_key_in_output.kmn'], KmnCompilerMessages.WARN_VirtualKeyInOutput);
+    await testForMessage(this, ['invalid-keyboards', 'warn_virtual_key_in_output-index.kmn'], KmnCompilerMessages.WARN_VirtualKeyInOutput);
   });
 
   // ERROR_OutsTooLong

developer/src/kmcmplib/src/Compiler.cpp

@@ -1531,7 +1531,6 @@ KMX_DWORD CheckStatementOffsets(PFILE_KEYBOARD fk, PFILE_GROUP gp, PKMX_WCHAR co
  *   Rule structure: [context] ['+' key] '>' output
  *   Context structure: [nul] [if()|baselayout()|platform()]+ [char|any|context()|deadkey()|dk()|index()|notany()|outs()]
  * Test that nul is first, then if(), baselayout(), platform() statements are before any other content.
- * Also verifies that virtual keys are not found in the context.
  */
 void CheckContextStatementPositions(PKMX_WCHAR context) {
   KMX_BOOL hadContextChar = FALSE;
@@ -1549,9 +1548,6 @@ void CheckContextStatementPositions(PKMX_WCHAR context) {
           ReportCompilerMessage(KmnCompilerMessages::WARN_IfShouldBeAtStartOfContext);
         }
         break;
-      case CODE_EXTENDED:
-        ReportCompilerMessage(KmnCompilerMessages::ERROR_VirtualKeyInContext);
-        break;
       default:
         hadContextChar = TRUE;
       }
@@ -1579,18 +1575,50 @@ KMX_DWORD CheckUseStatementsInOutput(PKMX_WCHAR output) {
   return STATUS_Success;
 }
 
+KMX_BOOL DoesStringContainVirtualKeys(PFILE_KEYBOARD fk, PKMX_WCHAR p) {
+  for (; *p; p = incxstr(p)) {
+    if (*p == UC_SENTINEL) {
+      auto code = *(p+1);
+      if(code == CODE_EXTENDED) {
+        return TRUE;
+      } else if(code == CODE_INDEX) {
+        PFILE_STORE s;
+        s = &fk->dpStoreArray[*(p + 2) - 1];
+        if(DoesStringContainVirtualKeys(fk, s->dpString)) {
+          return TRUE;
+        }
+      } else if(code == CODE_ANY || code == CODE_NOTANY) {
+        PFILE_STORE s;
+        s = &fk->dpStoreArray[*(p + 2) - 1];
+        if(DoesStringContainVirtualKeys(fk, s->dpString)) {
+          return TRUE;
+        }
+      }
+    }
+  }
+  return FALSE;
+}
+
 /**
  * Warn if output has virtual keys in it, which is not supported by Core at all,
  * but was unofficially supported, but never worked properly, in Keyman for
  * Windows for many years
  */
-KMX_DWORD CheckVirtualKeysInOutput(PKMX_WCHAR output) {
-  PKMX_WCHAR p;
-  for (p = output; *p; p = incxstr(p)) {
-    if (*p == UC_SENTINEL && *(p + 1) == CODE_EXTENDED) {
-      ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualKeyInOutput);
-      break;
-    }
+KMX_DWORD CheckVirtualKeysInOutput(PFILE_KEYBOARD fk, PKMX_WCHAR output) {
+  if(DoesStringContainVirtualKeys(fk, output)) {
+    ReportCompilerMessage(KmnCompilerMessages::WARN_VirtualKeyInOutput);
+    return STATUS_Success;
+  }
+  return STATUS_Success;
+}
+
+/**
+ * Error if context has virtual keys in it -- context can never contain keys,
+ * only characters and deadkeys.
+ */
+KMX_DWORD CheckVirtualKeysInContext(PFILE_KEYBOARD fk, PKMX_WCHAR context) {
+  if(DoesStringContainVirtualKeys(fk, context)) {
+    return KmnCompilerMessages::ERROR_VirtualKeyInContext;
   }
   return STATUS_Success;
 }
@@ -1785,6 +1813,9 @@ KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnico
 
   CheckContextStatementPositions(pklIn);
 
+  // Error if virtual keys are used in the context
+  if ((msg = CheckVirtualKeysInContext(fk, pklIn)) != STATUS_Success) return msg;
+
   // Test index and context offsets in context
   if ((msg = CheckStatementOffsets(fk, gp, pklIn, pklOut, pklKey)) != STATUS_Success) return msg;
 
@@ -1794,7 +1825,7 @@ KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnico
   }
 
   // Warn if virtual keys are used in the output, as they are unsupported by Core
-  if ((msg = CheckVirtualKeysInOutput(pklOut)) != STATUS_Success) {
+  if ((msg = CheckVirtualKeysInOutput(fk, pklOut)) != STATUS_Success) {
     return msg;
   }
 

developer/src/kmcmplib/tests/get-test-source.sh

@@ -9,11 +9,13 @@
 set -eu
 find "$1" -name '*.kmn' | \
   grep -E '(release|experimental)/([a-z0-9_]+)/([a-z0-9_]+)/source/\3\.kmn$' | \
-  grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul|basic_kbdcherp|basic_kbdolch|sil_yi'
+  grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul|basic_kbdcherp|basic_kbdolch|sil_yi|gff_geez'
 # #12623: exclude masaram_gondi due to #11806
 # #12631: exclude anii, sil_kmhmu as ico references have mismatching case
 # #12631: exclude fv_statimcets, fv_nuucaanul as these include U+2002 which is not
 #         treated as whitespace on mac arch
 # #12604: exclude basic_kbdcherp, basic_kbdolch as these do not include now necessary
 #         whitespace, see also issue #12307
-# #14746: exclude sil_yi, as this has whitespace in group name 'Unicode Group'
+# #14746: exclude sil_yi, as this has whitespace in group name 'Unicode Group'
+# #16717: exclude gff_geez, as this has vkeys referenced in store in an any()
+#         statement