Reworking how expression functions are defined (#2948)

Co-authored-by: Ole Martin Handeland <[email protected]>

src/features/expressions/index.test.ts

@@ -1,6 +1,8 @@
 import { NodeNotFoundWithoutContext } from 'src/features/expressions/errors';
+import { ExprFunctionDefinitions } from 'src/features/expressions/expression-functions';
 import { evalExpr } from 'src/features/expressions/index';
 import { ExprVal } from 'src/features/expressions/types';
+import type { AnyFuncDef } from 'src/features/expressions/expression-functions';
 import type { ExprConfig } from 'src/features/expressions/types';
 import type { ExpressionDataSources } from 'src/utils/layout/useExpressionDataSources';
 
@@ -23,49 +25,28 @@ describe('Expressions', () => {
     ).toEqual('hello world');
   });
 
-  describe('formatDate', () => {
-    it('should be able to format a date when the selected language is norwegian', () => {
-      const dataSources = {
-        formDataSelector: () => null,
-        applicationSettings: {},
-        hiddenFields: new Set<string>(),
-        instanceDataSources: {},
-        langToolsRef: { current: {} },
-        currentLanguage: 'nb',
-      } as unknown as ExpressionDataSources;
-      const node = new NodeNotFoundWithoutContext('test');
-
-      const result = evalExpr(['formatDate', '2023-10-26T13:12:38.069Z'], node, dataSources);
-      expect(result).toEqual('26.10.2023');
-    });
-
-    it('should be able to format a date when the selected language is english', () => {
-      const dataSources = {
-        formDataSelector: () => null,
-        applicationSettings: {},
-        hiddenFields: new Set<string>(),
-        instanceDataSources: {},
-        langToolsRef: { current: {} },
-        currentLanguage: 'en',
-      } as unknown as ExpressionDataSources;
-      const node = new NodeNotFoundWithoutContext('test');
-
-      const result = evalExpr(['formatDate', '2023-10-26T13:12:38.069Z'], node, dataSources);
-      expect(result).toEqual('10/26/23');
-    });
-
-    it('should be able to specify a custom format in which the date should be formatted', () => {
-      const dataSources = {
-        formDataSelector: () => null,
-        applicationSettings: {},
-        hiddenFields: new Set<string>(),
-        instanceDataSources: {},
-        langToolsRef: { current: {} },
-      } as unknown as ExpressionDataSources;
-      const node = new NodeNotFoundWithoutContext('test');
-
-      const result = evalExpr(['formatDate', '2023-10-26T13:12:38.069Z', 'dd.MM'], node, dataSources);
-      expect(result).toEqual('26.10');
+  describe('all function definitions should be valid', () => {
+    it.each(Object.keys(ExprFunctionDefinitions))('%s should have a valid function definition', (name) => {
+      const def = ExprFunctionDefinitions[name] as AnyFuncDef;
+
+      let optionalFound = false;
+      let restFound = false;
+      for (const arg of def.args) {
+        if (!optionalFound && !restFound && arg.variant === 'optional') {
+          optionalFound = true;
+        } else if (!restFound && arg.variant === 'rest') {
+          restFound = true;
+        } else if (arg.variant === 'required' && (optionalFound || restFound)) {
+          throw new Error('Required argument found after optional or rest argument');
+        } else if (arg.variant === 'optional' && restFound) {
+          throw new Error('Optional argument found after rest argument');
+        } else if (arg.variant === 'rest' && restFound) {
+          throw new Error('Multiple rest arguments found');
+        }
+      }
+
+      expect(def.returns).toBeDefined();
+      expect(def.args).toBeDefined();
     });
   });
 });

src/features/expressions/index.ts

@@ -8,7 +8,7 @@ import {
   UnknownSourceType,
   UnknownTargetType,
 } from 'src/features/expressions/errors';
-import { ExprFunctions } from 'src/features/expressions/expression-functions';
+import { ExprFunctionDefinitions, ExprFunctionImplementations } from 'src/features/expressions/expression-functions';
 import { ExprVal } from 'src/features/expressions/types';
 import type { NodeNotFoundWithoutContext } from 'src/features/expressions/errors';
 import type {
@@ -62,15 +62,15 @@ function isExpression(input: unknown): input is Expression {
     Array.isArray(input) &&
     input.length >= 1 &&
     typeof input[0] === 'string' &&
-    Object.keys(ExprFunctions).includes(input[0])
+    Object.keys(ExprFunctionDefinitions).includes(input[0])
   );
 }
 
 /**
  * Run/evaluate an expression. You have to provide your own context containing functions for looking up external values.
  */
-export function evalExpr(
-  expr: Expression | ExprValToActual | undefined,
+export function evalExpr<V extends ExprVal = ExprVal>(
+  expr: ExprValToActualOrExpr<V> | undefined,
   node: LayoutNode | LayoutPage | NodeNotFoundWithoutContext,
   dataSources: ExpressionDataSources,
   options?: EvalExprOptions,
@@ -106,7 +106,7 @@ export function evalExpr(
     ) {
       // If you have an expression that expects (for example) a true|false return value, and the actual returned result
       // is "true" (as a string), it makes sense to finally cast the value to the proper return value type.
-      return castValue(result, options.config.returnType, evalParams);
+      return exprCastValue(result, options.config.returnType, evalParams);
     }
 
     return result;
@@ -132,15 +132,17 @@ export function evalExpr(
 }
 
 export function argTypeAt(func: ExprFunction, argIndex: number): ExprVal | undefined {
-  const funcDef = ExprFunctions[func];
+  const funcDef = ExprFunctionDefinitions[func];
   const possibleArgs = funcDef.args;
-  const maybeReturn = possibleArgs[argIndex];
+  const maybeReturn = possibleArgs[argIndex]?.type;
   if (maybeReturn) {
     return maybeReturn;
   }
 
-  if (funcDef.lastArgSpreads) {
-    return possibleArgs[possibleArgs.length - 1];
+  const lastArg = funcDef.args[funcDef.args.length - 1];
+  const lastArgSpreads = lastArg?.variant === 'rest';
+  if (lastArg && lastArgSpreads) {
+    return lastArg.type;
   }
 
   return undefined;
@@ -152,24 +154,24 @@ function innerEvalExpr(params: EvaluateExpressionParams): any {
   const stringPath = stringifyPath(path);
 
   const [func, ...args] = stringPath ? dot.pick(stringPath, expr, false) : expr;
-  const returnType = ExprFunctions[func].returns;
+  const returnType = ExprFunctionDefinitions[func].returns;
 
   const computedArgs = args.map((arg: unknown, idx: number) => {
     const realIdx = idx + 1;
 
     const paramsWithNewPath = { ...params, path: [...path, `[${realIdx}]`] };
     const argValue = Array.isArray(arg) ? innerEvalExpr(paramsWithNewPath) : arg;
     const argType = argTypeAt(func, idx);
-    return castValue(argValue, argType, paramsWithNewPath);
+    return exprCastValue(argValue, argType, paramsWithNewPath);
   });
 
   const { onBeforeFunctionCall, onAfterFunctionCall } = params.callbacks;
 
-  const actualFunc = ExprFunctions[func].impl;
+  const actualFunc = ExprFunctionImplementations[func];
 
   onBeforeFunctionCall?.(path, func, computedArgs);
   const returnValue = actualFunc.apply(params, computedArgs);
-  const returnValueCasted = castValue(returnValue, returnType, params);
+  const returnValueCasted = exprCastValue(returnValue, returnType, params);
   onAfterFunctionCall?.(path, func, computedArgs, returnValueCasted);
 
   return returnValueCasted;
@@ -203,7 +205,7 @@ function valueToExprValueType(value: unknown): ExprVal {
  * This function is used to cast any value to a target type before/after it is passed
  * through a function call.
  */
-function castValue<T extends ExprVal>(
+export function exprCastValue<T extends ExprVal>(
   value: unknown,
   toType: T | undefined,
   context: EvaluateExpressionParams,
@@ -240,7 +242,7 @@ function asNumber(arg: string) {
 
 /**
  * All the types available in expressions, along with functions to cast possible values to them
- * @see castValue
+ * @see exprCastValue
  */
 export const ExprTypes: {
   [Type in ExprVal]: {

src/features/expressions/schema.test.ts

@@ -1,84 +1,85 @@
 import Ajv from 'ajv';
 import expressionSchema from 'schemas/json/layout/expression.schema.v1.json';
 
-import { ExprFunctions } from 'src/features/expressions/expression-functions';
+import { ExprFunctionDefinitions } from 'src/features/expressions/expression-functions';
 import { ExprVal } from 'src/features/expressions/types';
-import type { FuncDef } from 'src/features/expressions/expression-functions';
+import type { AnyFuncDef } from 'src/features/expressions/expression-functions';
 
-type Func = { name: string } & FuncDef<ExprVal[], ExprVal>;
+type Func = { name: string } & AnyFuncDef;
 
 describe('expression schema tests', () => {
   const functions: Func[] = [];
-  for (const name of Object.keys(ExprFunctions)) {
-    const func = ExprFunctions[name];
+  for (const name of Object.keys(ExprFunctionDefinitions)) {
+    const func = ExprFunctionDefinitions[name];
     functions.push({ name, ...func });
   }
 
-  it.each(functions)(
-    '$name should have a valid func-$name definition',
-    ({ name, args, minArguments, returns, lastArgSpreads }) => {
-      if (name === 'if') {
-        // if is a special case, we'll skip it here
-        return;
-      }
+  it.each(functions)('$name should have a valid func-$name definition', ({ name, args, returns }) => {
+    if (name === 'if') {
+      // if is a special case, we'll skip it here
+      return;
+    }
 
-      expect(expressionSchema.definitions[`func-${name}`]).toBeDefined();
-      expect(expressionSchema.definitions[`func-${name}`].type).toBe('array');
-      expect(expressionSchema.definitions[`func-${name}`].items[0]).toEqual({ const: name });
-
-      // It might be tempting to add these into the definitions to make the schema stricter and properly validate
-      // min/max arguments, but this would make the schema less useful for the user, as they would not get
-      // autocompletion in vscode until they had the minimum number of arguments.
-      expect(expressionSchema.definitions[`func-${name}`].minItems).toBe(undefined);
-      expect(expressionSchema.definitions[`func-${name}`].maxItems).toBe(undefined);
-
-      if (returns === ExprVal.Any) {
-        // At least one of the definitions should be a match
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        const allTypes: any[] = [];
-        for (const type of ['number', 'string', 'boolean']) {
-          allTypes.push(...expressionSchema.definitions[`strict-${type}`].anyOf);
-        }
-        expect(allTypes).toContainEqual({
-          $ref: `#/definitions/func-${name}`,
-        });
-      } else {
-        const returnString = exprValToString(returns);
-        expect(expressionSchema.definitions[`strict-${returnString}`].anyOf).toContainEqual({
-          $ref: `#/definitions/func-${name}`,
-        });
+    expect(expressionSchema.definitions[`func-${name}`]).toBeDefined();
+    expect(expressionSchema.definitions[`func-${name}`].type).toBe('array');
+    expect(expressionSchema.definitions[`func-${name}`].items[0]).toEqual({ const: name });
+
+    // It might be tempting to add these into the definitions to make the schema stricter and properly validate
+    // min/max arguments, but this would make the schema less useful for the user, as they would not get
+    // autocompletion in vscode until they had the minimum number of arguments.
+    expect(expressionSchema.definitions[`func-${name}`].minItems).toBe(undefined);
+    expect(expressionSchema.definitions[`func-${name}`].maxItems).toBe(undefined);
+
+    if (returns === ExprVal.Any) {
+      // At least one of the definitions should be a match
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const allTypes: any[] = [];
+      for (const type of ['number', 'string', 'boolean']) {
+        allTypes.push(...expressionSchema.definitions[`strict-${type}`].anyOf);
       }
+      expect(allTypes).toContainEqual({
+        $ref: `#/definitions/func-${name}`,
+      });
+    } else {
+      const returnString = exprValToString(returns);
+      expect(expressionSchema.definitions[`strict-${returnString}`].anyOf).toContainEqual({
+        $ref: `#/definitions/func-${name}`,
+      });
+    }
 
-      if (minArguments === undefined) {
-        expect(expressionSchema.definitions[`func-${name}`].items.length).toBe(args.length + 1);
-      } else {
-        expect(expressionSchema.definitions[`func-${name}`].items.length).toBeGreaterThanOrEqual(minArguments + 1);
-      }
+    const minArguments = args.findIndex((arg) => arg.variant === 'optional' || arg.variant === 'rest');
+    const lastArgSpreads = args[args.length - 1]?.variant === 'rest';
 
-      if (lastArgSpreads) {
-        const lastArg = args[args.length - 1];
-        expect(expressionSchema.definitions[`func-${name}`].additionalItems).toEqual({ $ref: exprValToDef(lastArg) });
-      } else {
-        expect(expressionSchema.definitions[`func-${name}`].additionalItems).toBe(false);
-      }
-    },
-  );
+    if (minArguments === -1) {
+      expect(expressionSchema.definitions[`func-${name}`].items.length).toBe(args.length + 1);
+    } else {
+      expect(expressionSchema.definitions[`func-${name}`].items.length).toBeGreaterThanOrEqual(minArguments + 1);
+    }
+
+    if (lastArgSpreads) {
+      const lastArg = args[args.length - 1].type;
+      expect(expressionSchema.definitions[`func-${name}`].additionalItems).toEqual({ $ref: exprValToDef(lastArg) });
+    } else {
+      expect(expressionSchema.definitions[`func-${name}`].additionalItems).toBe(false);
+    }
+  });
 
   const ajv = new Ajv({ strict: false });
   const validate = ajv.compile(expressionSchema);
 
-  it.each(functions)('$name should validate against generated function calls', ({ name, args, lastArgSpreads }) => {
+  it.each(functions)('$name should validate against generated function calls', ({ name, args }) => {
     if (name === 'if') {
       // if is a special case, we'll skip it here
       return;
     }
 
     const funcDef = expressionSchema.definitions[`func-${name}`];
+    const lastArgSpreads = args[args.length - 1]?.variant === 'rest';
 
     // With exactly the right number of arguments
-    const funcCall = [name, ...args.map(exprValToString)];
+    const funcCall = [name, ...args.map((arg) => exprValToString(arg.type))];
     if (lastArgSpreads) {
-      funcCall.push(...args.map(exprValToString));
+      funcCall.push(...args.map((arg) => exprValToString(arg.type)));
     }
 
     // Use enum value, if defined in schema
@@ -105,7 +106,7 @@ describe('expression schema tests', () => {
     expect(validMinArguments).toBe(true);
 
     // With too many arguments
-    const funcCallExtra = [name, ...args.map(exprValToString), 'extra'];
+    const funcCallExtra = [name, ...args.map((arg) => exprValToString(arg.type)), 'extra'];
     const validExtra = validate(funcCallExtra);
     if (lastArgSpreads) {
       // This always validates, because the last argument spread
@@ -120,6 +121,14 @@ describe('expression schema tests', () => {
     const valid = validate(['invalid_function']);
     expect(valid).toBe(false);
   });
+
+  it('no other function definitions should be present', () => {
+    const hardcodedAllowed = new Set(['func-if-without-else', 'func-if-with-else']);
+    const functionDefs = Object.keys(expressionSchema.definitions).filter((key) => key.startsWith('func-'));
+    const validFunctionDefs = new Set(Object.keys(ExprFunctionDefinitions).map((name) => `func-${name}`));
+    const unknownFunctionDefs = functionDefs.filter((key) => !validFunctionDefs.has(key) && !hardcodedAllowed.has(key));
+    expect(unknownFunctionDefs).toEqual([]);
+  });
 });
 
 function exprValToString(val: ExprVal): string {

src/features/expressions/shared-functions-exist.test.ts

@@ -1,4 +1,4 @@
-import { ExprFunctions } from 'src/features/expressions/expression-functions';
+import { ExprFunctionDefinitions } from 'src/features/expressions/expression-functions';
 import { getSharedTests } from 'src/features/expressions/shared';
 import { implementsDisplayData } from 'src/layout';
 import { getComponentConfigs } from 'src/layout/components.generated';
@@ -34,7 +34,7 @@ describe('Shared function tests should exist', () => {
   });
 
   describe('Function tests', () => {
-    for (const exprFunc of Object.keys(ExprFunctions)) {
+    for (const exprFunc of Object.keys(ExprFunctionDefinitions)) {
       it(`Expression function ${exprFunc} should have a test folder`, () => {
         expect(sharedTests?.content.find(({ folderName }) => folderName === exprFunc)).toBeTruthy();
       });

src/features/expressions/shared-functions.test.tsx

@@ -8,7 +8,7 @@ import { getInstanceDataMock } from 'src/__mocks__/getInstanceDataMock';
 import { getSubFormLayoutSetMock } from 'src/__mocks__/getLayoutSetsMock';
 import { getProcessDataMock } from 'src/__mocks__/getProcessDataMock';
 import { getProfileMock } from 'src/__mocks__/getProfileMock';
-import { getSharedTests } from 'src/features/expressions/shared';
+import { getSharedTests, type SharedTestFunctionContext } from 'src/features/expressions/shared';
 import { ExprVal } from 'src/features/expressions/types';
 import { ExprValidation } from 'src/features/expressions/validation';
 import { useExternalApis } from 'src/features/externalApi/useExternalApi';
@@ -17,7 +17,6 @@ import { fetchApplicationMetadata, fetchProcessState } from 'src/queries/queries
 import { renderWithNode } from 'src/test/renderWithProviders';
 import { useEvalExpression } from 'src/utils/layout/generator/useEvalExpression';
 import { useExpressionDataSources } from 'src/utils/layout/useExpressionDataSources';
-import type { SharedTestFunctionContext } from 'src/features/expressions/shared';
 import type { ExprPositionalArgs, ExprValToActualOrExpr, ExprValueArgs } from 'src/features/expressions/types';
 import type { ExternalApisResult } from 'src/features/externalApi/useExternalApi';
 import type { RoleResult } from 'src/features/useCurrentPartyRoles';
@@ -267,7 +266,7 @@ describe('Expressions shared function tests', () => {
             valueArguments={valueArguments}
           />
         ),
-        inInstance: !!instance,
+        inInstance: !!instance || !!dataModels,
         queries: {
           fetchLayoutSets: async () => ({
             sets: [{ id: 'layout-set', dataType: 'default', tasks: ['Task_1'] }, getSubFormLayoutSetMock()],

src/features/expressions/shared-tests/functions/if/fail-else-as-expr.json

@@ -0,0 +1,5 @@
+{
+  "name": "Should fail when given an expression instead of 'else'",
+  "expression": ["if", true, 123.456, ["if", true, "else", "else", "not-else?"], 654.321],
+  "expectsFailure": "Expected third argument to be \"else\""
+}