fix(FEEL): vars in filters are properly parsed

packages/form-js-viewer/src/features/expression-language/variableExtractionHelpers.js

@@ -1,6 +1,6 @@
 import { parseExpression, parseUnaryTests } from 'feelin';
 
-export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options = {}) => {
+export const getFlavouredFeelVariableNames = (feelString, feelFlavour = 'expression', options = {}) => {
 
   const {
     depth = 0,
@@ -13,7 +13,7 @@ export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options =
 
   const simpleExpressionTree = _buildSimpleFeelStructureTree(tree, feelString);
 
-  return (function _unfoldVariables(node) {
+  const variables = (function _unfoldVariables(node) {
 
     if (node.name === 'PathExpression') {
 
@@ -29,14 +29,19 @@ export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options =
 
     // for any other kind of node, traverse its children and flatten the result
     if (node.children) {
-      return node.children.reduce((acc, child) => {
+      const variables = node.children.reduce((acc, child) => {
         return acc.concat(_unfoldVariables(child));
       }, []);
+
+      // if we are within a filter context, we need to remove the item variable as it is used for iteration there
+      return node.name === 'FilterContext' ? variables.filter((name) => name !== 'item') : variables;
     }
 
     return [];
   })(simpleExpressionTree);
 
+  return [ ...new Set(variables) ];
+
 };
 
 
@@ -163,5 +168,54 @@ const _buildSimpleFeelStructureTree = (parseTree, feelString) => {
     }
   });
 
-  return stack[0].children[0];
-};
+  return _extractFilterExpressions(stack[0].children[0]);
+};
+
+/**
+ * Restructure the tree in such a way to bring filters (which create new contexts) to the root of the tree.
+ * This is done to simplify the extraction of variables and match the context hierarchy.
+ */
+const _extractFilterExpressions = (tree) => {
+
+  const flattenedExpressionTree = {
+    name: 'Root',
+    children: [ tree ]
+  };
+
+  const iterate = (node) => {
+
+    if (node.children) {
+      for (let x = 0; x < node.children.length; x++) {
+
+        if (node.children[x].name === 'FilterExpression') {
+
+          const filterTarget = node.children[x].children[0];
+          const filterExpression = node.children[x].children[2];
+
+          // bypass the filter expression
+          node.children[x] = filterTarget;
+
+          const taggedFilterExpression = {
+            name: 'FilterContext',
+            children: [ filterExpression ]
+          };
+
+          // append the filter expression to the root
+          flattenedExpressionTree.children.push(taggedFilterExpression);
+
+          // recursively iterate the expression
+          iterate(filterExpression);
+
+        } else {
+          iterate(node.children[x]);
+        }
+      }
+
+    }
+
+  };
+
+  iterate(tree);
+
+  return flattenedExpressionTree;
+};

packages/form-js-viewer/src/util/index.js

@@ -143,14 +143,12 @@ export function getSchemaVariables(schema, expressionLanguage = new FeelExpressi
       const property = get(component, prop.split('.'));
 
       if (property && !expressionLanguage.isExpression(property) && templating.isTemplate(property)) {
-
         const templateVariables = templating.getVariableNames(property);
-
         variables = [ ...variables, ...templateVariables ];
       }
     });
 
-    return variables;
+    return variables.filter(variable => variable !== undefined || variable !== null);
   }, []);
 
   // remove duplicates

packages/form-js-viewer/test/spec/features/expression-language/variableExtractionHelpers.spec.js

@@ -0,0 +1,96 @@
+import { getFlavouredFeelVariableNames } from '../../../../src/features/expression-language/variableExtractionHelpers';
+
+describe('getFlavouredFeelVariableNames', () => {
+
+  expectVariables('SimpleAddition', '2 + 2', []);
+
+  expectVariables('SingleVariable', 'variable1 + 2', [ 'variable1' ]);
+
+  expectVariables('MultipleVariables', 'variable1 + variable2', [ 'variable1', 'variable2' ]);
+
+  expectVariables('ExpressionWithConstants', 'variable1 + 2*3/4 - 5', [ 'variable1' ]);
+
+  expectVariables('MultipleOccurrences', 'variable1 + variable1 - variable1 * variable1 / variable1', [ 'variable1' ]);
+
+  expectVariables('ExpressionWithList', '[variable1, variable2, variable3]', [ 'variable1', 'variable2', 'variable3' ]);
+
+  expectVariables('VariableInContextEntry', '{entry1: variable1}', [ 'variable1' ]);
+
+  expectVariables('VariableInNestedContext', '{entry1: {entry2: variable1}}', [ 'variable1' ]);
+
+  expectVariables('VariableInBooleanExpression', 'variable1 > variable2', [ 'variable1', 'variable2' ]);
+
+  expectVariables('VariableInIfExpression', 'if variable1 > variable2 then "yes" else "no"', [ 'variable1', 'variable2' ]);
+
+  expectVariables('VariableInInterval', '[variable1..variable2]', [ 'variable1', 'variable2' ]);
+
+  expectVariables('VariableInDateFunction', 'date(variable1)', [ 'variable1' ]);
+
+  expectVariables('VariableInTimeFunction', 'time(variable1)', [ 'variable1' ]);
+
+  expectVariables('VariableInDateTimeFunction', 'date and time(variable1)', [ 'variable1' ]);
+
+  expectVariables('NestedList', '[[variable1, variable2], [variable3, variable4]]', [ 'variable1', 'variable2', 'variable3', 'variable4' ]);
+
+  expectVariables('ListInContextEntry', '{entry1: [variable1, variable2]}', [ 'variable1', 'variable2' ]);
+
+  expectVariables('VariableInUnaryTest', 'variable1 in (variable2..variable3)', [ 'variable1', 'variable2', 'variable3' ]);
+
+  expectVariables('ArrayAccessorSingleVariable', 'variable1[1]', [ 'variable1' ]);
+
+  expectVariables('ArrayAccessorMultipleVariables', 'variable1[variable2]', [ 'variable1', 'variable2' ]);
+
+  expectVariables('ArrayAccessorNestedVariables', 'variable1[variable2[1]]', [ 'variable1', 'variable2' ]);
+
+  expectVariables('NestedArrayAccessor', 'variable1[variable2[1]][2]', [ 'variable1', 'variable2' ]);
+
+  expectVariables('VariableInArrayFilter', 'variable1[item > 3]', [ 'variable1' ]);
+
+  expectVariables('ComplexArrayFilter', 'variable1[item.a > 3 and item.b < 5]', [ 'variable1' ]);
+
+
+  // TODO(@skaiir) these tests should ideally pass, but right now our variable extraction logic doesn't ignore certain keywords
+  // https://github.com/bpmn-io/form-js/issues/710
+
+  describe.skip('Oversensitive', () => {
+
+    expectVariables('FunctionWithConstants', 'sum(1, 2, 3)', []);
+
+    expectVariables('FunctionWithVariable', 'sum(variable1, 2, 3)', [ 'variable1' ]);
+
+    expectVariables('FunctionWithMultipleVariables', 'sum(variable1, variable2, variable3)', [ 'variable1', 'variable2', 'variable3' ]);
+
+    expectVariables('FunctionNestedInFunction', 'sum(product(variable1, variable2), sum(variable3, variable4))', [ 'variable1', 'variable2', 'variable3', 'variable4' ]);
+
+    expectVariables('FunctionInList', '[sum(variable1, variable2), product(variable3, variable4)]', [ 'variable1', 'variable2', 'variable3', 'variable4' ]);
+
+    expectVariables('VariableInFunction', 'sum(variable1, variable2)', [ 'variable1', 'variable2' ]);
+
+    expectVariables('NestedFunctions', 'sum(product(variable1, variable2), variable3)', [ 'variable1', 'variable2', 'variable3' ]);
+
+    expectVariables('ContextEntryWithFunction', '{entry1: sum(variable1, variable2)}', [ 'variable1', 'variable2' ]);
+
+    expectVariables('FunctionInContextEntry', '{entry1: sum(variable1, variable2), entry2: product(variable3, variable4)}', [ 'variable1', 'variable2', 'variable3', 'variable4' ]);
+
+    expectVariables('VariableInUnaryNotTest', 'variable1 not in (variable2..variable3)', [ 'variable1', 'variable2', 'variable3' ]);
+
+    expectVariables('VariableInQuantifiedExpression', 'some x in variable1 satisfies x > variable2', [ 'variable1', 'variable2' ]);
+
+    expectVariables('VariableInInstanceOfTest', 'variable1 instance of tNumber', [ 'variable1' ]);
+
+    expectVariables('VariableInForExpression', 'for x in variable1 return x > variable2', [ 'variable1', 'variable2' ]);
+
+    expectVariables('ArrayAccessorInFunction', 'sum(variable1[1], variable2[2])', [ 'variable1', 'variable2' ]);
+
+    expectVariables('ArrayFilterInFunction', 'sum(variable1[item > 3])', [ 'variable1' ]);
+
+  });
+
+});
+
+
+// helpers ///////////////
+
+function expectVariables(name, feel, variables) {
+  it(name, () => expect(getFlavouredFeelVariableNames(feel)).to.eql(variables));
+}

packages/form-js-viewer/test/spec/ships-example.json

@@ -0,0 +1,19 @@
+{
+  "components": [
+    {
+      "text": "#### Ship Name:\n {{shipsForSale[item.name = selectedShip].name}}\n#### Ship Description: \n {{shipsForSale[item.name = selectedShip].description}}\n#### Ship Price: \n {{shipsForSale[item.name = selectedShip].purchasePrice}}\n#### Frame:\n {{shipsForSale[item.name = selectedShip].frame.name}}\n#### Reactor:\n {{shipsForSale[item.name = selectedShip].reactor.name}}\n#### Engine:\n {{shipsForSale[item.name = selectedShip].engine.name}}\n#### Moduels:\n {{shipsForSale[item.name = selectedShip].modules.name}}\n",
+      "type": "text",
+      "layout": {
+        "row": "Row_1oxhkbh",
+        "columns": null
+      },
+      "id": "Field_10y8si5",
+      "conditional": {
+        "hide": "=selectedShip = null"
+      }
+    }
+  ],
+  "schemaVersion": 9,
+  "type": "default",
+  "id": "Form_0ysvmoe"
+}

packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js

@@ -15,6 +15,7 @@ import adornersSchema from '../appearance.json';
 import imagesSchema from '../images.json';
 import valuesExpressionSchema from '../valuesExpression.json';
 import validateSchema from '../validate.json';
+import shipsExampleSchema from '../ships-example.json';
 
 describe('util/getSchemaVariables', () => {
 
@@ -148,4 +149,15 @@ describe('util/getSchemaVariables', () => {
     ]);
   });
 
+
+  it('should include variables in ships example', () => {
+
+    const variables = getSchemaVariables(shipsExampleSchema);
+
+    expect(variables).to.eql([
+      'selectedShip',
+      'shipsForSale'
+    ]);
+  });
+
 });