X Tutup
Skip to content

Commit b7e7296

Browse files
PerryvwPerryvw
authored
Fixed issue in test code formatting, fixed array.splice and array.join (#734)
* Refactored test formatting helper causing some tests to start (correctly) failing * Fix array.join with unidentified argument * Fixed splice implementation * Improved code from PR feedback
1 parent bf9f609 commit b7e7296

File tree

11 files changed

+94
-65
lines changed

11 files changed

+94
-65
lines changed

‎package-lock.json‎

Lines changed: 9 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"@types/node": "^11.13.14",
5050
"@types/resolve": "0.0.8",
5151
"fengari": "^0.1.4",
52+
"javascript-stringify": "^2.0.0",
5253
"jest": "^24.8.0",
5354
"jest-circus": "^24.8.0",
5455
"prettier": "^1.18.2",

‎src/LuaTransformer.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5139,8 +5139,13 @@ export class LuaTransformer {
51395139
case "splice":
51405140
return this.transformLuaLibFunction(LuaLibFeature.ArraySplice, node, caller, ...params);
51415141
case "join":
5142-
const parameters =
5143-
node.arguments.length === 0 ? [caller, tstl.createStringLiteral(",")] : [caller].concat(params);
5142+
const defaultSeparatorLiteral = tstl.createStringLiteral(",");
5143+
const parameters = [
5144+
caller,
5145+
node.arguments.length === 0
5146+
? defaultSeparatorLiteral
5147+
: tstl.createBinaryExpression(params[0], defaultSeparatorLiteral, tstl.SyntaxKind.OrOperator),
5148+
];
51445149

51455150
return tstl.createCallExpression(
51465151
tstl.createTableIndexExpression(tstl.createIdentifier("table"), tstl.createStringLiteral("concat")),

‎src/lualib/ArraySplice.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
function __TS__ArraySplice<T>(this: void, list: T[], start: number, deleteCount: number, ...items: T[]): T[] {
1+
// https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.splice
2+
function __TS__ArraySplice<T>(this: void, list: T[], ...args: Vararg<unknown>): T[] {
23
const len = list.length;
34

5+
const actualArgumentCount = select("#", ...args);
6+
const start = select(1, ...args) as number;
7+
const deleteCount = select(2, ...args) as number;
8+
49
let actualStart: number;
510

611
if (start < 0) {
@@ -9,16 +14,18 @@ function __TS__ArraySplice<T>(this: void, list: T[], start: number, deleteCount:
914
actualStart = Math.min(start, len);
1015
}
1116

12-
const itemCount = items.length;
17+
const itemCount = Math.max(actualArgumentCount - 2, 0);
1318

1419
let actualDeleteCount: number;
1520

16-
if (!start) {
21+
if (actualArgumentCount === 0) {
22+
// ECMA-spec line 5: if number of actual arguments is 0
1723
actualDeleteCount = 0;
18-
} else if (!deleteCount) {
24+
} else if (actualArgumentCount === 1) {
25+
// ECMA-spec line 6: if number of actual arguments is 1
1926
actualDeleteCount = len - actualStart;
2027
} else {
21-
actualDeleteCount = Math.min(Math.max(deleteCount, 0), len - actualStart);
28+
actualDeleteCount = Math.min(Math.max(deleteCount || 0, 0), len - actualStart);
2229
}
2330

2431
const out: T[] = [];
@@ -59,8 +66,8 @@ function __TS__ArraySplice<T>(this: void, list: T[], start: number, deleteCount:
5966
}
6067

6168
let j = actualStart;
62-
for (const e of items) {
63-
list[j] = e;
69+
for (const i of forRange(3, actualArgumentCount)) {
70+
list[j] = select(i, ...args) as T;
6471
j++;
6572
}
6673

‎test/unit/builtins/array.spec.ts‎

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ test.each([
256256
{ array: [0, 2, 4, 8], predicate: "false" },
257257
])("array.find (%p)", ({ array, predicate }) => {
258258
util.testFunction`
259-
const array = ${util.valueToString(array)};
259+
const array = ${util.formatCode(array)};
260260
return array.find((elem, index, arr) => ${predicate} && arr[index] === elem);
261261
`.expectToMatchJsResult();
262262
});
@@ -268,7 +268,7 @@ test.each([
268268
{ array: [0, 2, 4, 8], searchElement: 8 },
269269
])("array.findIndex (%p)", ({ array, searchElement }) => {
270270
util.testFunction`
271-
const array = ${util.valueToString(array)};
271+
const array = ${util.formatCode(array)};
272272
return array.findIndex((elem, index, arr) => elem === ${searchElement} && arr[index] === elem);
273273
`.expectToMatchJsResult();
274274
});
@@ -281,7 +281,7 @@ test.each([
281281
{ array: [0, 1, 2, 3], func: "x => x+2" },
282282
{ array: [0, 1, 2, 3], func: "x => x%2 == 0 ? x + 1 : x - 1" },
283283
])("array.map (%p)", ({ array, func }) => {
284-
util.testExpression`${util.valueToString(array)}.map(${func})`.expectToMatchJsResult();
284+
util.testExpression`${util.formatCode(array)}.map(${func})`.expectToMatchJsResult();
285285
});
286286

287287
test.each([
@@ -293,7 +293,7 @@ test.each([
293293
{ array: [0, 1, 2, 3], func: "() => true" },
294294
{ array: [0, 1, 2, 3], func: "() => false" },
295295
])("array.filter (%p)", ({ array, func }) => {
296-
util.testExpression`${util.valueToString(array)}.filter(${func})`.expectToMatchJsResult();
296+
util.testExpression`${util.formatCode(array)}.filter(${func})`.expectToMatchJsResult();
297297
});
298298

299299
test.each([
@@ -302,7 +302,7 @@ test.each([
302302
{ array: [false, true, false], func: "x => x" },
303303
{ array: [true, true, true], func: "x => x" },
304304
])("array.every (%p)", ({ array, func }) => {
305-
util.testExpression`${util.valueToString(array)}.every(${func})`.expectToMatchJsResult();
305+
util.testExpression`${util.formatCode(array)}.every(${func})`.expectToMatchJsResult();
306306
});
307307

308308
test.each([
@@ -311,7 +311,7 @@ test.each([
311311
{ array: [false, true, false], func: "x => x" },
312312
{ array: [true, true, true], func: "x => x" },
313313
])("array.some (%p)", ({ array, func }) => {
314-
util.testExpression`${util.valueToString(array)}.some(${func})`.expectToMatchJsResult();
314+
util.testExpression`${util.formatCode(array)}.some(${func})`.expectToMatchJsResult();
315315
});
316316

317317
test.each([
@@ -324,7 +324,8 @@ test.each([
324324
{ array: [0, 1, 2, 3, 4, 5], args: [1, 3] },
325325
{ array: [0, 1, 2, 3, 4, 5], args: [3] },
326326
])("array.slice (%p)", ({ array, args }) => {
327-
util.testExpression`${util.valueToString(array)}.slice(${util.valuesToString(args)})`.expectToMatchJsResult();
327+
const argumentString = util.formatCode(...args);
328+
util.testExpression`${util.formatCode(array)}.slice(${argumentString})`.expectToMatchJsResult();
328329
});
329330

330331
test.each([
@@ -338,26 +339,37 @@ test.each([
338339
{ array: [0, 1, 2, 3], start: -3, deleteCount: 0, newElements: [8, 9] },
339340
{ array: [0, 1, 2, 3, 4, 5], start: 5, deleteCount: 9, newElements: [10, 11] },
340341
{ array: [0, 1, 2, 3, 4, 5], start: 3, deleteCount: 2, newElements: [3, 4, 5] },
342+
{ array: [0, 1, 2, 3, 4, 5, 6, 7, 8], start: 5, deleteCount: 9, newElements: [10, 11] },
343+
{ array: [0, 1, 2, 3, 4, 5, 6, 7, 8], start: 5, deleteCount: undefined, newElements: [10, 11] },
344+
// tslint:disable-next-line:no-null-keyword
345+
{ array: [0, 1, 2, 3, 4, 5, 6, 7, 8], start: 5, deleteCount: null, newElements: [10, 11] },
341346

342347
// Remove
343348
{ array: [], start: 1, deleteCount: 1 },
344349
{ array: [0, 1, 2, 3], start: 1, deleteCount: 1 },
345350
{ array: [0, 1, 2, 3], start: 10, deleteCount: 1 },
346-
{ array: [0, 1, 2, 3], start: 1, deleteCount: undefined },
347-
{ array: [0, 1, 2, 3], start: 4 },
348-
{ array: [0, 1, 2, 3, 4, 5], start: 3 },
349-
{ array: [0, 1, 2, 3, 4, 5], start: -3 },
350-
{ array: [0, 1, 2, 3, 4, 5], start: -2 },
351351
{ array: [0, 1, 2, 3, 4, 5], start: 2, deleteCount: 2 },
352-
{ array: [0, 1, 2, 3, 4, 5, 6, 7, 8], start: 5, deleteCount: 9, newElements: [10, 11] },
352+
{ array: [0, 1, 2, 3, 4, 5], start: -3, deleteCount: 2 },
353+
{ array: [0, 1, 2, 3], start: 1, deleteCount: undefined },
354+
// tslint:disable-next-line:no-null-keyword
355+
{ array: [0, 1, 2, 3], start: 1, deleteCount: null },
353356
])("array.splice (%p)", ({ array, start, deleteCount, newElements = [] }) => {
354357
util.testFunction`
355-
const array = ${util.valueToString(array)};
356-
array.splice(${util.valuesToString([start, deleteCount, ...newElements])});
358+
const array = ${util.formatCode(array)};
359+
array.splice(${util.formatCode(start, deleteCount, ...newElements)});
357360
return array;
358361
`.expectToMatchJsResult();
359362
});
360363

364+
test.each([
365+
{ array: [0, 1, 2, 3], start: 4 },
366+
{ array: [0, 1, 2, 3, 4, 5], start: 3 },
367+
{ array: [0, 1, 2, 3, 4, 5], start: -3 },
368+
{ array: [0, 1, 2, 3, 4, 5], start: -2 },
369+
])("array.splice no delete argument", ({ array, start }) => {
370+
util.testExpression`${util.formatCode(array)}.splice(${start})`.expectToMatchJsResult();
371+
});
372+
361373
test.each([
362374
{ array: [], args: [[]] },
363375
{ array: [1, 2, 3], args: [[]] },
@@ -371,8 +383,8 @@ test.each([
371383
{ array: [1, 2, "test"], args: ["test", ["test1", "test2"]] },
372384
])("array.concat (%p)", ({ array, args }) => {
373385
util.testFunction`
374-
const array: any[] = ${util.valueToString(array)};
375-
return array.concat(${util.valuesToString(args)});
386+
const array: any[] = ${util.formatCode(array)};
387+
return array.concat(${util.formatCode(...args)});
376388
`.expectToMatchJsResult();
377389
});
378390

@@ -383,7 +395,11 @@ test.each([
383395
{ array: ["test1", "test2"], separator: ";" },
384396
{ array: ["test1", "test2"], separator: "" },
385397
])("array.join (%p)", ({ array, separator }) => {
386-
util.testExpression`${util.valueToString(array)}.join(${util.valueToString(separator)})`.expectToMatchJsResult();
398+
util.testExpression`${util.formatCode(array)}.join(${util.formatCode(separator)})`.expectToMatchJsResult();
399+
});
400+
401+
test("array.join without separator argument", () => {
402+
util.testExpression`["test1", "test2"].join()`.expectToMatchJsResult();
387403
});
388404

389405
test.each([
@@ -395,13 +411,13 @@ test.each([
395411
{ array: ["test1", "test2", "test3"], args: ["test1", -2] },
396412
{ array: ["test1", "test2", "test3"], args: ["test1", 12] },
397413
])("array.indexOf (%p)", ({ array, args }) => {
398-
util.testExpression`${util.valueToString(array)}.indexOf(${util.valuesToString(args)})`.expectToMatchJsResult();
414+
util.testExpression`${util.formatCode(array)}.indexOf(${util.formatCode(...args)})`.expectToMatchJsResult();
399415
});
400416

401417
test.each([{ args: [1] }, { args: [1, 2, 3] }])("array.push (%p)", ({ args }) => {
402418
util.testFunction`
403419
const array = [0];
404-
const value = array.push(${util.valuesToString(args)});
420+
const value = array.push(${util.formatCode(...args)});
405421
return { array, value };
406422
`.expectToMatchJsResult();
407423
});
@@ -411,7 +427,7 @@ test.each([{ array: [1, 2, 3], expected: [3, 2] }, { array: [1, 2, 3, null], exp
411427
"array.pop (%p)",
412428
({ array, expected }) => {
413429
util.testFunction`
414-
const array = ${util.valueToString(array)};
430+
const array = ${util.formatCode(array)};
415431
const value = array.pop();
416432
return [value, array.length];
417433
`.expectToEqual(expected);
@@ -422,7 +438,7 @@ test.each([{ array: [1, 2, 3] }, { array: [1, 2, 3, 4] }, { array: [1] }, { arra
422438
"array.reverse (%p)",
423439
({ array }) => {
424440
util.testFunction`
425-
const array = ${util.valueToString(array)};
441+
const array = ${util.formatCode(array)};
426442
array.reverse();
427443
return array;
428444
`.expectToMatchJsResult();
@@ -431,7 +447,7 @@ test.each([{ array: [1, 2, 3] }, { array: [1, 2, 3, 4] }, { array: [1] }, { arra
431447

432448
test.each([{ array: [1, 2, 3] }, { array: [1] }, { array: [] }])("array.shift (%p)", ({ array }) => {
433449
util.testFunction`
434-
const array = ${util.valueToString(array)};
450+
const array = ${util.formatCode(array)};
435451
const value = array.shift();
436452
return { array, value };
437453
`.expectToMatchJsResult();
@@ -444,8 +460,8 @@ test.each([
444460
{ array: [], args: [1] },
445461
])("array.unshift (%p)", ({ array, args }) => {
446462
util.testFunction`
447-
const array = ${util.valueToString(array)};
448-
const value = array.unshift(${util.valuesToString(args)});
463+
const array = ${util.formatCode(array)};
464+
const value = array.unshift(${util.formatCode(...args)});
449465
return { array, value };
450466
`.expectToMatchJsResult();
451467
});
@@ -499,7 +515,7 @@ test.each<[[(total: number, currentItem: number, index: number, array: number[])
499515
[[(total, _, index, array) => total + array[index]]],
500516
[[(a, b) => a + b]],
501517
])("array.reduce (%p)", args => {
502-
util.testExpression`[1, 3, 5, 7].reduce(${util.valuesToString(args)})`.expectToMatchJsResult();
518+
util.testExpression`[1, 3, 5, 7].reduce(${util.formatCode(...args)})`.expectToMatchJsResult();
503519
});
504520

505521
test("array.reduce empty undefined initial", () => {

‎test/unit/builtins/object.spec.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ test.each([
66
{ initial: { a: 3 }, args: [{ a: 5 }] },
77
{ initial: { a: 3 }, args: [{ b: 5 }, { c: 7 }] },
88
])("Object.assign (%p)", ({ initial, args }) => {
9-
util.testExpression`Object.assign(${util.valueToString(initial)}, ${util.valuesToString(
10-
args
11-
)})`.expectToMatchJsResult();
9+
const argsString = util.formatCode(...args);
10+
util.testExpression`Object.assign(${util.formatCode(initial)}, ${argsString})`.expectToMatchJsResult();
1211
});
1312

1413
test.each([{}, { abc: 3 }, { abc: 3, def: "xyz" }])("Object.entries (%p)", obj => {

‎test/unit/builtins/string.spec.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ test("Supported lua string function", () => {
2020
});
2121

2222
test.each([[], [65], [65, 66], [65, 66, 67]])("String.fromCharCode (%p)", (...args) => {
23-
util.testExpression`String.fromCharCode(${util.valuesToString(args)})`.expectToMatchJsResult();
23+
util.testExpression`String.fromCharCode(${util.formatCode(...args)})`.expectToMatchJsResult();
2424
});
2525

2626
test.each([
@@ -61,13 +61,13 @@ test.each([
6161
{ inp: "hello test", searchValue: "test", replaceValue: (): string => "%a" },
6262
{ inp: "aaa", searchValue: "a", replaceValue: "b" },
6363
])("string.replace (%p)", ({ inp, searchValue, replaceValue }) => {
64-
util.testExpression`"${inp}".replace(${util.valuesToString([searchValue, replaceValue])})`.expectToMatchJsResult();
64+
util.testExpression`"${inp}".replace(${util.formatCode(searchValue, replaceValue)})`.expectToMatchJsResult();
6565
});
6666

6767
test.each([["", ""], ["hello", "test"], ["hello", "test", "bye"], ["hello", 42], [42, "hello"]])(
6868
"string.concat[+] (%p)",
6969
(...elements) => {
70-
util.testExpression(elements.map(e => util.valueToString(e)).join(" + ")).expectToMatchJsResult();
70+
util.testExpression(elements.map(e => util.formatCode(e)).join(" + ")).expectToMatchJsResult();
7171
}
7272
);
7373

@@ -77,7 +77,7 @@ test.each([
7777
{ str: "hello", args: [] },
7878
{ str: "hello", args: ["test", "bye"] },
7979
])("string.concatFct (%p)", ({ str, args }) => {
80-
util.testExpression`"${str}".concat(${util.valuesToString(args)})`.expectToMatchJsResult();
80+
util.testExpression`"${str}".concat(${util.formatCode(...args)})`.expectToMatchJsResult();
8181
});
8282

8383
test.each([
@@ -113,7 +113,7 @@ test.each([
113113
{ inp: "hello test", args: [1, 2] },
114114
{ inp: "hello test", args: [1, 5] },
115115
])("string.slice (%p)", ({ inp, args }) => {
116-
util.testExpression`"${inp}".slice(${util.valuesToString(args)})`.expectToMatchJsResult();
116+
util.testExpression`"${inp}".slice(${util.formatCode(...args)})`.expectToMatchJsResult();
117117
});
118118

119119
test.each([
@@ -122,7 +122,7 @@ test.each([
122122
{ inp: "hello test", args: [1, 2] },
123123
{ inp: "hello test", args: [1, 5] },
124124
])("string.substring (%p)", ({ inp, args }) => {
125-
util.testExpression`"${inp}".substring(${util.valuesToString(args)})`.expectToMatchJsResult();
125+
util.testExpression`"${inp}".substring(${util.formatCode(...args)})`.expectToMatchJsResult();
126126
});
127127

128128
test.each([{ inp: "hello test", start: 1, ignored: 0 }, { inp: "hello test", start: 3, ignored: 0, end: 5 }])(
@@ -139,7 +139,7 @@ test.each([
139139
{ inp: "hello test", args: [1, 2] },
140140
{ inp: "hello test", args: [1, 5] },
141141
])("string.substr (%p)", ({ inp, args }) => {
142-
util.testExpression`"${inp}".substr(${util.valuesToString(args)})`.expectToMatchJsResult();
142+
util.testExpression`"${inp}".substr(${util.formatCode(...args)})`.expectToMatchJsResult();
143143
});
144144

145145
test.each([{ inp: "hello test", start: 1, ignored: 0 }, { inp: "hello test", start: 3, ignored: 0, end: 2 }])(
@@ -207,7 +207,7 @@ test.each<{ inp: string; args: Parameters<string["startsWith"]> }>([
207207
{ inp: "hello test", args: ["test"] },
208208
{ inp: "hello test", args: ["test", 6] },
209209
])("string.startsWith (%p)", ({ inp, args }) => {
210-
util.testExpression`"${inp}".startsWith(${util.valuesToString(args)})`.expectToMatchJsResult();
210+
util.testExpression`"${inp}".startsWith(${util.formatCode(...args)})`.expectToMatchJsResult();
211211
});
212212

213213
test.each<{ inp: string; args: Parameters<string["endsWith"]> }>([
@@ -216,8 +216,7 @@ test.each<{ inp: string; args: Parameters<string["endsWith"]> }>([
216216
{ inp: "hello test", args: ["hello"] },
217217
{ inp: "hello test", args: ["hello", 5] },
218218
])("string.endsWith (%p)", ({ inp, args }) => {
219-
const argsString = util.valuesToString(args);
220-
util.testExpression`"${inp}".endsWith(${argsString})`.expectToMatchJsResult();
219+
util.testExpression`"${inp}".endsWith(${util.formatCode(...args)})`.expectToMatchJsResult();
221220
});
222221

223222
test.each([
@@ -243,11 +242,11 @@ const padCases = [
243242
];
244243

245244
test.each(padCases)("string.padStart (%p)", ({ inp, args }) => {
246-
util.testExpression`"${inp}".padStart(${util.valuesToString(args)})`.expectToMatchJsResult();
245+
util.testExpression`"${inp}".padStart(${util.formatCode(...args)})`.expectToMatchJsResult();
247246
});
248247

249248
test.each(padCases)("string.padEnd (%p)", ({ inp, args }) => {
250-
util.testExpression`"${inp}".padEnd(${util.valuesToString(args)})`.expectToMatchJsResult();
249+
util.testExpression`"${inp}".padEnd(${util.formatCode(...args)})`.expectToMatchJsResult();
251250
});
252251

253252
test.each([

0 commit comments

Comments
 (0)
X Tutup