Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix not being able to use and and or inside a function definition #3150

Merged
merged 7 commits into from
Feb 8, 2024
9 changes: 5 additions & 4 deletions docs/expressions/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,17 @@ allowing the function to process the arguments in a customized way. Raw
functions are called as:

```
rawFunction(args: Node[], math: Object, scope: Object)
rawFunction(args: Node[], math: Object, scope: Map)
```

Where :

- `args` is an Array with nodes of the parsed arguments.
- `math` is the math namespace against which the expression was compiled.
- `scope` is a shallow _copy_ of the `scope` object provided when evaluating
the expression, optionally extended with nested variables like a function
parameter `x` of in a custom defined function like `f(x) = x^2`.
- `scope` is a `Map` containing the variables defined in the scope passed
via `evaluate(scope)`. In case of using a custom defined function like
`f(x) = rawFunction(x) ^ 2`, the scope passed to `rawFunction` also contains
the current value of parameter `x`.

Raw functions must be imported in the `math` namespace, as they need to be
processed at compile time. They are not supported when passed via a scope
Expand Down
3 changes: 2 additions & 1 deletion src/expression/node/OperatorNode.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { isNode, isConstantNode, isOperatorNode, isParenthesisNode } from '../../utils/is.js'
import { map } from '../../utils/array.js'
import { createSubScope } from '../../utils/scope.js'
import { escape } from '../../utils/string.js'
import { getSafeProperty, isSafeMethod } from '../../utils/customs.js'
import { getAssociativity, getPrecedence, isAssociativeWith, properties } from '../operators.js'
Expand Down Expand Up @@ -309,7 +310,7 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
// "raw" evaluation
const rawArgs = this.args
return function evalOperatorNode (scope, args, context) {
return fn(rawArgs, math, scope)
return fn(rawArgs, math, createSubScope(scope, args))
}
} else if (evalArgs.length === 1) {
const evalArg0 = evalArgs[0]
Expand Down
9 changes: 5 additions & 4 deletions src/expression/transform/utils/compileInlineExpression.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { isSymbolNode } from '../../../utils/is.js'
import { createSubScope } from '../../../utils/scope.js'
import { PartitionedMap } from '../../../utils/map.js'

/**
* Compile an inline expression like "x > 0"
* @param {Node} expression
* @param {Object} math
* @param {Object} scope
* @param {Map} scope
* @return {function} Returns a function with one argument which fills in the
* undefined variable (like "x") and evaluates the expression
*/
Expand All @@ -23,10 +23,11 @@ export function compileInlineExpression (expression, math, scope) {

// create a test function for this equation
const name = symbol.name // variable name
const subScope = createSubScope(scope)
const argsScope = new Map()
const subScope = new PartitionedMap(scope, argsScope, new Set([name]))
const eq = expression.compile()
return function inlineExpression (x) {
subScope.set(name, x)
argsScope.set(name, x)
return eq.evaluate(subScope)
}
}
121 changes: 120 additions & 1 deletion src/utils/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class ObjectWrappingMap {
}

keys () {
return Object.keys(this.wrappedObject)
return Object.keys(this.wrappedObject).values()
}

get (key) {
Expand All @@ -30,6 +30,125 @@ export class ObjectWrappingMap {
has (key) {
return hasSafeProperty(this.wrappedObject, key)
}

entries () {
return mapIterator(this.keys(), key => [key, this.get(key)])
}

forEach (callback) {
for (const key of this.keys()) {
callback(this.get(key), key, this)
}
}

delete (key) {
delete this.wrappedObject[key]
}

clear () {
for (const key of this.keys()) {
this.delete(key)
}
}

get size () {
return Object.keys(this.wrappedObject).length
}
}

/**
* Create a map with two partitions: a and b.
* The set with bKeys determines which keys/values are read/written to map b,
* all other values are read/written to map a
*
* For example:
*
* const a = new Map()
* const b = new Map()
* const p = new PartitionedMap(a, b, new Set(['x', 'y']))
*
* In this case, values `x` and `y` are read/written to map `b`,
* all other values are read/written to map `a`.
*/
export class PartitionedMap {
/**
* @param {Map} a
* @param {Map} b
* @param {Set} bKeys
*/
constructor (a, b, bKeys) {
this.a = a
this.b = b
this.bKeys = bKeys
}

get (key) {
return this.bKeys.has(key)
? this.b.get(key)
: this.a.get(key)
}

set (key, value) {
if (this.bKeys.has(key)) {
this.b.set(key, value)
} else {
this.a.set(key, value)
}
return this
}

has (key) {
return this.b.has(key) || this.a.has(key)
}

keys () {
return new Set([
...this.a.keys(),
...this.b.keys()
])[Symbol.iterator]()
}

entries () {
return mapIterator(this.keys(), key => [key, this.get(key)])
}

forEach (callback) {
for (const key of this.keys()) {
callback(this.get(key), key, this)
}
}

delete (key) {
return this.bKeys.has(key)
? this.b.delete(key)
: this.a.delete(key)
}

clear () {
this.a.clear()
this.b.clear()
}

get size () {
return [...this.keys()].length
}
}

/**
* Create a new iterator that maps over the provided iterator, applying a mapping function to each item
*/
function mapIterator (it, callback) {
return {
next: () => {
const n = it.next()
return (n.done)
? n
: {
value: callback(n.value),
done: false
}
}
}
}

/**
Expand Down
18 changes: 9 additions & 9 deletions src/utils/scope.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createEmptyMap, assign } from './map.js'
import { ObjectWrappingMap, PartitionedMap } from './map.js'

/**
* Create a new scope which can access the parent scope,
Expand All @@ -10,13 +10,13 @@ import { createEmptyMap, assign } from './map.js'
* the remaining `args`.
*
* @param {Map} parentScope
* @param {...any} args
* @returns {Map}
* @param {Object} args
* @returns {PartitionedMap}
*/
export function createSubScope (parentScope, ...args) {
if (typeof parentScope.createSubScope === 'function') {
return assign(parentScope.createSubScope(), ...args)
}

return assign(createEmptyMap(), parentScope, ...args)
export function createSubScope (parentScope, args) {
return new PartitionedMap(
parentScope,
new ObjectWrappingMap(args),
new Set(Object.keys(args))
)
}
32 changes: 32 additions & 0 deletions test/unit-tests/expression/parse.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ describe('parse', function () {
assert.strictEqual(scope.f(3), 9)
})

it('should support variable assignment inside a function definition', function () {
const scope = {}
parse('f(x)=(y=x)*2').compile().evaluate(scope)
assert.strictEqual(scope.f(2), 4)
assert.strictEqual(scope.y, 2)
})

it('should spread a function over multiple lines', function () {
assert.deepStrictEqual(parse('add(\n4\n,\n2\n)').compile().evaluate(), 6)
})
Expand Down Expand Up @@ -1538,6 +1545,23 @@ describe('parse', function () {
assert.deepStrictEqual(scope, { a: false })
})

it('should parse logical and inside a function definition', function () {
const scope = {}
const f = parseAndEval('f(x) = x > 2 and x < 4', scope)
assert.strictEqual(f(1), false)
assert.strictEqual(f(3), true)
assert.strictEqual(f(5), false)
})

it('should use a variable assignment with a rawArgs function inside a function definition', function () {
const scope = {}
const f = parseAndEval('f(x) = (a=false) and (b=true)', scope)
assert.deepStrictEqual(parseAndEval('f(2)', scope), false)
assert.deepStrictEqual(Object.keys(scope), ['f', 'a'])
assert.strictEqual(scope.f, f)
assert.strictEqual(scope.a, false)
})

it('should parse logical xor', function () {
assert.strictEqual(parseAndEval('2 xor 6'), false)
assert.strictEqual(parseAndEval('2 xor 0'), true)
Expand All @@ -1560,6 +1584,14 @@ describe('parse', function () {
assert.throws(function () { parseAndEval('false or undefined') }, TypeError)
})

it('should parse logical or inside a function definition', function () {
const scope = {}
const f = parseAndEval('f(x) = x < 2 or x > 4', scope)
assert.strictEqual(f(1), true)
assert.strictEqual(f(3), false)
assert.strictEqual(f(5), true)
})

it('should parse logical or lazily', function () {
const scope = {}
parseAndEval('(a=true) or (b=true)', scope)
Expand Down
Loading