Fix array.join accepting only string and number elements (#887)

ark120202 · web-flow · commit b3b4a21f6d5f · 2020-05-24T15:56:22.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,8 @@
 
 - Fixed iteration over generators stopping at first yielded `nil` value
 
+- Fixed `Array.prototype.join` throwing an error when array contains anything other than strings and numbers
+
 - Fixed extending a class not keeping `toString` implementation from a super class
 
 ## 0.33.0
diff --git a/src/LuaLib.ts b/src/LuaLib.ts
@@ -10,6 +10,7 @@ export enum LuaLibFeature {
     ArrayFindIndex = "ArrayFindIndex",
     ArrayIncludes = "ArrayIncludes",
     ArrayIndexOf = "ArrayIndexOf",
+    ArrayJoin = "ArrayJoin",
     ArrayMap = "ArrayMap",
     ArrayPush = "ArrayPush",
     ArrayReduce = "ArrayReduce",
diff --git a/src/lualib/ArrayJoin.ts b/src/lualib/ArrayJoin.ts
@@ -0,0 +1,8 @@
+function __TS__ArrayJoin(this: unknown[], separator = ",") {
+    let result = "";
+    for (const [index, value] of ipairs(this)) {
+        if (index > 1) result += separator;
+        result += value.toString();
+    }
+    return result;
+}
diff --git a/src/lualib/Iterator.ts b/src/lualib/Iterator.ts
@@ -27,6 +27,6 @@ function __TS__Iterator<T>(
         const iterator = iterable[Symbol.iterator]();
         return [__TS__IteratorIteratorStep, iterator];
     } else {
-        return ipairs(iterable as readonly T[]);
+        return ipairs(iterable as readonly T[]) as any;
     }
 }
diff --git a/src/lualib/declarations/global.d.ts b/src/lualib/declarations/global.d.ts
@@ -29,4 +29,6 @@ declare function select<T>(index: "#", ...args: T[]): number;
  * @luaIterator
  * @tupleReturn
  */
-declare function ipairs<T>(t: Record<number, T>): LuaTupleIterable<[number, T]>;
+type LuaTupleIterator<T extends any[]> = Iterable<T> & { " LuaTupleIterator": never };
+
+declare function ipairs<T>(t: Record<number, T>): LuaTupleIterator<[number, T]>;
diff --git a/src/transformation/builtins/array.ts b/src/transformation/builtins/array.ts
@@ -4,6 +4,7 @@ import { TransformationContext } from "../context";
 import { unsupportedProperty } from "../utils/diagnostics";
 import { LuaLibFeature, transformLuaLibFunction } from "../utils/lualib";
 import { PropertyCallExpression, transformArguments } from "../visitors/call";
+import { isStringType, isNumberType } from "../utils/typescript";
 
 export function transformArrayPrototypeCall(
     context: TransformationContext,
@@ -61,19 +62,25 @@ export function transformArrayPrototypeCall(
         case "splice":
             return transformLuaLibFunction(context, LuaLibFeature.ArraySplice, node, caller, ...params);
         case "join":
-            const defaultSeparatorLiteral = lua.createStringLiteral(",");
-            const parameters = [
-                caller,
-                node.arguments.length === 0
-                    ? defaultSeparatorLiteral
-                    : lua.createBinaryExpression(params[0], defaultSeparatorLiteral, lua.SyntaxKind.OrOperator),
-            ];
+            const callerType = context.checker.getTypeAtLocation(expression.expression);
+            const elementType = context.checker.getElementTypeOfArrayType(callerType);
+            if (elementType && (isStringType(context, elementType) || isNumberType(context, elementType))) {
+                const defaultSeparatorLiteral = lua.createStringLiteral(",");
+                const parameters = [
+                    caller,
+                    node.arguments.length === 0
+                        ? defaultSeparatorLiteral
+                        : lua.createBinaryExpression(params[0], defaultSeparatorLiteral, lua.SyntaxKind.OrOperator),
+                ];
 
-            return lua.createCallExpression(
-                lua.createTableIndexExpression(lua.createIdentifier("table"), lua.createStringLiteral("concat")),
-                parameters,
-                node
-            );
+                return lua.createCallExpression(
+                    lua.createTableIndexExpression(lua.createIdentifier("table"), lua.createStringLiteral("concat")),
+                    parameters,
+                    node
+                );
+            }
+
+            return transformLuaLibFunction(context, LuaLibFeature.ArrayJoin, node, caller, ...params);
         case "flat":
             return transformLuaLibFunction(context, LuaLibFeature.ArrayFlat, node, caller, ...params);
         case "flatMap":
diff --git a/src/typescript-internal.d.ts b/src/typescript-internal.d.ts
@@ -20,4 +20,8 @@ declare module "typescript" {
         configFile?: TsConfigSourceFile;
         configFilePath?: string;
     }
+
+    interface TypeChecker {
+        getElementTypeOfArrayType(type: Type): Type | undefined;
+    }
 }
diff --git a/test/unit/builtins/array.spec.ts b/test/unit/builtins/array.spec.ts
@@ -424,10 +424,19 @@ test.each([
     { array: ["test1", "test2"] },
     { array: ["test1", "test2"], separator: ";" },
     { array: ["test1", "test2"], separator: "" },
+    { array: [1, "2"] },
 ])("array.join (%p)", ({ array, separator }) => {
     util.testExpression`${util.formatCode(array)}.join(${util.formatCode(separator)})`.expectToMatchJsResult();
 });
 
+test('array.join (1, "2", {})', () => {
+    util.testExpression`[1, "2", {}].join()`.expectToEqual("1,2,table: 0x168");
+});
+
+test('array.join (1, "2", Symbol("foo"))', () => {
+    util.testExpression`[1, "2", Symbol("foo")].join()`.expectToEqual("1,2,Symbol(foo)");
+});
+
 test("array.join without separator argument", () => {
     util.testExpression`["test1", "test2"].join()`.expectToMatchJsResult();
 });

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,6 @@ function __TS__Iterator<T>(`
`27`	`27`	`const iterator = iterable[Symbol.iterator]();`
`28`	`28`	`return [__TS__IteratorIteratorStep, iterator];`
`29`	`29`	`} else {`
`30`		`- return ipairs(iterable as readonly T[]);`
	`30`	`+ return ipairs(iterable as readonly T[]) as any;`
`31`	`31`	`}`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,4 +20,8 @@ declare module "typescript" {`
`20`	`20`	`configFile?: TsConfigSourceFile;`
`21`	`21`	`configFilePath?: string;`
`22`	`22`	`}`
	`23`	`+`
	`24`	`+ interface TypeChecker {`
	`25`	`+ getElementTypeOfArrayType(type: Type): Type \| undefined;`
	`26`	`+ }`
`23`	`27`	`}`