From 8edbb8c232ca9138d028d791622dabecb757205e Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sat, 16 Mar 2019 13:16:45 -0600 Subject: [PATCH 1/3] fixed array access when mixed in a union with an empty tuple --- src/TSHelper.ts | 3 ++- test/unit/array.spec.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 2de95d570..db2f657d6 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -147,7 +147,8 @@ export class TSHelper { } public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker): boolean { - const typeNode = checker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.InTypeAlias); + const flags = ts.NodeBuilderFlags.InTypeAlias | ts.NodeBuilderFlags.AllowEmptyTuple; + const typeNode = checker.typeToTypeNode(type, undefined, flags); return typeNode && TSHelper.isArrayTypeNode(typeNode); } diff --git a/test/unit/array.spec.ts b/test/unit/array.spec.ts index eecdf4cd1..ca57027a7 100644 --- a/test/unit/array.spec.ts +++ b/test/unit/array.spec.ts @@ -21,6 +21,16 @@ export class ArrayTests { Expect(result).toBe(5); } + @Test("Array union access with empty tuple") + public arrayUnionAccessWithEmptyTuple(): void { + const result = util.transpileAndExecute( + `function makeArray(): number[] | [] { return [3,5,1]; } + const arr = makeArray(); + return arr[1];` + ); + Expect(result).toBe(5); + } + @Test("Array union length") public arrayUnionLength(): void { const result = util.transpileAndExecute( From f25b41a7b3cc25b0377e16c486d6d5712d87e599 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sat, 16 Mar 2019 15:31:19 -0600 Subject: [PATCH 2/3] reworked array detection to catch ReadonlyArray --- src/LuaTransformer.ts | 43 ++++++++++++++++++------------- src/TSHelper.ts | 59 +++++++++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index b367495e0..ab7587e97 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1088,7 +1088,7 @@ export class LuaTransformer { } const type = this.checker.getTypeAtLocation(node); - const context = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void + const context = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void ? this.createSelfIdentifier() : undefined; const [paramNames, dots, restParamName] = this.transformParameters(node.parameters, context); @@ -1594,7 +1594,7 @@ export class LuaTransformer { } const type = this.checker.getTypeAtLocation(functionDeclaration); - const context = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void + const context = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void ? this.createSelfIdentifier() : undefined; const [params, dotsLiteral, restParamName] = this.transformParameters(functionDeclaration.parameters, context); @@ -1794,7 +1794,7 @@ export class LuaTransformer { const expressionType = this.checker.getTypeAtLocation(statement.expression); this.validateFunctionAssignment(statement, expressionType, returnType); } - if (tsHelper.isInTupleReturnFunction(statement, this.checker)) { + if (tsHelper.isInTupleReturnFunction(statement, this.checker, this.program)) { // Parent function is a TupleReturn function if (ts.isArrayLiteralExpression(statement.expression)) { // If return expression is an array literal, leave out brackets. @@ -2080,7 +2080,11 @@ export class LuaTransformer { // LuaIterators return this.transformForOfLuaIteratorStatement(statement, body); - } else if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker)) { + } else if (tsHelper.isArrayType( + this.checker.getTypeAtLocation(statement.expression), + this.checker, + this.program) + ) { // Arrays return this.transformForOfArrayStatement(statement, body); @@ -2099,7 +2103,7 @@ export class LuaTransformer { const pairsIdentifier = tstl.createIdentifier("pairs"); const expression = tstl.createCallExpression(pairsIdentifier, [this.transformExpression(statement.expression)]); - if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker)) { + if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker, this.program)) { throw TSTLErrors.ForbiddenForIn(statement); } @@ -2514,7 +2518,7 @@ export class LuaTransformer { // Element access indexExpression = this.transformExpression(expression.left.argumentExpression); const argType = this.checker.getTypeAtLocation(expression.left.expression); - if (tsHelper.isArrayType(argType, this.checker)) { + if (tsHelper.isArrayType(argType, this.checker, this.program)) { // Array access needs a +1 indexExpression = this.expressionPlusOne(indexExpression); } @@ -2548,7 +2552,8 @@ export class LuaTransformer { const [hasEffects, objExpression, indexExpression] = tsHelper.isAccessExpressionWithEvaluationEffects( lhs, - this.checker + this.checker, + this.program ); if (hasEffects) { // Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects @@ -2709,7 +2714,8 @@ export class LuaTransformer { const [hasEffects, objExpression, indexExpression] = tsHelper.isAccessExpressionWithEvaluationEffects( lhs, - this.checker + this.checker, + this.program ); if (hasEffects) { // Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects @@ -2999,7 +3005,7 @@ export class LuaTransformer { ): ExpressionVisitResult { const type = this.checker.getTypeAtLocation(node); - const hasContext = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void; + const hasContext = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void; // Build parameter string const [paramNames, dotsLiteral, spreadIdentifier] = this.transformParameters( node.parameters, @@ -3091,8 +3097,9 @@ export class LuaTransformer { let parameters: tstl.Expression[] = []; const isTupleReturn = tsHelper.isTupleReturnCall(node, this.checker); - const isTupleReturnForward = - node.parent && ts.isReturnStatement(node.parent) && tsHelper.isInTupleReturnFunction(node, this.checker); + const isTupleReturnForward = node.parent + && ts.isReturnStatement(node.parent) + && tsHelper.isInTupleReturnFunction(node, this.checker, this.program); const isInDestructingAssignment = tsHelper.isInDestructingAssignment(node); const isInSpread = node.parent && ts.isSpreadElement(node.parent); const returnValueIsUsed = node.parent && !ts.isExpressionStatement(node.parent); @@ -3186,12 +3193,12 @@ export class LuaTransformer { } // if ownerType is a array, use only supported functions - if (tsHelper.isExplicitArrayType(ownerType, this.checker)) { + if (tsHelper.isExplicitArrayType(ownerType, this.checker, this.program)) { return this.transformArrayCallExpression(node); } // if ownerType inherits from an array, use array calls where appropriate - if (tsHelper.isArrayType(ownerType, this.checker) && + if (tsHelper.isArrayType(ownerType, this.checker, this.program) && tsHelper.isDefaultArrayCallMethodName(node.expression.name.escapedText as string)) { return this.transformArrayCallExpression(node); } @@ -3320,7 +3327,7 @@ export class LuaTransformer { if (tsHelper.isStringType(type)) { return this.transformStringProperty(node); - } else if (tsHelper.isArrayType(type, this.checker)) { + } else if (tsHelper.isArrayType(type, this.checker, this.program)) { const arrayPropertyAccess = this.transformArrayProperty(node); if (arrayPropertyAccess) { return arrayPropertyAccess; @@ -3485,7 +3492,7 @@ export class LuaTransformer { return this.transformConstEnumValue(type, node.argumentExpression.text, node); } - if (tsHelper.isArrayType(type, this.checker)) { + if (tsHelper.isArrayType(type, this.checker, this.program)) { return tstl.createTableIndexExpression(table, this.expressionPlusOne(index), node); } else if (tsHelper.isStringType(type)) { return tstl.createCallExpression( @@ -3878,7 +3885,7 @@ export class LuaTransformer { public transformFunctionCallExpression(node: ts.CallExpression): tstl.CallExpression { const expression = node.expression as ts.PropertyAccessExpression; const callerType = this.checker.getTypeAtLocation(expression.expression); - if (tsHelper.getFunctionContextType(callerType, this.checker) === ContextType.Void) { + if (tsHelper.getFunctionContextType(callerType, this.checker, this.program) === ContextType.Void) { throw TSTLErrors.UnsupportedSelfFunctionConversion(node); } const params = this.transformArguments(node.arguments); @@ -4313,8 +4320,8 @@ export class LuaTransformer { fromTypeCache.add(toType); // Check function assignments - const fromContext = tsHelper.getFunctionContextType(fromType, this.checker); - const toContext = tsHelper.getFunctionContextType(toType, this.checker); + const fromContext = tsHelper.getFunctionContextType(fromType, this.checker, this.program); + const toContext = tsHelper.getFunctionContextType(toType, this.checker, this.program); if (fromContext === ContextType.Mixed || toContext === ContextType.Mixed) { throw TSTLErrors.UnsupportedOverloadAssignment(node, toName); diff --git a/src/TSHelper.ts b/src/TSHelper.ts index db2f657d6..154a5efc9 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -140,16 +140,18 @@ export class TSHelper { (type.flags & ts.TypeFlags.StringLiteral) !== 0; } - public static isArrayTypeNode(typeNode: ts.TypeNode): boolean { - return typeNode.kind === ts.SyntaxKind.ArrayType || typeNode.kind === ts.SyntaxKind.TupleType || - ((typeNode.kind === ts.SyntaxKind.UnionType || typeNode.kind === ts.SyntaxKind.IntersectionType) && - (typeNode as ts.UnionOrIntersectionTypeNode).types.some(TSHelper.isArrayTypeNode)); - } + public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): boolean { + if (type.isUnionOrIntersection()) { + return type.types.some(t => TSHelper.isExplicitArrayType(t, checker, program)); + } + + if (TSHelper.isStandardLibraryType(type, "ReadonlyArray", program)) { + return true; + } - public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker): boolean { const flags = ts.NodeBuilderFlags.InTypeAlias | ts.NodeBuilderFlags.AllowEmptyTuple; const typeNode = checker.typeToTypeNode(type, undefined, flags); - return typeNode && TSHelper.isArrayTypeNode(typeNode); + return typeNode && (ts.isArrayTypeNode(typeNode) || ts.isTupleTypeNode(typeNode)); } public static isFunctionType(type: ts.Type, checker: ts.TypeChecker): boolean { @@ -162,8 +164,8 @@ export class TSHelper { return TSHelper.isFunctionType(type, checker); } - public static isArrayType(type: ts.Type, checker: ts.TypeChecker): boolean { - return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.isExplicitArrayType(t, checker)); + public static isArrayType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): boolean { + return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.isExplicitArrayType(t, checker, program)); } public static isLuaIteratorType(node: ts.Node, checker: ts.TypeChecker): boolean { @@ -181,12 +183,12 @@ export class TSHelper { } } - public static isInTupleReturnFunction(node: ts.Node, checker: ts.TypeChecker): boolean { + public static isInTupleReturnFunction(node: ts.Node, checker: ts.TypeChecker, program: ts.Program): boolean { const declaration = TSHelper.findFirstNodeAbove(node, ts.isFunctionLike); if (declaration) { let functionType: ts.Type; if (ts.isFunctionExpression(declaration) || ts.isArrowFunction(declaration)) { - functionType = TSHelper.inferAssignedType(declaration, checker); + functionType = TSHelper.inferAssignedType(declaration, checker, program); } else { functionType = checker.getTypeAtLocation(declaration); } @@ -331,13 +333,17 @@ export class TSHelper { // If expression is property/element access with possible effects from being evaluated, returns true along with the // separated object and index expressions. - public static isAccessExpressionWithEvaluationEffects(node: ts.Expression, checker: ts.TypeChecker): - [boolean, ts.Expression, ts.Expression] { + public static isAccessExpressionWithEvaluationEffects( + node: ts.Expression, + checker: ts.TypeChecker, + program: ts.Program + ): [boolean, ts.Expression, ts.Expression] + { if (ts.isElementAccessExpression(node) && (TSHelper.isExpressionWithEvaluationEffect(node.expression) || TSHelper.isExpressionWithEvaluationEffect(node.argumentExpression))) { const type = checker.getTypeAtLocation(node.expression); - if (TSHelper.isArrayType(type, checker)) { + if (TSHelper.isArrayType(type, checker, program)) { // Offset arrays by one const oneLit = ts.createNumericLiteral("1"); const exp = ts.createParen(node.argumentExpression); @@ -447,10 +453,10 @@ export class TSHelper { ) !== undefined; } - public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker): ts.Type { + public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker, program: ts.Program): ts.Type { if (ts.isParenthesizedExpression(expression.parent)) { // Ignore expressions wrapped in parenthesis - return this.inferAssignedType(expression.parent, checker); + return this.inferAssignedType(expression.parent, checker, program); } else if (ts.isCallOrNewExpression(expression.parent)) { // Expression being passed as argument to a function @@ -463,7 +469,7 @@ export class TSHelper { parentSignature.parameters[signatureIndex], expression ); - if (TSHelper.isArrayType(parameterType, checker)) { + if (TSHelper.isArrayType(parameterType, checker, program)) { // Check for elipses argument const parentSignatureDeclaration = parentSignature.getDeclaration(); if (parentSignatureDeclaration) { @@ -497,7 +503,7 @@ export class TSHelper { } else if (ts.isPropertyAssignment(expression.parent)) { // Expression being assigned to an object literal property - const objType = this.inferAssignedType(expression.parent.parent, checker); + const objType = this.inferAssignedType(expression.parent.parent, checker, program); const property = objType.getProperty(expression.parent.name.getText()); if (!property) { const stringPropertyType = objType.getStringIndexType(); @@ -510,7 +516,7 @@ export class TSHelper { } else if (ts.isArrayLiteralExpression(expression.parent)) { // Expression in an array literal - const arrayType = this.inferAssignedType(expression.parent, checker); + const arrayType = this.inferAssignedType(expression.parent, checker, program); if (ts.isTupleTypeNode(checker.typeToTypeNode(arrayType))) { // Tuples const i = expression.parent.elements.indexOf(expression); @@ -531,7 +537,7 @@ export class TSHelper { return checker.getTypeAtLocation(expression.parent.left); } else { // Other binary expressions - return TSHelper.inferAssignedType(expression.parent, checker); + return TSHelper.inferAssignedType(expression.parent, checker, program); } } else if (ts.isAssertionExpression(expression.parent)) { @@ -551,7 +557,8 @@ export class TSHelper { public static getSignatureDeclarations( signatures: ReadonlyArray, - checker: ts.TypeChecker + checker: ts.TypeChecker, + program: ts.Program ): ts.SignatureDeclaration[] { const signatureDeclarations: ts.SignatureDeclaration[] = []; @@ -561,7 +568,7 @@ export class TSHelper { && !TSHelper.getExplicitThisParameter(signatureDeclaration)) { // Infer type of function expressions/arrow functions - const inferredType = TSHelper.inferAssignedType(signatureDeclaration, checker); + const inferredType = TSHelper.inferAssignedType(signatureDeclaration, checker, program); if (inferredType) { const inferredSignatures = TSHelper.getAllCallSignatures(inferredType); if (inferredSignatures.length > 0) { @@ -651,20 +658,22 @@ export class TSHelper { return contexts.reduce(reducer, ContextType.None); } - public static getFunctionContextType(type: ts.Type, checker: ts.TypeChecker): ContextType { + public static getFunctionContextType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): ContextType { if (type.isTypeParameter()) { type = type.getConstraint() || type; } if (type.isUnion()) { - return TSHelper.reduceContextTypes(type.types.map(t => TSHelper.getFunctionContextType(t, checker))); + return TSHelper.reduceContextTypes( + type.types.map(t => TSHelper.getFunctionContextType(t, checker, program)) + ); } const signatures = checker.getSignaturesOfType(type, ts.SignatureKind.Call); if (signatures.length === 0) { return ContextType.None; } - const signatureDeclarations = TSHelper.getSignatureDeclarations(signatures, checker); + const signatureDeclarations = TSHelper.getSignatureDeclarations(signatures, checker, program); return TSHelper.reduceContextTypes( signatureDeclarations.map(s => TSHelper.getDeclarationContextType(s, checker))); } From e7114cb31d4057b63b4af87ee6d4e53cae106d06 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sat, 16 Mar 2019 15:36:52 -0600 Subject: [PATCH 3/3] ReadonlyArray test --- test/unit/array.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/unit/array.spec.ts b/test/unit/array.spec.ts index ca57027a7..4ddb7b8fd 100644 --- a/test/unit/array.spec.ts +++ b/test/unit/array.spec.ts @@ -11,6 +11,15 @@ export class ArrayTests { Expect(result).toBe(5); } + @Test("Readonly Array access") + public readonlyArrayAccess(): void { + const result = util.transpileAndExecute( + `const arr: ReadonlyArray = [3,5,1]; + return arr[1];` + ); + Expect(result).toBe(5); + } + @Test("Array union access") public arrayUnionAccess(): void { const result = util.transpileAndExecute(