Skip to content

Commit 9ce6b94

Browse files
authored
Merge pull request #949 from glayzzle/fix-unpacking-multiple-arguments
Fix error on variadic function calls.
2 parents fb2f446 + af231be commit 9ce6b94

File tree

5 files changed

+142
-94
lines changed

5 files changed

+142
-94
lines changed

src/parser/function.js

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,29 @@ module.exports = {
217217
* ```
218218
*/
219219
read_parameter_list: function (is_class_constructor) {
220-
if (this.token != ")") {
220+
if (this.token !== ")") {
221+
let wasVariadic = false;
222+
221223
return this.read_list_with_dangling_comma(
222-
this.read_parameter.bind(this, is_class_constructor)
224+
function () {
225+
const parameter = this.read_parameter(is_class_constructor);
226+
if (parameter) {
227+
// variadic parameters can only be defined at the end of the parameter list
228+
if (wasVariadic) {
229+
this.raiseError(
230+
"Unexpected parameter after a variadic parameter"
231+
);
232+
}
233+
if (parameter.variadic) {
234+
wasVariadic = true;
235+
}
236+
}
237+
return parameter;
238+
}.bind(this),
239+
","
223240
);
224241
}
242+
225243
return [];
226244
},
227245
/*
@@ -388,10 +406,14 @@ module.exports = {
388406
function () {
389407
const argument = this.read_argument();
390408
if (argument) {
391-
if (wasVariadic) {
392-
this.raiseError("Unexpected argument after a variadic argument");
409+
const isVariadic = argument.kind === "variadic";
410+
// variadic arguments can only be followed by other variadic arguments
411+
if (wasVariadic && !isVariadic) {
412+
this.raiseError(
413+
"Unexpected non-variadic argument after a variadic argument"
414+
);
393415
}
394-
if (argument.kind === "variadic") {
416+
if (isVariadic) {
395417
wasVariadic = true;
396418
}
397419
}

test/snapshot/__snapshots__/call.test.js.snap

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,91 @@ Program {
199199
}
200200
`;
201201

202+
exports[`Test call handles spread operator at call site 1`] = `
203+
Program {
204+
"children": Array [
205+
ExpressionStatement {
206+
"expression": Call {
207+
"arguments": Array [
208+
variadic {
209+
"kind": "variadic",
210+
"what": Variable {
211+
"curly": false,
212+
"kind": "variable",
213+
"name": "bar",
214+
},
215+
},
216+
],
217+
"kind": "call",
218+
"what": Name {
219+
"kind": "name",
220+
"name": "foo",
221+
"resolution": "uqn",
222+
},
223+
},
224+
"kind": "expressionstatement",
225+
},
226+
ExpressionStatement {
227+
"expression": Call {
228+
"arguments": Array [
229+
Variable {
230+
"curly": false,
231+
"kind": "variable",
232+
"name": "bar",
233+
},
234+
variadic {
235+
"kind": "variadic",
236+
"what": Variable {
237+
"curly": false,
238+
"kind": "variable",
239+
"name": "baz",
240+
},
241+
},
242+
],
243+
"kind": "call",
244+
"what": Name {
245+
"kind": "name",
246+
"name": "foo",
247+
"resolution": "uqn",
248+
},
249+
},
250+
"kind": "expressionstatement",
251+
},
252+
ExpressionStatement {
253+
"expression": Call {
254+
"arguments": Array [
255+
variadic {
256+
"kind": "variadic",
257+
"what": Variable {
258+
"curly": false,
259+
"kind": "variable",
260+
"name": "bar",
261+
},
262+
},
263+
variadic {
264+
"kind": "variadic",
265+
"what": Variable {
266+
"curly": false,
267+
"kind": "variable",
268+
"name": "baz",
269+
},
270+
},
271+
],
272+
"kind": "call",
273+
"what": Name {
274+
"kind": "name",
275+
"name": "foo",
276+
"resolution": "uqn",
277+
},
278+
},
279+
"kind": "expressionstatement",
280+
},
281+
],
282+
"errors": Array [],
283+
"kind": "program",
284+
}
285+
`;
286+
202287
exports[`Test call inside offsetlookup 1`] = `
203288
Program {
204289
"children": Array [

test/snapshot/__snapshots__/function.test.js.snap

Lines changed: 5 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,88 +1722,11 @@ Program {
17221722
}
17231723
`;
17241724

1725-
exports[`Function tests test variadic error 1`] = `
1726-
Program {
1727-
"children": Array [
1728-
ExpressionStatement {
1729-
"expression": Assign {
1730-
"kind": "assign",
1731-
"left": Variable {
1732-
"curly": false,
1733-
"kind": "variable",
1734-
"name": "b",
1735-
},
1736-
"operator": "=",
1737-
"right": Call {
1738-
"arguments": Array [
1739-
variadic {
1740-
"kind": "variadic",
1741-
"what": Array {
1742-
"items": Array [
1743-
Entry {
1744-
"byRef": false,
1745-
"key": null,
1746-
"kind": "entry",
1747-
"unpack": false,
1748-
"value": Number {
1749-
"kind": "number",
1750-
"value": "1",
1751-
},
1752-
},
1753-
Entry {
1754-
"byRef": false,
1755-
"key": null,
1756-
"kind": "entry",
1757-
"unpack": false,
1758-
"value": Number {
1759-
"kind": "number",
1760-
"value": "2",
1761-
},
1762-
},
1763-
Entry {
1764-
"byRef": false,
1765-
"key": null,
1766-
"kind": "entry",
1767-
"unpack": false,
1768-
"value": Number {
1769-
"kind": "number",
1770-
"value": "3",
1771-
},
1772-
},
1773-
],
1774-
"kind": "array",
1775-
"shortForm": true,
1776-
},
1777-
},
1778-
Variable {
1779-
"curly": false,
1780-
"kind": "variable",
1781-
"name": "a",
1782-
},
1783-
],
1784-
"kind": "call",
1785-
"what": Name {
1786-
"kind": "name",
1787-
"name": "foo",
1788-
"resolution": "uqn",
1789-
},
1790-
},
1791-
},
1792-
"kind": "expressionstatement",
1793-
},
1794-
],
1795-
"errors": Array [
1796-
Error {
1797-
"expected": undefined,
1798-
"kind": "error",
1799-
"line": 1,
1800-
"message": "Unexpected argument after a variadic argument on line 1",
1801-
"token": undefined,
1802-
},
1803-
],
1804-
"kind": "program",
1805-
}
1806-
`;
1725+
exports[`Function tests test variadic call error 1`] = `"Unexpected non-variadic argument after a variadic argument on line 1"`;
1726+
1727+
exports[`Function tests test variadic function error 1 1`] = `"Unexpected parameter after a variadic parameter on line 1"`;
1728+
1729+
exports[`Function tests test variadic function error 2 1`] = `"Unexpected parameter after a variadic parameter on line 1"`;
18071730

18081731
exports[`Function tests test without danging comma in closure use-block php 8.0 1`] = `
18091732
Program {

test/snapshot/call.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,4 +303,13 @@ describe("Test call", function () {
303303
);
304304
expect(astErr).toMatchSnapshot();
305305
});
306+
it("handles spread operator at call site", function () {
307+
expect(
308+
parser.parseEval(`
309+
foo(...$bar);
310+
foo($bar, ...$baz);
311+
foo(...$bar, ...$baz);
312+
`)
313+
).toMatchSnapshot();
314+
});
306315
});

test/snapshot/function.test.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,22 @@ describe("Function tests", function () {
100100
expect(ast).toMatchSnapshot();
101101
});
102102

103-
it("test variadic error", function () {
104-
const astErr = parser.parseEval(`$b = foo(...[1, 2, 3], $a);`, {
105-
parser: {
106-
suppressErrors: true,
107-
},
108-
});
109-
expect(astErr).toMatchSnapshot();
103+
it("test variadic call error", function () {
104+
expect(() =>
105+
parser.parseEval(`$b = foo(...[1, 2, 3], $a);`)
106+
).toThrowErrorMatchingSnapshot();
107+
});
108+
109+
it("test variadic function error 1", function () {
110+
expect(() =>
111+
parser.parseEval(`function foo(...$bar, $baz) {}`)
112+
).toThrowErrorMatchingSnapshot();
113+
});
114+
115+
it("test variadic function error 2", function () {
116+
expect(() =>
117+
parser.parseEval(`function foo(...$bar, ...$baz) {}`)
118+
).toThrowErrorMatchingSnapshot();
110119
});
111120

112121
it("test reserved word for function name error", function () {

0 commit comments

Comments
 (0)