Skip to content

Commit

Permalink
(fix) Correctly extract function names in expression evaluation excep…
Browse files Browse the repository at this point in the history
…tions (#1224)
  • Loading branch information
samuelmale authored Dec 6, 2024
1 parent 6398e3d commit fef1824
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
33 changes: 30 additions & 3 deletions packages/framework/esm-expression-evaluator/src/evaluator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('OpenMRS Expression Evaluator', () => {
expect(evaluate('1 + 1')).toBe(2);
});

it('Should support mulitplication', () => {
it('Should support multiplication', () => {
expect(evaluate('1 * 2')).toBe(2);
});

Expand Down Expand Up @@ -93,7 +93,7 @@ describe('OpenMRS Expression Evaluator', () => {
expect(evaluate('a ? 1 : 2', { a: false })).toBe(2);
});

it('Should support hexdecimal', () => {
it('Should support hexadecimal', () => {
expect(evaluate('0xff')).toBe(255);
});

Expand Down Expand Up @@ -154,7 +154,7 @@ describe('OpenMRS Expression Evaluator', () => {
expect(evaluate(exp)).toBe(2);
});

it('Should not support variable assignement', () => {
it('Should not support variable assignment', () => {
expect(() => evaluate('var a = 1; a')).toThrow();
});

Expand Down Expand Up @@ -198,4 +198,31 @@ describe('OpenMRS Expression Evaluator', () => {
),
).toBe(true);
});

it('Should throw an error with correct message for non-existent function', () => {
expect(() => {
evaluate('api.nonExistingFunction()', { api: {} });
}).toThrow('No function named nonExistingFunction is defined in this context');

// nested ref
expect(() => {
evaluate('objectWrapper.path.deepNested()', {
objectWrapper: {
path: {
deepNested: undefined,
},
},
});
}).toThrow('No function named deepNested is defined in this context');
});

it('Should throw an error with correct message for non-callable targets', () => {
expect(() => {
evaluate('objectWrapper.path()', {
objectWrapper: {
path: {},
},
});
}).toThrow('path is not a function');
});
});
17 changes: 14 additions & 3 deletions packages/framework/esm-expression-evaluator/src/evaluator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @category Utility */
import jsep from 'jsep';
import jsep, { type Expression, type MemberExpression } from 'jsep';
import jsepArrow, { type ArrowExpression } from '@jsep-plugin/arrow';
import jsepNew, { type NewExpression } from '@jsep-plugin/new';
import jsepNumbers from '@jsep-plugin/numbers';
Expand Down Expand Up @@ -467,9 +467,9 @@ function visitCallExpression(expression: jsep.CallExpression, context: Evaluatio
let callee = visitExpression(expression.callee, context);

if (!callee) {
throw `No function named ${expression.callee} is defined in this context`;
throw `No function named ${getCallTargetName(expression.callee)} is defined in this context`;
} else if (!(typeof callee === 'function')) {
throw `${expression.callee} is not a function`;
throw `${getCallTargetName(expression.callee)} is not a function`;
}

return callee(...args);
Expand Down Expand Up @@ -722,3 +722,14 @@ function isValidVariableType(val: unknown): val is VariablesMap['a'] {

return false;
}

function getCallTargetName(expression: Expression) {
if (!expression) {
return null;
}
if (expression.type === 'MemberExpression') {
return (expression.property as MemberExpression)?.name;
}
// identifier expression
return expression.name;
}

0 comments on commit fef1824

Please sign in to comment.