Skip to content

Array detection fixes #483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tomblind merged 3 commits into from
Mar 17, 2019
Merged

Array detection fixes #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8edbb8c
fixed array access when mixed in a union with an empty tuple
tomblind Mar 16, 2019
f25b41a
reworked array detection to catch ReadonlyArray
tomblind Mar 16, 2019
e7114cb
ReadonlyArray test
tomblind Mar 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions src/LuaTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
62 changes: 36 additions & 26 deletions src/TSHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +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));
}

public static isExplicitArrayType(type: ts.Type, checker: ts.TypeChecker): boolean {
const typeNode = checker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.InTypeAlias);
return typeNode && TSHelper.isArrayTypeNode(typeNode);
if (TSHelper.isStandardLibraryType(type, "ReadonlyArray", program)) {
return true;
}

const flags = ts.NodeBuilderFlags.InTypeAlias | ts.NodeBuilderFlags.AllowEmptyTuple;
const typeNode = checker.typeToTypeNode(type, undefined, flags);
return typeNode && (ts.isArrayTypeNode(typeNode) || ts.isTupleTypeNode(typeNode));
}

public static isFunctionType(type: ts.Type, checker: ts.TypeChecker): boolean {
Expand All @@ -161,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 {
Expand All @@ -180,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);
}
Expand Down Expand Up @@ -330,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);
Expand Down Expand Up @@ -446,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
Expand All @@ -462,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) {
Expand Down Expand Up @@ -496,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();
Expand All @@ -509,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);
Expand All @@ -530,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)) {
Expand All @@ -550,7 +557,8 @@ export class TSHelper {

public static getSignatureDeclarations(
signatures: ReadonlyArray<ts.Signature>,
checker: ts.TypeChecker
checker: ts.TypeChecker,
program: ts.Program
): ts.SignatureDeclaration[]
{
const signatureDeclarations: ts.SignatureDeclaration[] = [];
Expand All @@ -560,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) {
Expand Down Expand Up @@ -650,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)));
}
Expand Down
19 changes: 19 additions & 0 deletions test/unit/array.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> = [3,5,1];
return arr[1];`
);
Expect(result).toBe(5);
}

@Test("Array union access")
public arrayUnionAccess(): void {
const result = util.transpileAndExecute(
Expand All @@ -21,6 +30,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(
Expand Down