Skip to content

Commit 8b42131

Browse files
tomblindtomblind
authored
Array detection fixes (#483)
* fixed array access when mixed in a union with an empty tuple * reworked array detection to catch ReadonlyArray * ReadonlyArray test
1 parent d577493 commit 8b42131

File tree

3 files changed

+80
-44
lines changed

3 files changed

+80
-44
lines changed

‎src/LuaTransformer.ts‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,7 +1088,7 @@ export class LuaTransformer {
10881088
}
10891089

10901090
const type = this.checker.getTypeAtLocation(node);
1091-
const context = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void
1091+
const context = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void
10921092
? this.createSelfIdentifier()
10931093
: undefined;
10941094
const [paramNames, dots, restParamName] = this.transformParameters(node.parameters, context);
@@ -1594,7 +1594,7 @@ export class LuaTransformer {
15941594
}
15951595

15961596
const type = this.checker.getTypeAtLocation(functionDeclaration);
1597-
const context = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void
1597+
const context = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void
15981598
? this.createSelfIdentifier()
15991599
: undefined;
16001600
const [params, dotsLiteral, restParamName] = this.transformParameters(functionDeclaration.parameters, context);
@@ -1794,7 +1794,7 @@ export class LuaTransformer {
17941794
const expressionType = this.checker.getTypeAtLocation(statement.expression);
17951795
this.validateFunctionAssignment(statement, expressionType, returnType);
17961796
}
1797-
if (tsHelper.isInTupleReturnFunction(statement, this.checker)) {
1797+
if (tsHelper.isInTupleReturnFunction(statement, this.checker, this.program)) {
17981798
// Parent function is a TupleReturn function
17991799
if (ts.isArrayLiteralExpression(statement.expression)) {
18001800
// If return expression is an array literal, leave out brackets.
@@ -2080,7 +2080,11 @@ export class LuaTransformer {
20802080
// LuaIterators
20812081
return this.transformForOfLuaIteratorStatement(statement, body);
20822082

2083-
} else if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker)) {
2083+
} else if (tsHelper.isArrayType(
2084+
this.checker.getTypeAtLocation(statement.expression),
2085+
this.checker,
2086+
this.program)
2087+
) {
20842088
// Arrays
20852089
return this.transformForOfArrayStatement(statement, body);
20862090

@@ -2099,7 +2103,7 @@ export class LuaTransformer {
20992103
const pairsIdentifier = tstl.createIdentifier("pairs");
21002104
const expression = tstl.createCallExpression(pairsIdentifier, [this.transformExpression(statement.expression)]);
21012105

2102-
if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker)) {
2106+
if (tsHelper.isArrayType(this.checker.getTypeAtLocation(statement.expression), this.checker, this.program)) {
21032107
throw TSTLErrors.ForbiddenForIn(statement);
21042108
}
21052109

@@ -2514,7 +2518,7 @@ export class LuaTransformer {
25142518
// Element access
25152519
indexExpression = this.transformExpression(expression.left.argumentExpression);
25162520
const argType = this.checker.getTypeAtLocation(expression.left.expression);
2517-
if (tsHelper.isArrayType(argType, this.checker)) {
2521+
if (tsHelper.isArrayType(argType, this.checker, this.program)) {
25182522
// Array access needs a +1
25192523
indexExpression = this.expressionPlusOne(indexExpression);
25202524
}
@@ -2548,7 +2552,8 @@ export class LuaTransformer {
25482552

25492553
const [hasEffects, objExpression, indexExpression] = tsHelper.isAccessExpressionWithEvaluationEffects(
25502554
lhs,
2551-
this.checker
2555+
this.checker,
2556+
this.program
25522557
);
25532558
if (hasEffects) {
25542559
// Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects
@@ -2709,7 +2714,8 @@ export class LuaTransformer {
27092714

27102715
const [hasEffects, objExpression, indexExpression] = tsHelper.isAccessExpressionWithEvaluationEffects(
27112716
lhs,
2712-
this.checker
2717+
this.checker,
2718+
this.program
27132719
);
27142720
if (hasEffects) {
27152721
// Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects
@@ -2999,7 +3005,7 @@ export class LuaTransformer {
29993005
): ExpressionVisitResult
30003006
{
30013007
const type = this.checker.getTypeAtLocation(node);
3002-
const hasContext = tsHelper.getFunctionContextType(type, this.checker) !== ContextType.Void;
3008+
const hasContext = tsHelper.getFunctionContextType(type, this.checker, this.program) !== ContextType.Void;
30033009
// Build parameter string
30043010
const [paramNames, dotsLiteral, spreadIdentifier] = this.transformParameters(
30053011
node.parameters,
@@ -3091,8 +3097,9 @@ export class LuaTransformer {
30913097
let parameters: tstl.Expression[] = [];
30923098

30933099
const isTupleReturn = tsHelper.isTupleReturnCall(node, this.checker);
3094-
const isTupleReturnForward =
3095-
node.parent && ts.isReturnStatement(node.parent) && tsHelper.isInTupleReturnFunction(node, this.checker);
3100+
const isTupleReturnForward = node.parent
3101+
&& ts.isReturnStatement(node.parent)
3102+
&& tsHelper.isInTupleReturnFunction(node, this.checker, this.program);
30963103
const isInDestructingAssignment = tsHelper.isInDestructingAssignment(node);
30973104
const isInSpread = node.parent && ts.isSpreadElement(node.parent);
30983105
const returnValueIsUsed = node.parent && !ts.isExpressionStatement(node.parent);
@@ -3186,12 +3193,12 @@ export class LuaTransformer {
31863193
}
31873194

31883195
// if ownerType is a array, use only supported functions
3189-
if (tsHelper.isExplicitArrayType(ownerType, this.checker)) {
3196+
if (tsHelper.isExplicitArrayType(ownerType, this.checker, this.program)) {
31903197
return this.transformArrayCallExpression(node);
31913198
}
31923199

31933200
// if ownerType inherits from an array, use array calls where appropriate
3194-
if (tsHelper.isArrayType(ownerType, this.checker) &&
3201+
if (tsHelper.isArrayType(ownerType, this.checker, this.program) &&
31953202
tsHelper.isDefaultArrayCallMethodName(node.expression.name.escapedText as string)) {
31963203
return this.transformArrayCallExpression(node);
31973204
}
@@ -3320,7 +3327,7 @@ export class LuaTransformer {
33203327
if (tsHelper.isStringType(type)) {
33213328
return this.transformStringProperty(node);
33223329

3323-
} else if (tsHelper.isArrayType(type, this.checker)) {
3330+
} else if (tsHelper.isArrayType(type, this.checker, this.program)) {
33243331
const arrayPropertyAccess = this.transformArrayProperty(node);
33253332
if (arrayPropertyAccess) {
33263333
return arrayPropertyAccess;
@@ -3485,7 +3492,7 @@ export class LuaTransformer {
34853492
return this.transformConstEnumValue(type, node.argumentExpression.text, node);
34863493
}
34873494

3488-
if (tsHelper.isArrayType(type, this.checker)) {
3495+
if (tsHelper.isArrayType(type, this.checker, this.program)) {
34893496
return tstl.createTableIndexExpression(table, this.expressionPlusOne(index), node);
34903497
} else if (tsHelper.isStringType(type)) {
34913498
return tstl.createCallExpression(
@@ -3878,7 +3885,7 @@ export class LuaTransformer {
38783885
public transformFunctionCallExpression(node: ts.CallExpression): tstl.CallExpression {
38793886
const expression = node.expression as ts.PropertyAccessExpression;
38803887
const callerType = this.checker.getTypeAtLocation(expression.expression);
3881-
if (tsHelper.getFunctionContextType(callerType, this.checker) === ContextType.Void) {
3888+
if (tsHelper.getFunctionContextType(callerType, this.checker, this.program) === ContextType.Void) {
38823889
throw TSTLErrors.UnsupportedSelfFunctionConversion(node);
38833890
}
38843891
const params = this.transformArguments(node.arguments);
@@ -4313,8 +4320,8 @@ export class LuaTransformer {
43134320
fromTypeCache.add(toType);
43144321

43154322
// Check function assignments
4316-
const fromContext = tsHelper.getFunctionContextType(fromType, this.checker);
4317-
const toContext = tsHelper.getFunctionContextType(toType, this.checker);
4323+
const fromContext = tsHelper.getFunctionContextType(fromType, this.checker, this.program);
4324+
const toContext = tsHelper.getFunctionContextType(toType, this.checker, this.program);
43184325

43194326
if (fromContext === ContextType.Mixed || toContext === ContextType.Mixed) {
43204327
throw TSTLErrors.UnsupportedOverloadAssignment(node, toName);

‎src/TSHelper.ts‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,18 @@ export class TSHelper {
140140
(type.flags & ts.TypeFlags.StringLiteral) !== 0;
141141
}
142142

143-
public static isArrayTypeNode(typeNode: ts.TypeNode): boolean {
144-
return typeNode.kind === ts.SyntaxKind.ArrayType || typeNode.kind === ts.SyntaxKind.TupleType ||
145-
((typeNode.kind === ts.SyntaxKind.UnionType || typeNode.kind === ts.SyntaxKind.IntersectionType) &&
146-
(typeNode as ts.UnionOrIntersectionTypeNode).types.some(TSHelper.isArrayTypeNode));
147-
}
143+
public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): boolean {
144+
if (type.isUnionOrIntersection()) {
145+
return type.types.some(t => TSHelper.isExplicitArrayType(t, checker, program));
146+
}
148147

149-
public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker): boolean {
150-
const typeNode = checker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.InTypeAlias);
151-
return typeNode && TSHelper.isArrayTypeNode(typeNode);
148+
if (TSHelper.isStandardLibraryType(type, "ReadonlyArray", program)) {
149+
return true;
150+
}
151+
152+
const flags = ts.NodeBuilderFlags.InTypeAlias | ts.NodeBuilderFlags.AllowEmptyTuple;
153+
const typeNode = checker.typeToTypeNode(type, undefined, flags);
154+
return typeNode && (ts.isArrayTypeNode(typeNode) || ts.isTupleTypeNode(typeNode));
152155
}
153156

154157
public static isFunctionType(type: ts.Type, checker: ts.TypeChecker): boolean {
@@ -161,8 +164,8 @@ export class TSHelper {
161164
return TSHelper.isFunctionType(type, checker);
162165
}
163166

164-
public static isArrayType(type: ts.Type, checker: ts.TypeChecker): boolean {
165-
return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.isExplicitArrayType(t, checker));
167+
public static isArrayType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): boolean {
168+
return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.isExplicitArrayType(t, checker, program));
166169
}
167170

168171
public static isLuaIteratorType(node: ts.Node, checker: ts.TypeChecker): boolean {
@@ -180,12 +183,12 @@ export class TSHelper {
180183
}
181184
}
182185

183-
public static isInTupleReturnFunction(node: ts.Node, checker: ts.TypeChecker): boolean {
186+
public static isInTupleReturnFunction(node: ts.Node, checker: ts.TypeChecker, program: ts.Program): boolean {
184187
const declaration = TSHelper.findFirstNodeAbove(node, ts.isFunctionLike);
185188
if (declaration) {
186189
let functionType: ts.Type;
187190
if (ts.isFunctionExpression(declaration) || ts.isArrowFunction(declaration)) {
188-
functionType = TSHelper.inferAssignedType(declaration, checker);
191+
functionType = TSHelper.inferAssignedType(declaration, checker, program);
189192
} else {
190193
functionType = checker.getTypeAtLocation(declaration);
191194
}
@@ -330,13 +333,17 @@ export class TSHelper {
330333

331334
// If expression is property/element access with possible effects from being evaluated, returns true along with the
332335
// separated object and index expressions.
333-
public static isAccessExpressionWithEvaluationEffects(node: ts.Expression, checker: ts.TypeChecker):
334-
[boolean, ts.Expression, ts.Expression] {
336+
public static isAccessExpressionWithEvaluationEffects(
337+
node: ts.Expression,
338+
checker: ts.TypeChecker,
339+
program: ts.Program
340+
): [boolean, ts.Expression, ts.Expression]
341+
{
335342
if (ts.isElementAccessExpression(node) &&
336343
(TSHelper.isExpressionWithEvaluationEffect(node.expression)
337344
|| TSHelper.isExpressionWithEvaluationEffect(node.argumentExpression))) {
338345
const type = checker.getTypeAtLocation(node.expression);
339-
if (TSHelper.isArrayType(type, checker)) {
346+
if (TSHelper.isArrayType(type, checker, program)) {
340347
// Offset arrays by one
341348
const oneLit = ts.createNumericLiteral("1");
342349
const exp = ts.createParen(node.argumentExpression);
@@ -446,10 +453,10 @@ export class TSHelper {
446453
) !== undefined;
447454
}
448455

449-
public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker): ts.Type {
456+
public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker, program: ts.Program): ts.Type {
450457
if (ts.isParenthesizedExpression(expression.parent)) {
451458
// Ignore expressions wrapped in parenthesis
452-
return this.inferAssignedType(expression.parent, checker);
459+
return this.inferAssignedType(expression.parent, checker, program);
453460

454461
} else if (ts.isCallOrNewExpression(expression.parent)) {
455462
// Expression being passed as argument to a function
@@ -462,7 +469,7 @@ export class TSHelper {
462469
parentSignature.parameters[signatureIndex],
463470
expression
464471
);
465-
if (TSHelper.isArrayType(parameterType, checker)) {
472+
if (TSHelper.isArrayType(parameterType, checker, program)) {
466473
// Check for elipses argument
467474
const parentSignatureDeclaration = parentSignature.getDeclaration();
468475
if (parentSignatureDeclaration) {
@@ -496,7 +503,7 @@ export class TSHelper {
496503

497504
} else if (ts.isPropertyAssignment(expression.parent)) {
498505
// Expression being assigned to an object literal property
499-
const objType = this.inferAssignedType(expression.parent.parent, checker);
506+
const objType = this.inferAssignedType(expression.parent.parent, checker, program);
500507
const property = objType.getProperty(expression.parent.name.getText());
501508
if (!property) {
502509
const stringPropertyType = objType.getStringIndexType();
@@ -509,7 +516,7 @@ export class TSHelper {
509516

510517
} else if (ts.isArrayLiteralExpression(expression.parent)) {
511518
// Expression in an array literal
512-
const arrayType = this.inferAssignedType(expression.parent, checker);
519+
const arrayType = this.inferAssignedType(expression.parent, checker, program);
513520
if (ts.isTupleTypeNode(checker.typeToTypeNode(arrayType))) {
514521
// Tuples
515522
const i = expression.parent.elements.indexOf(expression);
@@ -530,7 +537,7 @@ export class TSHelper {
530537
return checker.getTypeAtLocation(expression.parent.left);
531538
} else {
532539
// Other binary expressions
533-
return TSHelper.inferAssignedType(expression.parent, checker);
540+
return TSHelper.inferAssignedType(expression.parent, checker, program);
534541
}
535542

536543
} else if (ts.isAssertionExpression(expression.parent)) {
@@ -550,7 +557,8 @@ export class TSHelper {
550557

551558
public static getSignatureDeclarations(
552559
signatures: ReadonlyArray<ts.Signature>,
553-
checker: ts.TypeChecker
560+
checker: ts.TypeChecker,
561+
program: ts.Program
554562
): ts.SignatureDeclaration[]
555563
{
556564
const signatureDeclarations: ts.SignatureDeclaration[] = [];
@@ -560,7 +568,7 @@ export class TSHelper {
560568
&& !TSHelper.getExplicitThisParameter(signatureDeclaration))
561569
{
562570
// Infer type of function expressions/arrow functions
563-
const inferredType = TSHelper.inferAssignedType(signatureDeclaration, checker);
571+
const inferredType = TSHelper.inferAssignedType(signatureDeclaration, checker, program);
564572
if (inferredType) {
565573
const inferredSignatures = TSHelper.getAllCallSignatures(inferredType);
566574
if (inferredSignatures.length > 0) {
@@ -650,20 +658,22 @@ export class TSHelper {
650658
return contexts.reduce(reducer, ContextType.None);
651659
}
652660

653-
public static getFunctionContextType(type: ts.Type, checker: ts.TypeChecker): ContextType {
661+
public static getFunctionContextType(type: ts.Type, checker: ts.TypeChecker, program: ts.Program): ContextType {
654662
if (type.isTypeParameter()) {
655663
type = type.getConstraint() || type;
656664
}
657665

658666
if (type.isUnion()) {
659-
return TSHelper.reduceContextTypes(type.types.map(t => TSHelper.getFunctionContextType(t, checker)));
667+
return TSHelper.reduceContextTypes(
668+
type.types.map(t => TSHelper.getFunctionContextType(t, checker, program))
669+
);
660670
}
661671

662672
const signatures = checker.getSignaturesOfType(type, ts.SignatureKind.Call);
663673
if (signatures.length === 0) {
664674
return ContextType.None;
665675
}
666-
const signatureDeclarations = TSHelper.getSignatureDeclarations(signatures, checker);
676+
const signatureDeclarations = TSHelper.getSignatureDeclarations(signatures, checker, program);
667677
return TSHelper.reduceContextTypes(
668678
signatureDeclarations.map(s => TSHelper.getDeclarationContextType(s, checker)));
669679
}

‎test/unit/array.spec.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ export class ArrayTests {
1111
Expect(result).toBe(5);
1212
}
1313

14+
@Test("Readonly Array access")
15+
public readonlyArrayAccess(): void {
16+
const result = util.transpileAndExecute(
17+
`const arr: ReadonlyArray<number> = [3,5,1];
18+
return arr[1];`
19+
);
20+
Expect(result).toBe(5);
21+
}
22+
1423
@Test("Array union access")
1524
public arrayUnionAccess(): void {
1625
const result = util.transpileAndExecute(
@@ -21,6 +30,16 @@ export class ArrayTests {
2130
Expect(result).toBe(5);
2231
}
2332

33+
@Test("Array union access with empty tuple")
34+
public arrayUnionAccessWithEmptyTuple(): void {
35+
const result = util.transpileAndExecute(
36+
`function makeArray(): number[] | [] { return [3,5,1]; }
37+
const arr = makeArray();
38+
return arr[1];`
39+
);
40+
Expect(result).toBe(5);
41+
}
42+
2443
@Test("Array union length")
2544
public arrayUnionLength(): void {
2645
const result = util.transpileAndExecute(

0 commit comments

Comments
 (0)