From 603c124d12466ab4506679484c826d450aee1940 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Thu, 3 Oct 2019 22:00:53 +0200 Subject: [PATCH 1/6] Fixed out of spec behavior of array.reduce --- src/lualib/ArrayReduce.ts | 13 +++++++++---- test/unit/builtins/array.spec.ts | 8 ++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/lualib/ArrayReduce.ts b/src/lualib/ArrayReduce.ts index 9122afa68..dc68695b8 100644 --- a/src/lualib/ArrayReduce.ts +++ b/src/lualib/ArrayReduce.ts @@ -1,20 +1,25 @@ +declare function select(this: void, index: number, ...args: T[]): T; +declare function select(this: void, index: "#", ...args: T[]): number; +/** @vararg */ +interface VarArg extends Array {} + // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.reduce function __TS__ArrayReduce( this: void, arr: T[], callbackFn: (accumulator: T, currentValue: T, index: number, array: T[]) => T, - initial?: T + ...initial: VarArg ): T { const len = arr.length; - if (len === 0 && initial === undefined) { + if (len === 0 && select("#", ...initial) === 0) { // tslint:disable-next-line: no-string-throw throw "Reduce of empty array with no initial value"; } let k = 0; - let accumulator = initial; - if (initial === undefined) { + let accumulator = select(1, ...initial); + if (accumulator === undefined) { accumulator = arr[0]; k++; } diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index edac44090..0f37278cd 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -486,6 +486,14 @@ test.each<[[(total: number, currentItem: number, index: number, array: number[]) util.testExpression`[1, 3, 5, 7].reduce(${util.valuesToString(args)})`.expectToMatchJsResult(); }); +test("array.reduce empty undefined initial", () => { + util.testExpression`[].reduce(() => {}, undefined)`.expectToMatchJsResult(); +}); + +test("array.reduce empty no initial", () => { + util.testExpression`[].reduce(() => {})`.expectToMatchJsResult(true); +}); + const genericChecks = [ "function generic(array: T)", "function generic(array: T)", From cbc6acc353a209e91ab43454abe7edb10b5c0f84 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Thu, 3 Oct 2019 22:17:27 +0200 Subject: [PATCH 2/6] Moved select declarations from ArrayReduce to lualib declaration files --- src/lualib/ArrayReduce.ts | 2 -- src/lualib/declarations/global.d.ts | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lualib/ArrayReduce.ts b/src/lualib/ArrayReduce.ts index dc68695b8..3630db91d 100644 --- a/src/lualib/ArrayReduce.ts +++ b/src/lualib/ArrayReduce.ts @@ -1,5 +1,3 @@ -declare function select(this: void, index: number, ...args: T[]): T; -declare function select(this: void, index: "#", ...args: T[]): number; /** @vararg */ interface VarArg extends Array {} diff --git a/src/lualib/declarations/global.d.ts b/src/lualib/declarations/global.d.ts index e27b9ecea..fc04d3377 100644 --- a/src/lualib/declarations/global.d.ts +++ b/src/lualib/declarations/global.d.ts @@ -16,3 +16,6 @@ declare function rawset(table: T, key: K, val: T[K]): void declare function next(table: Record, index?: K): [K, V]; declare function pcall(func: () => any): any; declare function unpack(list: T[], i?: number, j?: number): T[]; + +declare function select(index: number, ...args: T[]): T; +declare function select(index: "#", ...args: T[]): number; From 826b878e23a9dbed7ebed719d817e68d0b47bbb4 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 5 Oct 2019 16:25:02 +0200 Subject: [PATCH 3/6] Fixed issue in reduce with undefined returning callback --- src/lualib/ArrayReduce.ts | 16 ++++++++++------ test/unit/builtins/array.spec.ts | 8 ++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/lualib/ArrayReduce.ts b/src/lualib/ArrayReduce.ts index 3630db91d..a4bd280dd 100644 --- a/src/lualib/ArrayReduce.ts +++ b/src/lualib/ArrayReduce.ts @@ -1,25 +1,29 @@ /** @vararg */ -interface VarArg extends Array {} +interface Vararg extends Array {} // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.reduce function __TS__ArrayReduce( this: void, arr: T[], callbackFn: (accumulator: T, currentValue: T, index: number, array: T[]) => T, - ...initial: VarArg + ...initial: Vararg ): T { const len = arr.length; - if (len === 0 && select("#", ...initial) === 0) { + const initialValuePresent = select("#", ...initial) !== 0; + if (len === 0 && !initialValuePresent) { // tslint:disable-next-line: no-string-throw throw "Reduce of empty array with no initial value"; } let k = 0; - let accumulator = select(1, ...initial); - if (accumulator === undefined) { + let accumulator = undefined; + + if (initialValuePresent) { + accumulator = select(1, ...initial); + } else { accumulator = arr[0]; - k++; + k = 1; } while (k < len) { diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index 0f37278cd..6b2956eda 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -494,6 +494,14 @@ test("array.reduce empty no initial", () => { util.testExpression`[].reduce(() => {})`.expectToMatchJsResult(true); }); +test("array.reduce undefined returning callback", () => { + util.testFunction` + const calls: {a: void, b: string}[] = []; + ['a', 'b'].reduce((a, b) => { calls.push({ a, b }) }, undefined); + return calls; + `.expectToMatchJsResult(); +}); + const genericChecks = [ "function generic(array: T)", "function generic(array: T)", From f905134b85bdf8f5292249f6ecf3751c46f99b62 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 5 Oct 2019 17:11:37 +0200 Subject: [PATCH 4/6] Changed reduce loop from while to for for performance reasons --- src/lualib/ArrayReduce.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lualib/ArrayReduce.ts b/src/lualib/ArrayReduce.ts index a4bd280dd..ed162cdc0 100644 --- a/src/lualib/ArrayReduce.ts +++ b/src/lualib/ArrayReduce.ts @@ -1,5 +1,7 @@ /** @vararg */ interface Vararg extends Array {} +/** @forRange */ +declare function forRange(start: number, limit: number, step?: number): number[]; // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.reduce function __TS__ArrayReduce( @@ -26,9 +28,8 @@ function __TS__ArrayReduce( k = 1; } - while (k < len) { - accumulator = callbackFn(accumulator, arr[k], k, arr); - k = k + 1; + for (const i of forRange(k, len - 1)) { + accumulator = callbackFn(accumulator, arr[i], i, arr); } return accumulator; From 5711abc0f2d52c9ce1b7bc554b49229e608d2864 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 5 Oct 2019 17:46:49 +0200 Subject: [PATCH 5/6] Cleaned up code a little from PR feedback --- src/lualib/ArrayReduce.ts | 19 ++++++------------- src/lualib/declarations/tstl.d.ts | 6 ++++++ test/unit/builtins/array.spec.ts | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/lualib/ArrayReduce.ts b/src/lualib/ArrayReduce.ts index ed162cdc0..db80f4e62 100644 --- a/src/lualib/ArrayReduce.ts +++ b/src/lualib/ArrayReduce.ts @@ -1,8 +1,3 @@ -/** @vararg */ -interface Vararg extends Array {} -/** @forRange */ -declare function forRange(start: number, limit: number, step?: number): number[]; - // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.reduce function __TS__ArrayReduce( this: void, @@ -12,20 +7,18 @@ function __TS__ArrayReduce( ): T { const len = arr.length; - const initialValuePresent = select("#", ...initial) !== 0; - if (len === 0 && !initialValuePresent) { - // tslint:disable-next-line: no-string-throw - throw "Reduce of empty array with no initial value"; - } - let k = 0; let accumulator = undefined; - if (initialValuePresent) { + // Check if initial value is present in function call + if (select("#", ...initial) !== 0) { accumulator = select(1, ...initial); - } else { + } else if (len > 0) { accumulator = arr[0]; k = 1; + } else { + // tslint:disable-next-line: no-string-throw + throw "Reduce of empty array with no initial value"; } for (const i of forRange(k, len - 1)) { diff --git a/src/lualib/declarations/tstl.d.ts b/src/lualib/declarations/tstl.d.ts index 8441e227b..7a7a8bf4f 100644 --- a/src/lualib/declarations/tstl.d.ts +++ b/src/lualib/declarations/tstl.d.ts @@ -1,5 +1,11 @@ /** @noSelfInFile */ +/** @vararg */ +interface Vararg extends Array {} + +/** @forRange */ +declare function forRange(start: number, limit: number, step?: number): number[]; + interface LuaClass { prototype: LuaObject; ____super?: LuaClass; diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index 6b2956eda..151ac05dc 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -496,7 +496,7 @@ test("array.reduce empty no initial", () => { test("array.reduce undefined returning callback", () => { util.testFunction` - const calls: {a: void, b: string}[] = []; + const calls: Array<{a: void, b: string}> = []; ['a', 'b'].reduce((a, b) => { calls.push({ a, b }) }, undefined); return calls; `.expectToMatchJsResult(); From 2ab6c5b53d40467cc2ab7b86ea13f98ad32ff777 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 5 Oct 2019 18:03:24 +0200 Subject: [PATCH 6/6] testcase formatting --- test/unit/builtins/array.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index 151ac05dc..7f1cf095b 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -496,8 +496,8 @@ test("array.reduce empty no initial", () => { test("array.reduce undefined returning callback", () => { util.testFunction` - const calls: Array<{a: void, b: string}> = []; - ['a', 'b'].reduce((a, b) => { calls.push({ a, b }) }, undefined); + const calls: Array<{ a: void, b: string }> = []; + ["a", "b"].reduce((a, b) => { calls.push({ a, b }) }, undefined); return calls; `.expectToMatchJsResult(); });