Skip to content

Commit 89ed9ad

Browse files
AnslemHackAnslemHack
authored
fix: issues with the optional chaining operator: disallow double dot ?.., allow optional function calls, throw errors on invalid uses of ?. (#3585)
1 parent f211411 commit 89ed9ad

File tree

3 files changed

+88
-36
lines changed

3 files changed

+88
-36
lines changed

‎src/expression/node/FunctionNode.js‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
9595
* the arguments, typically a SymbolNode or AccessorNode
9696
* @param {./Node[]} args
9797
*/
98-
constructor (fn, args) {
98+
constructor (fn, args, optional) {
9999
super()
100100
if (typeof fn === 'string') {
101101
fn = new SymbolNode(fn)
@@ -107,9 +107,14 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
107107
throw new TypeError(
108108
'Array containing Nodes expected for parameter "args"')
109109
}
110+
const optionalType = typeof optional
111+
if (!(optionalType === 'undefined' || optionalType === 'boolean')) {
112+
throw new TypeError('optional flag, if specified, must be boolean')
113+
}
110114

111115
this.fn = fn
112116
this.args = args || []
117+
this.optional = !!optional
113118
}
114119

115120
// readonly property name
@@ -137,7 +142,8 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
137142
_compile (math, argNames) {
138143
// compile arguments
139144
const evalArgs = this.args.map((arg) => arg._compile(math, argNames))
140-
const fromOptionalChaining = isAccessorNode(this.fn) && this.fn.optionalChaining
145+
const fromOptionalChaining = this.optional ||
146+
(isAccessorNode(this.fn) && this.fn.optionalChaining)
141147

142148
if (isSymbolNode(this.fn)) {
143149
const name = this.fn.name
@@ -153,12 +159,14 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
153159
value = scope.get(name)
154160
} else if (name in math) {
155161
value = getSafeProperty(math, name)
156-
} else {
157-
return FunctionNode.onUndefinedFunction(name)
158-
}
159-
if (typeof value === 'function') {
162+
} else if (fromOptionalChaining) value = undefined
163+
else return FunctionNode.onUndefinedFunction(name)
164+
165+
if (typeof value === 'function' ||
166+
(fromOptionalChaining && value === undefined)) {
160167
return value
161168
}
169+
162170
throw new TypeError(
163171
`'${name}' is not a function; its value is:\n ${strin(value)}`
164172
)
@@ -185,17 +193,20 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
185193
switch (evalArgs.length) {
186194
case 0: return function evalFunctionNode (scope, args, context) {
187195
const fn = resolveFn(scope)
196+
if (fromOptionalChaining && fn === undefined) return undefined
188197
return fn()
189198
}
190199
case 1: return function evalFunctionNode (scope, args, context) {
191200
const fn = resolveFn(scope)
201+
if (fromOptionalChaining && fn === undefined) return undefined
192202
const evalArg0 = evalArgs[0]
193203
return fn(
194204
evalArg0(scope, args, context)
195205
)
196206
}
197207
case 2: return function evalFunctionNode (scope, args, context) {
198208
const fn = resolveFn(scope)
209+
if (fromOptionalChaining && fn === undefined) return undefined
199210
const evalArg0 = evalArgs[0]
200211
const evalArg1 = evalArgs[1]
201212
return fn(
@@ -205,6 +216,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
205216
}
206217
default: return function evalFunctionNode (scope, args, context) {
207218
const fn = resolveFn(scope)
219+
if (fromOptionalChaining && fn === undefined) return undefined
208220
const values = evalArgs.map((evalArg) => evalArg(scope, args, context))
209221
return fn(...values)
210222
}
@@ -214,6 +226,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
214226
const rawArgs = this.args
215227
return function evalFunctionNode (scope, args, context) {
216228
const fn = getSafeProperty(args, name)
229+
if (fromOptionalChaining && fn === undefined) return undefined
217230
if (typeof fn !== 'function') {
218231
throw new TypeError(
219232
`Argument '${name}' was not a function; received: ${strin(fn)}`
@@ -245,7 +258,8 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
245258
const object = evalObject(scope, args, context)
246259

247260
// Optional chaining: if the base object is nullish, short-circuit to undefined
248-
if (fromOptionalChaining && object == null) {
261+
if (fromOptionalChaining &&
262+
(object == null || object[prop] === undefined)) {
249263
return undefined
250264
}
251265

@@ -270,6 +284,7 @@ export const createFunctionNode = /* #__PURE__ */ factory(name, dependencies, ({
270284

271285
return function evalFunctionNode (scope, args, context) {
272286
const fn = evalFn(scope, args, context)
287+
if (fromOptionalChaining && fn === undefined) return undefined
273288
if (typeof fn !== 'function') {
274289
throw new TypeError(
275290
`Expression '${fnExpr}' did not evaluate to a function; value is:` +

‎src/expression/parse.js‎

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,9 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
685685
return new AssignmentNode(new SymbolNode(name), value)
686686
} else if (isAccessorNode(node)) {
687687
// parse a matrix subset assignment like 'A[1,2] = 4'
688+
if (node.optionalChaining) {
689+
throw createSyntaxError(state, 'Cannot assign to optional chain')
690+
}
688691
getTokenSkipNewline(state)
689692
value = parseAssignment(state)
690693
return new AssignmentNode(node.object, node.index, value)
@@ -1396,44 +1399,21 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
13961399
optional = true
13971400
// consume the '?.' token
13981401
getToken(state)
1399-
1400-
// Special case: property access via dot-notation following optional chaining (obj?.foo)
1401-
// After consuming '?.', the dot is already consumed as part of the token,
1402-
// so the next token is the property name itself. Handle that here.
1403-
const isPropertyNameAfterOptional = (!types || types.includes('.')) && (
1404-
state.tokenType === TOKENTYPE.SYMBOL ||
1405-
(state.tokenType === TOKENTYPE.DELIMITER && state.token in NAMED_DELIMITERS)
1406-
)
1407-
if (isPropertyNameAfterOptional) {
1408-
params = []
1409-
params.push(new ConstantNode(state.token))
1410-
getToken(state)
1411-
const dotNotation = true
1412-
node = new AccessorNode(node, new IndexNode(params, dotNotation), true)
1413-
// Continue parsing, allowing more chaining after this accessor
1414-
continue
1415-
}
1416-
// Otherwise, fall through to allow patterns like obj?.[...]
14171402
}
14181403

1419-
// If the next token does not start an accessor, we're done
14201404
const hasNextAccessor =
14211405
(state.token === '(' || state.token === '[' || state.token === '.') &&
14221406
(!types || types.includes(state.token))
14231407

1424-
if (!hasNextAccessor) {
1425-
// A dangling '?.' without a following accessor is a syntax error
1426-
if (optional) {
1427-
throw createSyntaxError(state, 'Unexpected operator ?.')
1428-
}
1408+
if (!(optional || hasNextAccessor)) {
14291409
break
14301410
}
14311411

14321412
params = []
14331413

14341414
if (state.token === '(') {
1435-
if (isSymbolNode(node) || isAccessorNode(node)) {
1436-
// function invocation like fn(2, 3) or obj.fn(2, 3)
1415+
if (optional || isSymbolNode(node) || isAccessorNode(node)) {
1416+
// function invocation: fn(2, 3) or obj.fn(2, 3) or (anything)?.(2, 3)
14371417
openParams(state)
14381418
getToken(state)
14391419

@@ -1453,7 +1433,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
14531433
closeParams(state)
14541434
getToken(state)
14551435

1456-
node = new FunctionNode(node, params)
1436+
node = new FunctionNode(node, params, optional)
14571437
} else {
14581438
// implicit multiplication like (2+3)(4+5) or sqrt(2)(1+2)
14591439
// don't parse it here but let it be handled by parseImplicitMultiplication
@@ -1484,12 +1464,15 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
14841464
node = new AccessorNode(node, new IndexNode(params), optional)
14851465
} else {
14861466
// dot notation like variable.prop
1487-
getToken(state)
1467+
// consume the `.` (if it was ?., already consumed):
1468+
if (!optional) getToken(state)
14881469

14891470
const isPropertyName = state.tokenType === TOKENTYPE.SYMBOL ||
14901471
(state.tokenType === TOKENTYPE.DELIMITER && state.token in NAMED_DELIMITERS)
14911472
if (!isPropertyName) {
1492-
throw createSyntaxError(state, 'Property name expected after dot')
1473+
let message = 'Property name expected after '
1474+
message += optional ? 'optional chain' : 'dot'
1475+
throw createSyntaxError(state, message)
14931476
}
14941477

14951478
params.push(new ConstantNode(state.token))

‎test/unit-tests/expression/parse.test.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,8 @@ describe('parse', function () {
956956
const res = parseAndEval('obj["b"] = 2', scope)
957957
assert.strictEqual(res, 2)
958958
assert.deepStrictEqual(scope, { obj: { a: 3, b: 2 } })
959+
assert.deepStrictEqual(
960+
parseAndEval('b = {}; b.a = 2; b').valueOf(), [{ a: 2 }])
959961
})
960962

961963
it('should set a nested object property', function () {
@@ -965,6 +967,17 @@ describe('parse', function () {
965967
assert.deepStrictEqual(scope, { obj: { foo: { bar: 2 } } })
966968
})
967969

970+
it(
971+
'should not set an object property through optional chaining',
972+
function () {
973+
assert.throws(
974+
() => parseAndEval('obj = {a: 2}; obj?.b = 7'), SyntaxError)
975+
assert.throws(
976+
() => parseAndEval('obj = {a: 2}; obj?.["b"] = 7'), SyntaxError)
977+
assert.throws(
978+
() => parseAndEval('obj = {a: {}}; obj.a?.b = 7'), SyntaxError)
979+
})
980+
968981
it('should throw an error when trying to apply a matrix index as object property', function () {
969982
const scope = { a: {} }
970983
assert.throws(function () {
@@ -1196,6 +1209,13 @@ describe('parse', function () {
11961209
assert.throws(function () { parseAndEval('obj.foo["bar"].baz', { obj: { foo: { bar: null } } }) }, TypeError)
11971210
})
11981211

1212+
it('should throw an error when using double-dot after optional chaining operator', function () {
1213+
// ?.. is not valid in JavaScript and should be rejected
1214+
assert.throws(function () { parseAndEval('{a: 3}?..a') }, /SyntaxError: Property name expected after optional chain \(char 9\)/)
1215+
assert.throws(function () { parseAndEval('obj?..foo', { obj: { foo: 2 } }) }, /SyntaxError: Property name expected after optional chain \(char 6\)/)
1216+
assert.throws(function () { parseAndEval('obj?.["a"]?..b', { obj: { a: { b: 2 } } }) }, /SyntaxError: Property name expected after optional chain \(char 13\)/)
1217+
})
1218+
11991219
it('should set an object property with dot notation', function () {
12001220
const scope = { obj: {} }
12011221
parseAndEval('obj.foo = 2', scope)
@@ -1471,6 +1491,40 @@ describe('parse', function () {
14711491
parseAndEval('2(x, 2) = x^2', scope)
14721492
}, SyntaxError)
14731493
})
1494+
1495+
it('should call functions via optional chaining', function () {
1496+
assert.strictEqual(parseAndEval('square?.(2)'), 4)
1497+
assert.deepStrictEqual(parseAndEval('f(x) = x+x; f?.(2)').valueOf(), [4])
1498+
assert.strictEqual(parseAndEval('(_(x) = x^x)?.(2)'), 4)
1499+
assert.strictEqual(parseAndEval('foo?.(2)', { foo: x => x * x }), 4)
1500+
assert.deepStrictEqual(
1501+
parseAndEval('f(x) = 4x/x; bar = {a: f}; bar.a?.(2)').valueOf(), [4])
1502+
})
1503+
1504+
it(
1505+
'should shortcircuit undefined functions via optional chaining',
1506+
function () {
1507+
assert.strictEqual(
1508+
parseAndEval('foo?.(2)', { foo: undefined }), undefined)
1509+
assert.strictEqual(parseAndEval('{a: 3}.foo?.(2)'), undefined)
1510+
assert.strictEqual(
1511+
parseAndEval('foo.bar?.(2)', { foo: {} }), undefined)
1512+
assert.deepStrictEqual(
1513+
parseAndEval('f(x) = undefined; f(0)?.(2)').valueOf(), [undefined])
1514+
assert.strictEqual(parseAndEval('(undefined)?.(2)'), undefined)
1515+
assert.strictEqual(parseAndEval('foo?.(2)'), undefined)
1516+
})
1517+
1518+
it('should throw with optional chain call on non-function', function () {
1519+
// I guess it is OK to consider this a syntax error since we know just
1520+
// by reading the expression that the function call can't succeed.
1521+
assert.throws(() => parseAndEval('7?.(2)'), SyntaxError)
1522+
assert.throws(() => parseAndEval('a = 7; a?.(2)'), TypeError)
1523+
assert.throws(() => parseAndEval('(3+4)?.(2)'), TypeError)
1524+
assert.throws(() => parseAndEval('add(3,4)?.(2)'), TypeError)
1525+
assert.throws(() => parseAndEval('{a: true}.a?.(2)'), Error)
1526+
assert.throws(() => parseAndEval('[3, 4]?.(2)'), TypeError)
1527+
})
14741528
})
14751529

14761530
describe('parentheses', function () {

0 commit comments

Comments
 (0)