From d2d7fc7f2cba2b16ac9d706a01dca31ed415b182 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 14 Feb 2019 05:35:26 -0700 Subject: [PATCH 1/4] new implementation of get/set accessors --- src/LuaLib.ts | 2 + src/LuaTransformer.ts | 131 ++++++++++++++--------- src/TSHelper.ts | 71 +++--------- src/lualib/Index.ts | 38 +++++++ src/lualib/NewIndex.ts | 37 +++++++ test/translation/lua/getSetAccessors.lua | 16 +-- test/unit/expressions.spec.ts | 16 +-- 7 files changed, 183 insertions(+), 128 deletions(-) create mode 100644 src/lualib/Index.ts create mode 100644 src/lualib/NewIndex.ts diff --git a/src/LuaLib.ts b/src/LuaLib.ts index b39b3e4c4..a93b252c9 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -19,9 +19,11 @@ export enum LuaLibFeature { FunctionApply = "FunctionApply", FunctionBind = "FunctionBind", FunctionCall = "FunctionCall", + Index = "Index", InstanceOf = "InstanceOf", Iterator = "Iterator", Map = "Map", + NewIndex = "NewIndex", Set = "Set", StringReplace = "StringReplace", StringSplit = "StringSplit", diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 91c63a775..72a6180c0 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -516,13 +516,61 @@ export class LuaTransformer { const assignClassPrototype = tstl.createAssignmentStatement(createClassPrototype(), classPrototypeTable); result.push(assignClassPrototype); - // className.prototype.__index = className.prototype const classPrototypeIndex = tstl.createTableIndexExpression( createClassPrototype(), tstl.createStringLiteral("__index") ); - const assignClassPrototypeIndex = tstl.createAssignmentStatement(classPrototypeIndex, createClassPrototype()); - result.push(assignClassPrototypeIndex); + if (statement.members.some(ts.isGetAccessor)) { + // className.prototype.____getters = {} + const classPrototypeGetters = tstl.createTableIndexExpression( + createClassPrototype(), + tstl.createStringLiteral("____getters") + ); + const assignClassPrototypeGetters = tstl.createAssignmentStatement( + classPrototypeGetters, + tstl.createTableExpression() + ); + result.push(assignClassPrototypeGetters); + + // className.prototype.__index = __TS_Index(className.prototype) + const assignClassPrototypeIndex = tstl.createAssignmentStatement( + classPrototypeIndex, + this.transformLuaLibFunction(LuaLibFeature.Index, createClassPrototype()) + ); + result.push(assignClassPrototypeIndex); + + } else { + // className.prototype.__index = className.prototype + const assignClassPrototypeIndex = tstl.createAssignmentStatement( + classPrototypeIndex, + createClassPrototype() + ); + result.push(assignClassPrototypeIndex); + } + + if (tsHelper.hasSetAccessorInClassOrAncestor(statement, this.checker)) { + // className.prototype.____setters = {} + const classPrototypeSetters = tstl.createTableIndexExpression( + createClassPrototype(), + tstl.createStringLiteral("____setters") + ); + const assignClassPrototypeSetters = tstl.createAssignmentStatement( + classPrototypeSetters, + tstl.createTableExpression() + ); + result.push(assignClassPrototypeSetters); + + // className.prototype.__newindex = __TS_NewIndex(className.prototype) + const classPrototypeNewIndex = tstl.createTableIndexExpression( + createClassPrototype(), + tstl.createStringLiteral("__newindex") + ); + const assignClassPrototypeIndex = tstl.createAssignmentStatement( + classPrototypeNewIndex, + this.transformLuaLibFunction(LuaLibFeature.NewIndex, createClassPrototype()) + ); + result.push(assignClassPrototypeIndex); + } // className.prototype.constructor = className const classPrototypeConstructor = tstl.createTableIndexExpression( @@ -727,15 +775,20 @@ export class LuaTransformer { [this.createSelfIdentifier()] ); - return tstl.createAssignmentStatement( - tstl.createTableIndexExpression( - tstl.createTableIndexExpression( - this.addExportToIdentifier(tstl.cloneIdentifier(className)), - tstl.createStringLiteral("prototype") - ), - tstl.createStringLiteral("get__" + name.text)), - accessorFunction + const classPrototype = tstl.createTableIndexExpression( + this.addExportToIdentifier(tstl.cloneIdentifier(className)), + tstl.createStringLiteral("prototype") + ); + const classGetters = tstl.createTableIndexExpression( + classPrototype, + tstl.createStringLiteral("____getters") ); + const getter = tstl.createTableIndexExpression( + classGetters, + tstl.createStringLiteral(name.text) + ); + const assignGetter = tstl.createAssignmentStatement(getter, accessorFunction); + return assignGetter; } public transformSetAccessorDeclaration( @@ -756,15 +809,20 @@ export class LuaTransformer { restParam ); - return tstl.createAssignmentStatement( - tstl.createTableIndexExpression( - tstl.createTableIndexExpression( - this.addExportToIdentifier(tstl.cloneIdentifier(className)), - tstl.createStringLiteral("prototype") - ), - tstl.createStringLiteral("set__" + name.text)), - accessorFunction + const classPrototype = tstl.createTableIndexExpression( + this.addExportToIdentifier(tstl.cloneIdentifier(className)), + tstl.createStringLiteral("prototype") + ); + const classSetters = tstl.createTableIndexExpression( + classPrototype, + tstl.createStringLiteral("____setters") ); + const setter = tstl.createTableIndexExpression( + classSetters, + tstl.createStringLiteral(name.text) + ); + const assignSetter = tstl.createAssignmentStatement(setter, accessorFunction); + return assignSetter; } public transformMethodDeclaration( @@ -2137,16 +2195,6 @@ export class LuaTransformer { } public transformAssignment(lhs: ts.Expression, right: tstl.Expression): tstl.Statement { - if (ts.isPropertyAccessExpression(lhs)) { - const hasSetAccessor = tsHelper.hasSetAccessor(lhs, this.checker); - if (hasSetAccessor) { - return tstl.createExpressionStatement(this.transformSetAccessor(lhs, right), lhs.parent); - } else if (hasSetAccessor === undefined) { - // Undefined hasSetAccessor indicates a union with both set accessors and no accessors - // on the same field. - throw TSTLErrors.UnsupportedUnionAccessor(lhs); - } - } return tstl.createAssignmentStatement( this.transformExpression(lhs) as tstl.IdentifierOrTableIndexExpression, right, @@ -2214,10 +2262,7 @@ export class LuaTransformer { ); } - if ( - (ts.isPropertyAccessExpression(expression.left) && !tsHelper.hasSetAccessor(expression.left, this.checker)) - || ts.isElementAccessExpression(expression.left) - ) { + if (ts.isPropertyAccessExpression(expression.left) || ts.isElementAccessExpression(expression.left)) { // Left is property/element access: cache result while maintaining order of evaluation // (function(o, i, v) o[i] = v; return v end)(${objExpression}, ${indexExpression}, ${right}) const objParameter = tstl.createIdentifier("o"); @@ -2980,14 +3025,6 @@ export class LuaTransformer { public transformPropertyAccessExpression(node: ts.PropertyAccessExpression): tstl.Expression { const property = node.name.text; - const hasGetAccessor = tsHelper.hasGetAccessor(node, this.checker); - if (hasGetAccessor) { - return this.transformGetAccessor(node); - } else if (hasGetAccessor === undefined) { - // Undefined hasGetAccessor indicates a union with both get accessors and no accessors on the same field. - throw TSTLErrors.UnsupportedUnionAccessor(node); - } - // Check for primitive types to override const type = this.checker.getTypeAtLocation(node.expression); switch (type.flags) { @@ -3032,18 +3069,6 @@ export class LuaTransformer { return tstl.createTableIndexExpression(callPath, tstl.createStringLiteral(property), node); } - public transformGetAccessor(node: ts.PropertyAccessExpression): tstl.MethodCallExpression { - const name = tstl.createIdentifier(`get__${node.name.escapedText}`); - const expression = this.transformExpression(node.expression); - return tstl.createMethodCallExpression(expression, name, [], node); - } - - public transformSetAccessor(node: ts.PropertyAccessExpression, value: tstl.Expression): tstl.MethodCallExpression { - const name = tstl.createIdentifier(`set__${node.name.escapedText}`); - const expression = this.transformExpression(node.expression); - return tstl.createMethodCallExpression(expression, name, [value], node); - } - // Transpile a Math._ property public transformMathExpression(identifier: ts.Identifier): tstl.TableIndexExpression { const translation = { diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 360838678..7a550f974 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -262,60 +262,6 @@ export class TSHelper { return undefined; } - public static hasExplicitGetAccessor(type: ts.Type, name: ts.__String): boolean { - if (type && type.symbol && type.symbol.members) { - const field = type.symbol.members.get(name); - return field && (field.flags & ts.SymbolFlags.GetAccessor) !== 0; - } - } - - public static typeHasGetAccessor(type: ts.Type, name: ts.__String, checker: ts.TypeChecker): boolean | undefined { - if (type.isUnion()) { - if (type.types.some(t => TSHelper.typeHasGetAccessor(t, name, checker))) { - // undefined if only a subset of types implements the accessor - return type.types.every(t => TSHelper.typeHasGetAccessor(t, name, checker)) ? true : undefined; - } - return false; - } - return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.hasExplicitGetAccessor(t, name)); - } - - public static hasGetAccessor(node: ts.Node, checker: ts.TypeChecker): boolean | undefined { - if (ts.isPropertyAccessExpression(node)) { - const name = node.name.escapedText; - const type = checker.getTypeAtLocation(node.expression); - return TSHelper.typeHasGetAccessor(type, name, checker); - } - return false; - } - - public static hasExplicitSetAccessor(type: ts.Type, name: ts.__String): boolean { - if (type && type.symbol && type.symbol.members) { - const field = type.symbol.members.get(name); - return field && (field.flags & ts.SymbolFlags.SetAccessor) !== 0; - } - } - - public static typeHasSetAccessor(type: ts.Type, name: ts.__String, checker: ts.TypeChecker): boolean | undefined { - if (type.isUnion()) { - if (type.types.some(t => TSHelper.typeHasSetAccessor(t, name, checker))) { - // undefined if only a subset of types implements the accessor - return type.types.every(t => TSHelper.typeHasSetAccessor(t, name, checker)) ? true : undefined; - } - return false; - } - return TSHelper.forTypeOrAnySupertype(type, checker, t => TSHelper.hasExplicitSetAccessor(t, name)); - } - - public static hasSetAccessor(node: ts.Node, checker: ts.TypeChecker): boolean { - if (ts.isPropertyAccessExpression(node)) { - const name = node.name.escapedText; - const type = checker.getTypeAtLocation(node.expression); - return TSHelper.typeHasSetAccessor(type, name, checker); - } - return false; - } - public static isBinaryAssignmentToken(token: ts.SyntaxKind): [boolean, tstl.BinaryOperator] { switch (token) { case ts.SyntaxKind.BarEqualsToken: @@ -399,6 +345,23 @@ export class TSHelper { param => ts.isIdentifier(param.name) && param.name.originalKeywordKind === ts.SyntaxKind.ThisKeyword); } + public static hasSetAccessorInClassOrAncestor( + classDeclaration: ts.ClassLikeDeclarationBase, + checker: ts.TypeChecker + ): boolean + { + if (classDeclaration.members.some(ts.isSetAccessor)) { + return true; + } + const extendsType = TSHelper.getExtendedType(classDeclaration, checker); + if (!extendsType) { + return false; + } + const symbol = extendsType.getSymbol(); + const declarations = symbol.getDeclarations(); + return declarations.filter(ts.isClassLike).some(d => TSHelper.hasSetAccessorInClassOrAncestor(d, checker)); + } + public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker): ts.Type { if (ts.isParenthesizedExpression(expression.parent)) { // Ignore expressions wrapped in parenthesis diff --git a/src/lualib/Index.ts b/src/lualib/Index.ts new file mode 100644 index 000000000..f66489741 --- /dev/null +++ b/src/lualib/Index.ts @@ -0,0 +1,38 @@ +interface LuaClass { + prototype: LuaObject; + ____super?: LuaClass; +} + +declare interface LuaObject { + constructor: LuaClass; + ____getters?: { [key: string]: (self: LuaObject) => any }; +} + +declare function rawget(obj: T, key: K): T[K]; + +function __TS__Index(classProto: LuaObject): (tbl: LuaObject, key: keyof LuaObject) => any { + return (tbl, key) => { + let proto = classProto; + while (true) { + const val = rawget(proto, key); + if (val !== null) { + return val; + } + + const getters = rawget(proto, "____getters"); + if (getters) { + const getter = getters[key]; + if (getter) { + return getter(tbl); + } + } + + const base = rawget(rawget(proto, "constructor"), "____super"); + if (!base) { + break; + } + + proto = rawget(base, "prototype"); + } + }; +} diff --git a/src/lualib/NewIndex.ts b/src/lualib/NewIndex.ts new file mode 100644 index 000000000..e9ecd215a --- /dev/null +++ b/src/lualib/NewIndex.ts @@ -0,0 +1,37 @@ +interface LuaClass { + prototype: LuaObject; + ____super?: LuaClass; +} + +declare interface LuaObject { + constructor: LuaClass; + ____setters?: { [key: string]: (self: LuaObject, val: any) => void }; +} + +declare function rawget(obj: T, key: K): T[K]; +declare function rawset(obj: T, key: K, val: T[K]): void; + +function __TS__NewIndex(classProto: LuaObject): (tbl: LuaObject, key: keyof LuaObject, val: any) => void { + return (tbl, key, val) => { + let proto = classProto; + while (true) { + const setters = rawget(proto, "____setters"); + if (setters) { + const setter = setters[key]; + if (setter) { + setter(tbl, val); + return; + } + } + + const base = rawget(rawget(proto, "constructor"), "____super"); + if (!base) { + break; + } + + proto = rawget(base, "prototype"); + } + + rawset(tbl, key, val); + }; +} diff --git a/test/translation/lua/getSetAccessors.lua b/test/translation/lua/getSetAccessors.lua index c1c16ff51..a4188e9c7 100644 --- a/test/translation/lua/getSetAccessors.lua +++ b/test/translation/lua/getSetAccessors.lua @@ -1,7 +1,11 @@ +require("lualib_bundle"); MyClass = MyClass or {}; MyClass.__index = MyClass; MyClass.prototype = MyClass.prototype or {}; -MyClass.prototype.__index = MyClass.prototype; +MyClass.prototype.____getters = {}; +MyClass.prototype.__index = __TS__Index(MyClass.prototype); +MyClass.prototype.____setters = {}; +MyClass.prototype.__newindex = __TS__NewIndex(MyClass.prototype); MyClass.prototype.constructor = MyClass; MyClass.new = function(...) local self = setmetatable({}, MyClass.prototype); @@ -10,13 +14,13 @@ MyClass.new = function(...) end; MyClass.prototype.____constructor = function(self) end; -MyClass.prototype.get__field = function(self) +MyClass.prototype.____getters.field = function(self) return self._field + 4; end; -MyClass.prototype.set__field = function(self, v) +MyClass.prototype.____setters.field = function(self, v) self._field = v * 2; end; local instance = MyClass.new(); -instance:set__field(4); -local b = instance:get__field(); -local c = (4 + instance:get__field()) * 3; +instance.field = 4; +local b = instance.field; +local c = (4 + instance.field) * 3; diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index e7705338a..950802339 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -249,7 +249,7 @@ export class ExpressionTests { @TestCase("inst.field | 3", 8 | 3) @TestCase("inst.field << 3", 8 << 3) @TestCase("inst.field >> 1", 8 >> 1) - @TestCase("inst.field = 3", 7) + @TestCase("inst.field = 3", 3) @TestCase(`"abc" + inst.field`, "abc8") @Test("Get accessor expression") public getAccessorBinary(expression: string, expected: any): void { @@ -342,20 +342,6 @@ export class ExpressionTests { Expect(result).toBe(expected); } - @TestCase("x.value = 3;") - @TestCase("return x.value;") - @Test("Unsupported Union accessors") - public unsupportedUnionAccessors(expression: string): void { - const source = `class A{ get value(){ return 1; } } - class B{ value:number = 3; } - let x: A|B = new A(); - ${expression}`; - Expect(() => { util.transpileString(source); }).toThrowError( - TranspileError, - "Unsupported mixed union of accessor and non-accessor types for the same property." - ); - } - @TestCase("i++", 10) @TestCase("i--", 10) @TestCase("++i", 11) From 89e4c41047d24cfc74acca2c3dd03dadd0e469cf Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 14 Feb 2019 09:00:29 -0700 Subject: [PATCH 2/4] added test and fixed discovered edge-cases --- src/LuaTransformer.ts | 27 ++++++- src/TSHelper.ts | 71 ++++++++++++++++-- test/unit/accessors.spec.ts | 146 ++++++++++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 11 deletions(-) create mode 100644 test/unit/accessors.spec.ts diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 72a6180c0..58b80b054 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -415,12 +415,14 @@ export class LuaTransformer { instanceFields, statement )); - } else if (instanceFields.length > 0) { + } else if (instanceFields.length > 0 + || statement.members.some(m => tsHelper.isGetAccessorOverride(m, statement, this.checker))) + { // Generate a constructor if none was defined in a class with instance fields that need initialization // className.prototype.____constructor = function(self, ...) // baseClassName.prototype.____constructor(self, ...) // ... - const constructorBody = this.transformClassInstanceFields(instanceFields); + const constructorBody = this.transformClassInstanceFields(statement, instanceFields); const superCall = tstl.createExpressionStatement( tstl.createCallExpression( tstl.createTableIndexExpression( @@ -664,7 +666,11 @@ export class LuaTransformer { return result; } - public transformClassInstanceFields(instanceFields: ts.PropertyDeclaration[]): tstl.Statement[] { + public transformClassInstanceFields( + classDeclarataion: ts.ClassLikeDeclaration, + instanceFields: ts.PropertyDeclaration[] + ): tstl.Statement[] + { const statements: tstl.Statement[] = []; for (const f of instanceFields) { @@ -682,6 +688,19 @@ export class LuaTransformer { statements.push(assignClassField); } + const getOverrides = classDeclarataion.members.filter( + m => tsHelper.isGetAccessorOverride(m, classDeclarataion, this.checker) + ); + for (const getter of getOverrides) { + const resetGetter = tstl.createExpressionStatement( + tstl.createCallExpression( + tstl.createIdentifier("rawset"), + [this.createSelfIdentifier(), this.transformPropertyName(getter.name), tstl.createNilLiteral()] + ) + ); + statements.push(resetGetter); + } + return statements; } @@ -707,7 +726,7 @@ export class LuaTransformer { return undefined; } - const bodyStatements: tstl.Statement[] = this.transformClassInstanceFields(instanceFields); + const bodyStatements: tstl.Statement[] = this.transformClassInstanceFields(classDeclaration, instanceFields); // Check for field declarations in constructor const constructorFieldsDeclarations = statement.parameters.filter(p => p.modifiers !== undefined); diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 7a550f974..8a221e529 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -345,21 +345,78 @@ export class TSHelper { param => ts.isIdentifier(param.name) && param.name.originalKeywordKind === ts.SyntaxKind.ThisKeyword); } - public static hasSetAccessorInClassOrAncestor( + public static findInClassOrAncestor( classDeclaration: ts.ClassLikeDeclarationBase, + callback: (classDeclaration: ts.ClassLikeDeclarationBase) => boolean, checker: ts.TypeChecker - ): boolean + ): ts.ClassLikeDeclarationBase { - if (classDeclaration.members.some(ts.isSetAccessor)) { - return true; + if (callback(classDeclaration)) { + return classDeclaration; } + const extendsType = TSHelper.getExtendedType(classDeclaration, checker); if (!extendsType) { - return false; + return undefined; } + const symbol = extendsType.getSymbol(); - const declarations = symbol.getDeclarations(); - return declarations.filter(ts.isClassLike).some(d => TSHelper.hasSetAccessorInClassOrAncestor(d, checker)); + const declaration = symbol.getDeclarations().find(ts.isClassLike); + if (!declaration) { + return undefined; + } + + return TSHelper.findInClassOrAncestor(declaration, callback, checker); + } + + public static hasSetAccessorInClassOrAncestor( + classDeclaration: ts.ClassLikeDeclarationBase, + checker: ts.TypeChecker + ): boolean + { + return TSHelper.findInClassOrAncestor( + classDeclaration, + c => c.members.some(ts.isSetAccessor), + checker + ) !== undefined; + } + + public static getPropertyName(propertyName: ts.PropertyName): string | number | undefined { + if (ts.isIdentifier(propertyName) || ts.isStringLiteral(propertyName)) { + return propertyName.text; + } else if (ts.isNumericLiteral(propertyName)) { + return Number(propertyName.text); + } else { + return undefined; // TODO: how to handle computed property names? + } + } + + public static isSamePropertyName(a: ts.PropertyName, b: ts.PropertyName): boolean { + const aName = TSHelper.getPropertyName(a); + const bName = TSHelper.getPropertyName(b); + return aName !== undefined && aName === bName; + } + + public static isGetAccessorOverride( + element: ts.ClassElement, + classDeclaration: ts.ClassLikeDeclarationBase, + checker: ts.TypeChecker + ): element is ts.GetAccessorDeclaration + { + if (!ts.isGetAccessor(element)) { + return false; + } + + const hasInitializedField = (e: ts.ClassElement) => + ts.isPropertyDeclaration(e) + && e.initializer + && TSHelper.isSamePropertyName(e.name, element.name); + + return TSHelper.findInClassOrAncestor( + classDeclaration, + c => c.members.some(hasInitializedField), + checker + ) !== undefined; } public static inferAssignedType(expression: ts.Expression, checker: ts.TypeChecker): ts.Type { diff --git a/test/unit/accessors.spec.ts b/test/unit/accessors.spec.ts new file mode 100644 index 000000000..3769526a8 --- /dev/null +++ b/test/unit/accessors.spec.ts @@ -0,0 +1,146 @@ +import { Expect, Test } from "alsatian"; +import * as util from "../src/util"; + +export class AccessorTests { + @Test("get accessor") + public getAccessor(): void { + const code = + `class Foo { + get foo() { return "foo"; } + } + const f = new Foo(); + return f.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } + + @Test("get accessor in base class") + public getAccessorInBaseClass(): void { + const code = + `class Foo { + get foo() { return "foo"; } + } + class Bar extends Foo {} + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } + + @Test("get accessor override") + public getAccessorOverride(): void { + const code = + `class Foo { + foo = "foo"; + } + class Bar extends Foo { + get foo() { return "bar"; } + } + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + + @Test("get accessor overridden") + public getAccessorOverridden(): void { + const code = + `class Foo { + get foo() { return "foo"; } + } + class Bar extends Foo { + foo = "bar"; + } + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + + @Test("get accessor from interface") + public getAccessorFromINterface(): void { + const code = + `class Foo { + get foo() { return "foo"; } + } + interface Bar { + readonly foo: string; + } + const b: Bar = new Foo(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } + + @Test("set accessor") + public setAccessor(): void { + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + const f = new Foo(); + f.foo = "bar" + return f.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + + @Test("set accessor in base class") + public setAccessorInBaseClass(): void { + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + class Bar extends Foo {} + const b = new Bar(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + + @Test("set accessor override") + public setAccessorOverride(): void { + const code = + `class Foo { + prop = "prop"; + foo = "foo"; + } + class Bar extends Foo { + set foo(val: string) { this.prop = val; } + } + const b = new Bar(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + + @Test("set accessor overridden") + public setAccessorOverridden(): void { + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + class Bar extends Foo { + foo = "foo"; // triggers base class setter + } + const b = new Bar(); + const propOriginal = b.prop; + b.foo = "bar" + return propOriginal + b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("foobar"); + } + + @Test("set accessor from interface") + public setAccessorFromInterface(): void { + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + interface Bar { + prop: string; + foo: string; + } + const b: Bar = new Foo(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } +} From 760d0871ee7e96b993ac6e916c9726bf1f0b07bd Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 14 Feb 2019 12:52:49 -0700 Subject: [PATCH 3/4] tabs bad; spaces good --- src/lualib/Index.ts | 50 ++++---- src/lualib/NewIndex.ts | 46 ++++---- test/unit/accessors.spec.ts | 224 ++++++++++++++++++------------------ 3 files changed, 160 insertions(+), 160 deletions(-) diff --git a/src/lualib/Index.ts b/src/lualib/Index.ts index f66489741..a5f44081f 100644 --- a/src/lualib/Index.ts +++ b/src/lualib/Index.ts @@ -1,38 +1,38 @@ interface LuaClass { - prototype: LuaObject; - ____super?: LuaClass; + prototype: LuaObject; + ____super?: LuaClass; } declare interface LuaObject { - constructor: LuaClass; - ____getters?: { [key: string]: (self: LuaObject) => any }; + constructor: LuaClass; + ____getters?: { [key: string]: (self: LuaObject) => any }; } declare function rawget(obj: T, key: K): T[K]; function __TS__Index(classProto: LuaObject): (tbl: LuaObject, key: keyof LuaObject) => any { - return (tbl, key) => { - let proto = classProto; - while (true) { - const val = rawget(proto, key); - if (val !== null) { - return val; - } + return (tbl, key) => { + let proto = classProto; + while (true) { + const val = rawget(proto, key); + if (val !== null) { + return val; + } - const getters = rawget(proto, "____getters"); - if (getters) { - const getter = getters[key]; - if (getter) { - return getter(tbl); - } - } + const getters = rawget(proto, "____getters"); + if (getters) { + const getter = getters[key]; + if (getter) { + return getter(tbl); + } + } - const base = rawget(rawget(proto, "constructor"), "____super"); - if (!base) { - break; - } + const base = rawget(rawget(proto, "constructor"), "____super"); + if (!base) { + break; + } - proto = rawget(base, "prototype"); - } - }; + proto = rawget(base, "prototype"); + } + }; } diff --git a/src/lualib/NewIndex.ts b/src/lualib/NewIndex.ts index e9ecd215a..d50d76ab7 100644 --- a/src/lualib/NewIndex.ts +++ b/src/lualib/NewIndex.ts @@ -1,37 +1,37 @@ interface LuaClass { - prototype: LuaObject; - ____super?: LuaClass; + prototype: LuaObject; + ____super?: LuaClass; } declare interface LuaObject { - constructor: LuaClass; - ____setters?: { [key: string]: (self: LuaObject, val: any) => void }; + constructor: LuaClass; + ____setters?: { [key: string]: (self: LuaObject, val: any) => void }; } declare function rawget(obj: T, key: K): T[K]; declare function rawset(obj: T, key: K, val: T[K]): void; function __TS__NewIndex(classProto: LuaObject): (tbl: LuaObject, key: keyof LuaObject, val: any) => void { - return (tbl, key, val) => { - let proto = classProto; - while (true) { - const setters = rawget(proto, "____setters"); - if (setters) { - const setter = setters[key]; - if (setter) { - setter(tbl, val); - return; - } - } + return (tbl, key, val) => { + let proto = classProto; + while (true) { + const setters = rawget(proto, "____setters"); + if (setters) { + const setter = setters[key]; + if (setter) { + setter(tbl, val); + return; + } + } - const base = rawget(rawget(proto, "constructor"), "____super"); - if (!base) { - break; - } + const base = rawget(rawget(proto, "constructor"), "____super"); + if (!base) { + break; + } - proto = rawget(base, "prototype"); - } + proto = rawget(base, "prototype"); + } - rawset(tbl, key, val); - }; + rawset(tbl, key, val); + }; } diff --git a/test/unit/accessors.spec.ts b/test/unit/accessors.spec.ts index 3769526a8..6ba58481c 100644 --- a/test/unit/accessors.spec.ts +++ b/test/unit/accessors.spec.ts @@ -4,143 +4,143 @@ import * as util from "../src/util"; export class AccessorTests { @Test("get accessor") public getAccessor(): void { - const code = - `class Foo { - get foo() { return "foo"; } - } - const f = new Foo(); - return f.foo;`; - Expect(util.transpileAndExecute(code)).toBe("foo"); - } + const code = + `class Foo { + get foo() { return "foo"; } + } + const f = new Foo(); + return f.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } @Test("get accessor in base class") public getAccessorInBaseClass(): void { - const code = - `class Foo { - get foo() { return "foo"; } - } - class Bar extends Foo {} - const b = new Bar(); - return b.foo;`; - Expect(util.transpileAndExecute(code)).toBe("foo"); - } + const code = + `class Foo { + get foo() { return "foo"; } + } + class Bar extends Foo {} + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } @Test("get accessor override") public getAccessorOverride(): void { - const code = - `class Foo { - foo = "foo"; - } - class Bar extends Foo { - get foo() { return "bar"; } - } - const b = new Bar(); - return b.foo;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + foo = "foo"; + } + class Bar extends Foo { + get foo() { return "bar"; } + } + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } @Test("get accessor overridden") public getAccessorOverridden(): void { - const code = - `class Foo { - get foo() { return "foo"; } - } - class Bar extends Foo { - foo = "bar"; - } - const b = new Bar(); - return b.foo;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + get foo() { return "foo"; } + } + class Bar extends Foo { + foo = "bar"; + } + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } @Test("get accessor from interface") public getAccessorFromINterface(): void { - const code = - `class Foo { - get foo() { return "foo"; } - } - interface Bar { - readonly foo: string; - } - const b: Bar = new Foo(); - return b.foo;`; - Expect(util.transpileAndExecute(code)).toBe("foo"); - } + const code = + `class Foo { + get foo() { return "foo"; } + } + interface Bar { + readonly foo: string; + } + const b: Bar = new Foo(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("foo"); + } @Test("set accessor") public setAccessor(): void { - const code = - `class Foo { - prop = "prop"; - set foo(val: string) { this.prop = val; } - } - const f = new Foo(); - f.foo = "bar" - return f.prop;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + const f = new Foo(); + f.foo = "bar" + return f.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } @Test("set accessor in base class") public setAccessorInBaseClass(): void { - const code = - `class Foo { - prop = "prop"; - set foo(val: string) { this.prop = val; } - } - class Bar extends Foo {} - const b = new Bar(); - b.foo = "bar" - return b.prop;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + class Bar extends Foo {} + const b = new Bar(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } @Test("set accessor override") public setAccessorOverride(): void { - const code = - `class Foo { - prop = "prop"; - foo = "foo"; - } - class Bar extends Foo { - set foo(val: string) { this.prop = val; } - } - const b = new Bar(); - b.foo = "bar" - return b.prop;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + prop = "prop"; + foo = "foo"; + } + class Bar extends Foo { + set foo(val: string) { this.prop = val; } + } + const b = new Bar(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } @Test("set accessor overridden") public setAccessorOverridden(): void { - const code = - `class Foo { - prop = "prop"; - set foo(val: string) { this.prop = val; } - } - class Bar extends Foo { - foo = "foo"; // triggers base class setter - } - const b = new Bar(); - const propOriginal = b.prop; - b.foo = "bar" - return propOriginal + b.prop;`; - Expect(util.transpileAndExecute(code)).toBe("foobar"); - } + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + class Bar extends Foo { + foo = "foo"; // triggers base class setter + } + const b = new Bar(); + const propOriginal = b.prop; + b.foo = "bar" + return propOriginal + b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("foobar"); + } @Test("set accessor from interface") public setAccessorFromInterface(): void { - const code = - `class Foo { - prop = "prop"; - set foo(val: string) { this.prop = val; } - } - interface Bar { - prop: string; - foo: string; - } - const b: Bar = new Foo(); - b.foo = "bar" - return b.prop;`; - Expect(util.transpileAndExecute(code)).toBe("bar"); - } + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = val; } + } + interface Bar { + prop: string; + foo: string; + } + const b: Bar = new Foo(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } } From 4228f9acb39d2a60f5179b15665b833daace3ec8 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 15 Feb 2019 16:22:14 -0700 Subject: [PATCH 4/4] addressing feedback - removed special case for numerical literals when comparing property names - added tests for accessors overriding other accessors --- src/LuaTransformer.ts | 4 ++-- src/TSHelper.ts | 4 +--- test/unit/accessors.spec.ts | 30 ++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 052730cd7..e5701f844 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -541,7 +541,7 @@ export class LuaTransformer { // className.prototype.__index = __TS_Index(className.prototype) const assignClassPrototypeIndex = tstl.createAssignmentStatement( classPrototypeIndex, - this.transformLuaLibFunction(LuaLibFeature.Index, createClassPrototype()) + this.transformLuaLibFunction(LuaLibFeature.Index, undefined, createClassPrototype()) ); result.push(assignClassPrototypeIndex); @@ -573,7 +573,7 @@ export class LuaTransformer { ); const assignClassPrototypeIndex = tstl.createAssignmentStatement( classPrototypeNewIndex, - this.transformLuaLibFunction(LuaLibFeature.NewIndex, createClassPrototype()) + this.transformLuaLibFunction(LuaLibFeature.NewIndex, undefined, createClassPrototype()) ); result.push(assignClassPrototypeIndex); } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 894af7a0e..b33324682 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -381,10 +381,8 @@ export class TSHelper { } public static getPropertyName(propertyName: ts.PropertyName): string | number | undefined { - if (ts.isIdentifier(propertyName) || ts.isStringLiteral(propertyName)) { + if (ts.isIdentifier(propertyName) || ts.isStringLiteral(propertyName) || ts.isNumericLiteral(propertyName)) { return propertyName.text; - } else if (ts.isNumericLiteral(propertyName)) { - return Number(propertyName.text); } else { return undefined; // TODO: how to handle computed property names? } diff --git a/test/unit/accessors.spec.ts b/test/unit/accessors.spec.ts index 6ba58481c..7c3a255b4 100644 --- a/test/unit/accessors.spec.ts +++ b/test/unit/accessors.spec.ts @@ -53,6 +53,20 @@ export class AccessorTests { Expect(util.transpileAndExecute(code)).toBe("bar"); } + @Test("get accessor override accessor") + public getAccessorOverrideAccessor(): void { + const code = + `class Foo { + get foo() { return "foo"; } + } + class Bar extends Foo { + get foo() { return "bar"; } + } + const b = new Bar(); + return b.foo;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + @Test("get accessor from interface") public getAccessorFromINterface(): void { const code = @@ -127,6 +141,22 @@ export class AccessorTests { Expect(util.transpileAndExecute(code)).toBe("foobar"); } + @Test("set accessor override accessor") + public setAccessorOverrideAccessor(): void { + const code = + `class Foo { + prop = "prop"; + set foo(val: string) { this.prop = "foo"; } + } + class Bar extends Foo { + set foo(val: string) { this.prop = val; } + } + const b = new Bar(); + b.foo = "bar" + return b.prop;`; + Expect(util.transpileAndExecute(code)).toBe("bar"); + } + @Test("set accessor from interface") public setAccessorFromInterface(): void { const code =