[Lens] Formula: add validation for multiple field/metrics

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx

@@ -887,6 +887,58 @@ invalid: "
       ).toEqual(['The operation average does not accept any parameter']);
     });
 
+    it('returns an error if first argument type is passed multiple times', () => {
+      const formulas = [
+        'average(bytes, bytes)',
+        'average(bytes, kql="category.keyword: *", bytes)',
+        'moving_average(average(bytes), average(bytes))',
+        'moving_average(average(bytes), kql="category.keyword: *", average(bytes))',
+        'moving_average(average(bytes, bytes), count())',
+        'moving_average(moving_average(average(bytes, bytes), count(), count()))',
+      ];
+      for (const formula of formulas) {
+        expect(
+          formulaOperation.getErrorMessage!(
+            getNewLayerWithFormula(formula),
+            'col1',
+            indexPattern,
+            operationDefinitionMap
+          )
+        ).toEqual(
+          expect.arrayContaining([
+            expect.stringMatching(
+              /The operation (moving_average|average) in the Formula requires a single (field|metric), found \d+/
+            ),
+          ])
+        );
+      }
+    });
+
+    it('returns an error if a metric is passed to a field-only operation', () => {
+      const formulas = [
+        'average(bytes, count())',
+        'average(bytes, kql="category.keyword: *", count())',
+        'average(bytes, kql="category.keyword: *", count(), count())',
+        'moving_average(average(bytes), bytes)',
+      ];
+      for (const formula of formulas) {
+        expect(
+          formulaOperation.getErrorMessage!(
+            getNewLayerWithFormula(formula),
+            'col1',
+            indexPattern,
+            operationDefinitionMap
+          )
+        ).toEqual(
+          expect.arrayContaining([
+            expect.stringMatching(
+              /The operation (moving_average|average) in the Formula does not support (metric|field) parameters, found:/
+            ),
+          ])
+        );
+      }
+    });
+
     it('returns an error if the parameter passed to an operation is of the wrong type', () => {
       expect(
         formulaOperation.getErrorMessage!(

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts

@@ -431,7 +431,7 @@ ${'`square(last_value(length))`'}
   },
 };
 
-export function isMathNode(node: TinymathAST) {
+export function isMathNode(node: TinymathAST | string) {
   return isObject(node) && node.type === 'function' && tinymathFunctions[node.name];
 }
 

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts

@@ -7,7 +7,7 @@
 
 import { isObject, partition } from 'lodash';
 import { i18n } from '@kbn/i18n';
-import { parse, TinymathLocation } from '@kbn/tinymath';
+import { parse, TinymathLocation, TinymathVariable } from '@kbn/tinymath';
 import type { TinymathAST, TinymathFunction, TinymathNamedArgument } from '@kbn/tinymath';
 import { esKuery, esQuery } from '../../../../../../../../src/plugins/data/public';
 import {
@@ -63,6 +63,14 @@ interface ValidationErrors {
     message: string;
     type: {};
   };
+  tooManyFirstArguments: {
+    message: string;
+    type: { operation: string; type: string; count: number; text: string };
+  };
+  wrongArgument: {
+    message: string;
+    type: { operation: string; text: string; type: string };
+  };
 }
 
 type ErrorTypes = keyof ValidationErrors;
@@ -276,6 +284,20 @@ function getMessageFromId<K extends ErrorTypes>({
         defaultMessage: 'Use only one of kql= or lucene=, not both',
       });
       break;
+    case 'tooManyFirstArguments':
+      message = i18n.translate('xpack.lens.indexPattern.formulaOperationTooManyFirstArguments', {
+        defaultMessage:
+          'The operation {operation} in the Formula requires a single {type}, found {count}: {text}',
+        values: { operation: out.operation, text: out.text, count: out.count, type: out.type },
+      });
+      break;
+    case 'wrongArgument':
+      message = i18n.translate('xpack.lens.indexPattern.formulaOperationwrongArgument', {
+        defaultMessage:
+          'The operation {operation} in the Formula does not support {type} parameters, found: {text}',
+        values: { operation: out.operation, text: out.text, type: out.type },
+      });
+      break;
     // case 'mathRequiresFunction':
     //   message = i18n.translate('xpack.lens.indexPattern.formulaMathRequiresFunctionLabel', {
     //     defaultMessage; 'The function {name} requires an Elasticsearch function',
@@ -531,7 +553,7 @@ function runFullASTValidation(
     } else {
       if (nodeOperation.input === 'field') {
         if (shouldHaveFieldArgument(node)) {
-          if (!isFirstArgumentValidType(firstArg, 'variable')) {
+          if (!isArgumentValidType(firstArg, 'variable')) {
             if (isMathNode(firstArg)) {
               errors.push(
                 getMessageFromId({
@@ -561,6 +583,37 @@ function runFullASTValidation(
                 })
               );
             }
+          } else {
+            const fields = variables.filter(
+              (arg) => isArgumentValidType(arg, 'variable') && !isMathNode(arg)
+            );
+            if (fields.length > 1) {
+              errors.push(
+                getMessageFromId({
+                  messageId: 'tooManyFirstArguments',
+                  values: {
+                    operation: node.name,
+                    type: 'field',
+                    text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '),
+                    count: fields.length,
+                  },
+                  locations: node.location ? [node.location] : [],
+                })
+              );
+            }
+          }
+          if (functions.length) {
+            errors.push(
+              getMessageFromId({
+                messageId: 'wrongArgument',
+                values: {
+                  operation: node.name,
+                  text: (functions as TinymathFunction[]).map(({ text }) => text).join(', '),
+                  type: 'metric',
+                },
+                locations: node.location ? [node.location] : [],
+              })
+            );
           }
         } else {
           // Named arguments only
@@ -603,7 +656,7 @@ function runFullASTValidation(
         // What about fn(7 + 1)? We may want to allow that
         // In general this should be handled down the Esaggs route rather than here
         if (
-          !isFirstArgumentValidType(firstArg, 'function') ||
+          !isArgumentValidType(firstArg, 'function') ||
           (isMathNode(firstArg) && validateMathNodes(firstArg, missingVariablesSet).length)
         ) {
           errors.push(
@@ -621,6 +674,21 @@ function runFullASTValidation(
               locations: node.location ? [node.location] : [],
             })
           );
+        } else {
+          if (functions.length > 1) {
+            errors.push(
+              getMessageFromId({
+                messageId: 'tooManyFirstArguments',
+                values: {
+                  operation: node.name,
+                  type: 'metric',
+                  text: (functions as TinymathFunction[]).map(({ text }) => text).join(', '),
+                  count: functions.length,
+                },
+                locations: node.location ? [node.location] : [],
+              })
+            );
+          }
         }
         if (!canHaveParams(nodeOperation) && namedArguments.length) {
           errors.push(
@@ -633,6 +701,22 @@ function runFullASTValidation(
             })
           );
         } else {
+          const fields = variables.filter(
+            (arg) => isArgumentValidType(arg, 'variable') && !isMathNode(arg)
+          );
+          if (fields.length) {
+            errors.push(
+              getMessageFromId({
+                messageId: 'wrongArgument',
+                values: {
+                  operation: node.name,
+                  text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '),
+                  type: 'field',
+                },
+                locations: node.location ? [node.location] : [],
+              })
+            );
+          }
           const argumentsErrors = validateNameArguments(
             node,
             nodeOperation,
@@ -736,7 +820,7 @@ export function hasFunctionFieldArgument(type: string) {
   return !['count'].includes(type);
 }
 
-export function isFirstArgumentValidType(arg: TinymathAST, type: TinymathNodeTypes['type']) {
+export function isArgumentValidType(arg: TinymathAST | string, type: TinymathNodeTypes['type']) {
   return isObject(arg) && arg.type === type;
 }