From 13e5f7e0ea8b3294bc4df397c0226dd51e645533 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 25 Jan 2021 22:24:14 +0100 Subject: [PATCH 1/4] Added Array.IsArray to lualib --- src/LuaLib.ts | 5 +++-- src/lualib/ArrayFlat.ts | 8 +------- src/lualib/ArrayFlatMap.ts | 7 +------ src/lualib/ArrayIsArray.ts | 5 +++++ src/transformation/builtins/array.ts | 17 +++++++++++++++++ src/transformation/builtins/index.ts | 4 +++- test/unit/builtins/array.spec.ts | 15 +++++++++++++++ 7 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 src/lualib/ArrayIsArray.ts diff --git a/src/LuaLib.ts b/src/LuaLib.ts index fde43c604..de9ee2ead 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -10,6 +10,7 @@ export enum LuaLibFeature { ArrayFindIndex = "ArrayFindIndex", ArrayIncludes = "ArrayIncludes", ArrayIndexOf = "ArrayIndexOf", + ArrayIsArray = "ArrayIsArray", ArrayJoin = "ArrayJoin", ArrayMap = "ArrayMap", ArrayPush = "ArrayPush", @@ -86,8 +87,8 @@ export enum LuaLibFeature { } const luaLibDependencies: Partial> = { - ArrayFlat: [LuaLibFeature.ArrayConcat], - ArrayFlatMap: [LuaLibFeature.ArrayConcat], + ArrayFlat: [LuaLibFeature.ArrayConcat, LuaLibFeature.ArrayIsArray], + ArrayFlatMap: [LuaLibFeature.ArrayConcat, LuaLibFeature.ArrayIsArray], Decorate: [LuaLibFeature.CloneDescriptor], Delete: [LuaLibFeature.ObjectGetOwnPropertyDescriptors], Error: [LuaLibFeature.New, LuaLibFeature.Class], diff --git a/src/lualib/ArrayFlat.ts b/src/lualib/ArrayFlat.ts index 54120e69a..2bed314cb 100644 --- a/src/lualib/ArrayFlat.ts +++ b/src/lualib/ArrayFlat.ts @@ -1,13 +1,7 @@ function __TS__ArrayFlat(this: void, array: any[], depth = 1): any[] { let result: any[] = []; for (const value of array) { - if ( - depth > 0 && - type(value) === "table" && - // Workaround to determine if value is an array or not (fails in case of objects without keys) - // See discussion in: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/737 - (1 in value || (next as NextEmptyCheck)(value, undefined) === undefined) - ) { + if (depth > 0 && Array.isArray(value)) { result = result.concat(__TS__ArrayFlat(value, depth - 1)); } else { result[result.length] = value; diff --git a/src/lualib/ArrayFlatMap.ts b/src/lualib/ArrayFlatMap.ts index 02d58329b..c31af9046 100644 --- a/src/lualib/ArrayFlatMap.ts +++ b/src/lualib/ArrayFlatMap.ts @@ -6,12 +6,7 @@ function __TS__ArrayFlatMap( let result: U[] = []; for (let i = 0; i < array.length; i++) { const value = callback(array[i], i, array); - if ( - type(value) === "table" && - // Workaround to determine if value is an array or not (fails in case of objects without keys) - // See discussion in: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/737 - (1 in value || (next as NextEmptyCheck)(value as any, undefined) === undefined) - ) { + if (type(value) === "table" && Array.isArray(value)) { result = result.concat(value); } else { result[result.length] = value as U; diff --git a/src/lualib/ArrayIsArray.ts b/src/lualib/ArrayIsArray.ts new file mode 100644 index 000000000..38a4525ef --- /dev/null +++ b/src/lualib/ArrayIsArray.ts @@ -0,0 +1,5 @@ +function __TS__ArrayIsArray(this: void, value: any): value is any[] { + // Workaround to determine if value is an array or not (fails in case of objects without keys) + // See discussion in: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/7 + return type(value) === "table" && (1 in value || (next as NextEmptyCheck)(value, undefined) === undefined); +} diff --git a/src/transformation/builtins/array.ts b/src/transformation/builtins/array.ts index 35be01a00..a04e0e98b 100644 --- a/src/transformation/builtins/array.ts +++ b/src/transformation/builtins/array.ts @@ -6,6 +6,23 @@ import { LuaLibFeature, transformLuaLibFunction } from "../utils/lualib"; import { PropertyCallExpression, transformArguments } from "../visitors/call"; import { isStringType, isNumberType } from "../utils/typescript"; +export function transformArrayConstructorCall( + context: TransformationContext, + node: PropertyCallExpression +): lua.CallExpression | undefined { + const expression = node.expression; + const signature = context.checker.getResolvedSignature(node); + const params = transformArguments(context, node.arguments, signature); + + const expressionName = expression.name.text; + switch (expressionName) { + case "isArray": + return transformLuaLibFunction(context, LuaLibFeature.ArrayIsArray, node, ...params); + default: + context.diagnostics.push(unsupportedProperty(expression.name, "Array", expressionName)); + } +} + export function transformArrayPrototypeCall( context: TransformationContext, node: PropertyCallExpression diff --git a/src/transformation/builtins/index.ts b/src/transformation/builtins/index.ts index 1125e96b2..6d7f0bd8b 100644 --- a/src/transformation/builtins/index.ts +++ b/src/transformation/builtins/index.ts @@ -15,7 +15,7 @@ import { } from "../utils/typescript"; import { PropertyCallExpression } from "../visitors/call"; import { checkForLuaLibType } from "../visitors/class/new"; -import { transformArrayProperty, transformArrayPrototypeCall } from "./array"; +import { transformArrayConstructorCall, transformArrayProperty, transformArrayPrototypeCall } from "./array"; import { transformConsoleCall } from "./console"; import { transformFunctionPrototypeCall, transformFunctionProperty } from "./function"; import { transformGlobalCall } from "./global"; @@ -79,6 +79,8 @@ export function transformBuiltinCallExpression( if (isStandardLibraryType(context, ownerType, undefined)) { const symbol = ownerType.getSymbol(); switch (symbol?.name) { + case "ArrayConstructor": + return transformArrayConstructorCall(context, node); case "Console": return transformConsoleCall(context, node); case "Math": diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index ecaaaca53..db9cc4f47 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -618,3 +618,18 @@ test.each(genericChecks)("array constrained generic length (%p)", signature => { `; expect(util.transpileAndExecute(code)).toBe(3); }); + +test.each(["[]", '"hello"', "42", "[1, 2, 3]", '{ a: "foo", b: "bar" }'])( + "Array.isArray matches JavaScript (%p)", + valueString => { + util.testExpression`Array.isArray(${valueString})`.expectToMatchJsResult(); + } +); + +test("Array.isArray returns true for empty objects", () => { + // Important edge case we cannot handle correctly due to [] and {} + // being identical in Lua. We assume [] is more common than Array.isArray({}), + // so it is more important to handle [] right, sacrificing the result for {}. + // See discussion: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/737 + util.testExpression`Array.isArray([])`.expectToEqual(true); +}); From 6a8617147b7e09be1c0a8759c8aadcb8760abe25 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 25 Jan 2021 22:24:32 +0100 Subject: [PATCH 2/4] Fix bug in string.split lualib dependencies --- src/LuaLib.ts | 1 + test/unit/builtins/string.spec.ts | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/LuaLib.ts b/src/LuaLib.ts index de9ee2ead..ecc6aec30 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -103,6 +103,7 @@ const luaLibDependencies: Partial> = { WeakMap: [LuaLibFeature.InstanceOf, LuaLibFeature.Iterator, LuaLibFeature.Symbol, LuaLibFeature.Class], WeakSet: [LuaLibFeature.InstanceOf, LuaLibFeature.Iterator, LuaLibFeature.Symbol, LuaLibFeature.Class], Spread: [LuaLibFeature.Iterator, LuaLibFeature.Unpack], + StringSplit: [LuaLibFeature.StringSubstring], SymbolRegistry: [LuaLibFeature.Symbol], }; diff --git a/test/unit/builtins/string.spec.ts b/test/unit/builtins/string.spec.ts index 00f5c878d..37dadeedc 100644 --- a/test/unit/builtins/string.spec.ts +++ b/test/unit/builtins/string.spec.ts @@ -1,3 +1,4 @@ +import { LuaLibImportKind } from "../../../src"; import * as util from "../../util"; test("Supported lua string function", () => { @@ -193,6 +194,12 @@ test.each([ util.testExpressionTemplate`${inp}.split(${separator})`.expectToMatchJsResult(); }); +test("string.split inline", () => { + util.testExpression`"a, b, c".split(",")` + .setOptions({ luaLibImport: LuaLibImportKind.Inline }) + .expectToMatchJsResult(); +}); + test.each([ { inp: "hello test", index: 0 }, { inp: "hello test", index: 1 }, From 6a905998e90cff728e4a383cdc7fcee53d0f88d2 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 25 Jan 2021 22:27:23 +0100 Subject: [PATCH 3/4] Fix isArray test for empty object actually testing an object --- test/unit/builtins/array.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index db9cc4f47..53c323e44 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -631,5 +631,5 @@ test("Array.isArray returns true for empty objects", () => { // being identical in Lua. We assume [] is more common than Array.isArray({}), // so it is more important to handle [] right, sacrificing the result for {}. // See discussion: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/737 - util.testExpression`Array.isArray([])`.expectToEqual(true); + util.testExpression`Array.isArray({})`.expectToEqual(true); }); From 5a2af7ef2e3924da5456cd3a8485b59a350fc6c3 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Tue, 26 Jan 2021 21:01:43 +0100 Subject: [PATCH 4/4] Used Array.isArray in array.prototype.concat --- src/LuaLib.ts | 1 + src/lualib/ArrayConcat.ts | 5 ++--- test/unit/builtins/array.spec.ts | 5 +++++ test/unit/spread.spec.ts | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/LuaLib.ts b/src/LuaLib.ts index ecc6aec30..8a3fee340 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -87,6 +87,7 @@ export enum LuaLibFeature { } const luaLibDependencies: Partial> = { + ArrayConcat: [LuaLibFeature.ArrayIsArray], ArrayFlat: [LuaLibFeature.ArrayConcat, LuaLibFeature.ArrayIsArray], ArrayFlatMap: [LuaLibFeature.ArrayConcat, LuaLibFeature.ArrayIsArray], Decorate: [LuaLibFeature.CloneDescriptor], diff --git a/src/lualib/ArrayConcat.ts b/src/lualib/ArrayConcat.ts index a133bb97a..30da83514 100644 --- a/src/lualib/ArrayConcat.ts +++ b/src/lualib/ArrayConcat.ts @@ -4,9 +4,8 @@ function __TS__ArrayConcat(this: void, arr1: any[], ...args: any[]): any[] { out[out.length] = val; } for (const arg of args) { - // Hack because we don't have an isArray function - if (pcall(() => (arg as any[]).length) && type(arg) !== "string") { - const argAsArray = arg as any[]; + if (Array.isArray(arg)) { + const argAsArray = arg; for (const val of argAsArray) { out[out.length] = val; } diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts index 53c323e44..2cabbac29 100644 --- a/test/unit/builtins/array.spec.ts +++ b/test/unit/builtins/array.spec.ts @@ -633,3 +633,8 @@ test("Array.isArray returns true for empty objects", () => { // See discussion: https://github.com/TypeScriptToLua/TypeScriptToLua/pull/737 util.testExpression`Array.isArray({})`.expectToEqual(true); }); + +// Test fix for https://github.com/TypeScriptToLua/TypeScriptToLua/issues/738 +test("array.prototype.concat issue #738", () => { + util.testExpression`([] as any[]).concat(13, 323, {x: 3}, [2, 3])`.expectToMatchJsResult(); +}); diff --git a/test/unit/spread.spec.ts b/test/unit/spread.spec.ts index b3b7c95a0..3384b60fb 100644 --- a/test/unit/spread.spec.ts +++ b/test/unit/spread.spec.ts @@ -93,7 +93,7 @@ describe("in array literal", () => { test("of array literal /w OmittedExpression", () => { util.testFunction` - const array = [1, 2, ...[3], , 5]; + const array = [1, 2, ...[3], 5, , 6]; return { a: array[0], b: array[1], c: array[2], d: array[3] }; `.expectToMatchJsResult(); });