Skip to content

typeof improvements #625

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
Jun 6, 2019
Merged

typeof improvements #625

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
f744a37
typeof now uses lib function, with optimized path for comparisons
tomblind Jun 6, 2019
89bc7bb
simplified tests
tomblind Jun 6, 2019
613fb05
refactored transformBinaryExpression a bit
tomblind Jun 6, 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
1 change: 1 addition & 0 deletions src/LuaLib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export enum LuaLibFeature {
StringStartsWith = "StringStartsWith",
Symbol = "Symbol",
SymbolRegistry = "SymbolRegistry",
TypeOf = "TypeOf",
}

const luaLibDependencies: { [lib in LuaLibFeature]?: LuaLibFeature[] } = {
Expand Down
80 changes: 57 additions & 23 deletions src/LuaTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2655,10 +2655,42 @@ export class LuaTransformer {
}
}

protected transformTypeOfLiteralComparison(
typeOfExpression: ts.TypeOfExpression,
comparedExpression: tstl.StringLiteral,
operator: ts.BinaryOperator,
tsOriginal: ts.Node
): ExpressionVisitResult {
if (comparedExpression.value === "object") {
comparedExpression.value = "table";
} else if (comparedExpression.value === "undefined") {
comparedExpression.value = "nil";
}
const innerExpression = this.transformExpression(typeOfExpression.expression);
const typeCall = tstl.createCallExpression(tstl.createIdentifier("type"), [innerExpression], typeOfExpression);
return this.transformBinaryOperation(typeCall, comparedExpression, operator, tsOriginal);
}

protected transformComparisonExpression(expression: ts.BinaryExpression): ExpressionVisitResult {
Comment thread
Perryvw marked this conversation as resolved.
const left = this.transformExpression(expression.left);
const right = this.transformExpression(expression.right);
const operator = expression.operatorToken.kind;

// Custom handling for 'typeof(foo) === "type"'
if (ts.isTypeOfExpression(expression.left) && tstl.isStringLiteral(right)) {
return this.transformTypeOfLiteralComparison(expression.left, right, operator, expression);
} else if (ts.isTypeOfExpression(expression.right) && tstl.isStringLiteral(left)) {
return this.transformTypeOfLiteralComparison(expression.right, left, operator, expression);
}

return this.transformBinaryOperation(left, right, operator, expression);
}

public transformBinaryExpression(expression: ts.BinaryExpression): ExpressionVisitResult {
// Check if this is an assignment token, then handle accordingly
const operator = expression.operatorToken.kind;

const [isCompound, replacementOperator] = tsHelper.isBinaryAssignmentToken(expression.operatorToken.kind);
// Check if this is an assignment token, then handle accordingly
const [isCompound, replacementOperator] = tsHelper.isBinaryAssignmentToken(operator);
if (isCompound && replacementOperator) {
return this.transformCompoundAssignmentExpression(
expression,
Expand All @@ -2669,26 +2701,27 @@ export class LuaTransformer {
);
}

const lhs = this.transformExpression(expression.left);
const rhs = this.transformExpression(expression.right);

// Transpile operators
switch (expression.operatorToken.kind) {
switch (operator) {
case ts.SyntaxKind.AmpersandToken:
case ts.SyntaxKind.BarToken:
case ts.SyntaxKind.CaretToken:
case ts.SyntaxKind.LessThanLessThanToken:
case ts.SyntaxKind.GreaterThanGreaterThanToken:
case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
return this.transformBinaryBitOperation(expression, lhs, rhs, expression.operatorToken.kind);
case ts.SyntaxKind.PlusToken:
case ts.SyntaxKind.AmpersandAmpersandToken:
case ts.SyntaxKind.BarBarToken:
case ts.SyntaxKind.MinusToken:
case ts.SyntaxKind.AsteriskToken:
case ts.SyntaxKind.AsteriskAsteriskToken:
case ts.SyntaxKind.SlashToken:
case ts.SyntaxKind.PercentToken:
case ts.SyntaxKind.PercentToken: {
const lhs = this.transformExpression(expression.left);

@Perryvw Perryvw Jun 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we calculate these in here at all, instead of inside transformBinaryOperation? Seems like a historic thing that's probably not relevant anymore?

@tomblind tomblind Jun 6, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transformBinaryOperation expects already transformed expressions, because it is sometimes given manually constructed ones (like from compound assignments)

const rhs = this.transformExpression(expression.right);
return this.transformBinaryOperation(lhs, rhs, operator, expression);
}

case ts.SyntaxKind.GreaterThanToken:
case ts.SyntaxKind.GreaterThanEqualsToken:
case ts.SyntaxKind.LessThanToken:
Expand All @@ -2697,19 +2730,26 @@ export class LuaTransformer {
case ts.SyntaxKind.EqualsEqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsToken:
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
return this.transformBinaryOperation(lhs, rhs, expression.operatorToken.kind, expression);
return this.transformComparisonExpression(expression);

case ts.SyntaxKind.EqualsToken:
return this.transformAssignmentExpression(expression);
case ts.SyntaxKind.InKeyword:

case ts.SyntaxKind.InKeyword: {
const lhs = this.transformExpression(expression.left);
const rhs = this.transformExpression(expression.right);
const indexExpression = tstl.createTableIndexExpression(rhs, lhs);
return tstl.createBinaryExpression(
indexExpression,
tstl.createNilLiteral(),
tstl.SyntaxKind.InequalityOperator,
expression
);
}

case ts.SyntaxKind.InstanceOfKeyword:
case ts.SyntaxKind.InstanceOfKeyword: {
const lhs = this.transformExpression(expression.left);
const rhs = this.transformExpression(expression.right);
const rhsType = this.checker.getTypeAtLocation(expression.right);
const decorators = tsHelper.getCustomDecorators(rhsType, this.checker);

Expand All @@ -2727,16 +2767,19 @@ export class LuaTransformer {
}

return this.transformLuaLibFunction(LuaLibFeature.InstanceOf, expression, lhs, rhs);
}

case ts.SyntaxKind.CommaToken:
case ts.SyntaxKind.CommaToken: {
const rhs = this.transformExpression(expression.right);
return this.createImmediatelyInvokedFunctionExpression(
this.statementVisitResultToArray(this.transformExpressionStatement(expression.left)),
rhs,
expression
);
}

default:
throw TSTLErrors.UnsupportedKind("binary operator", expression.operatorToken.kind, expression);
throw TSTLErrors.UnsupportedKind("binary operator", operator, expression);
}
}

Expand Down Expand Up @@ -4489,16 +4532,7 @@ export class LuaTransformer {

public transformTypeOfExpression(expression: ts.TypeOfExpression): ExpressionVisitResult {
const innerExpression = this.transformExpression(expression.expression);
const typeFunctionIdentifier = tstl.createIdentifier("type");
const typeCall = tstl.createCallExpression(typeFunctionIdentifier, [innerExpression]);
const tableString = tstl.createStringLiteral("table");
const objectString = tstl.createStringLiteral("object");
const condition = tstl.createBinaryExpression(typeCall, tableString, tstl.SyntaxKind.EqualityOperator);
const andClause = tstl.createBinaryExpression(condition, objectString, tstl.SyntaxKind.AndOperator);

return tstl.createParenthesizedExpression(
tstl.createBinaryExpression(andClause, tstl.cloneNode(typeCall), tstl.SyntaxKind.OrOperator, expression)
);
return this.transformLuaLibFunction(LuaLibFeature.TypeOf, expression, innerExpression);
}

public transformSpreadElement(expression: ts.SpreadElement): ExpressionVisitResult {
Expand Down
12 changes: 12 additions & 0 deletions src/lualib/TypeOf.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
declare function type(this: void, value: unknown): string;

function __TS__TypeOf(this: void, value: unknown): string {
const luaType = type(value);
if (luaType === "table") {
return "object";
} else if (luaType === "nil") {
return "undefined";
} else {
return luaType;
}
}
43 changes: 42 additions & 1 deletion test/unit/typechecking.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test("typeof function", () => {
test.each(["null", "undefined"])("typeof undefined (%p)", inp => {
const result = util.transpileAndExecute(`return typeof ${inp};`);

expect(result).toBe("nil");
expect(result).toBe("undefined");
});

test("instanceof", () => {
Expand Down Expand Up @@ -140,3 +140,44 @@ test("instanceof Symbol.hasInstance", () => {

expect(result).toBe(true);
});

test.each([
{ expression: "{}", operator: "===", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "!==", compareTo: "object", expectResult: false },
{ expression: "{}", operator: "==", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "!=", compareTo: "object", expectResult: false },
{ expression: "{}", operator: "<=", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "<", compareTo: "object", expectResult: false },
{ expression: "undefined", operator: "===", compareTo: "undefined", expectResult: true },
{ expression: "() => {}", operator: "===", compareTo: "function", expectResult: true },
{ expression: "1", operator: "===", compareTo: "number", expectResult: true },
{ expression: "true", operator: "===", compareTo: "boolean", expectResult: true },
{ expression: `"foo"`, operator: "===", compareTo: "string", expectResult: true },
])("typeof literal comparison (%p)", ({ expression, operator, compareTo, expectResult }) => {
const code = `
let val = ${expression};
return typeof val ${operator} "${compareTo}";`;

expect(util.transpileAndExecute(code)).toBe(expectResult);
});

test.each([
{ expression: "{}", operator: "===", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "!==", compareTo: "object", expectResult: false },
{ expression: "{}", operator: "==", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "!=", compareTo: "object", expectResult: false },
{ expression: "{}", operator: "<=", compareTo: "object", expectResult: true },
{ expression: "{}", operator: "<", compareTo: "object", expectResult: false },
{ expression: "undefined", operator: "===", compareTo: "undefined", expectResult: true },
{ expression: "() => {}", operator: "===", compareTo: "function", expectResult: true },
{ expression: "1", operator: "===", compareTo: "number", expectResult: true },
{ expression: "true", operator: "===", compareTo: "boolean", expectResult: true },
{ expression: `"foo"`, operator: "===", compareTo: "string", expectResult: true },
])("typeof non-literal comparison (%p)", ({ expression, operator, compareTo, expectResult }) => {
const code = `
let val = ${expression};
let compareTo = "${compareTo}";
return typeof val ${operator} compareTo;`;

expect(util.transpileAndExecute(code)).toBe(expectResult);
});