From c7c3ac1ff61f23b5e261f543108b29e99796aff8 Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Fri, 18 Jan 2019 13:49:26 +0100 Subject: [PATCH 1/6] check unions for accessors --- src/Errors.ts | 3 +++ src/TSHelper.ts | 15 +++++++++++++-- src/Transpiler.ts | 5 ++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/Errors.ts b/src/Errors.ts index 556bf4775..d89f41a77 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -116,4 +116,7 @@ export class TSTLErrors { + "the TupleReturn decorator.", node); } + + public static UnsupporteUnionAccessor = (node: ts.Node) => + new TranspileError(`Unsupported union of accessor with non-accessor types.`, node) } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 1259ab423..333edfd78 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -245,11 +245,22 @@ export class TSHelper { } } - public static hasGetAccessor(node: ts.Node, checker: ts.TypeChecker): boolean { + public static typeHasGetAccessor(type: ts.Type, name: ts.__String, checker: ts.TypeChecker): boolean | undefined { + if (type.isUnion()) { + if (type.types.some(t => this.typeHasGetAccessor(t, name, checker))) { + // undefined if only a subset of types implements the accessor + return type.types.every(t => this.typeHasGetAccessor(t, name, checker)) ? true : undefined; + } + return false; + } + return this.forTypeOrAnySupertype(type, checker, t => this.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 this.forTypeOrAnySupertype(type, checker, t => this.hasExplicitGetAccessor(t, name)); + return this.typeHasGetAccessor(type, name, checker); } return false; } diff --git a/src/Transpiler.ts b/src/Transpiler.ts index 39289c37e..d35c6b16b 100644 --- a/src/Transpiler.ts +++ b/src/Transpiler.ts @@ -1611,7 +1611,10 @@ export abstract class LuaTranspiler { public transpilePropertyAccessExpression(node: ts.PropertyAccessExpression): string { const property = node.name.text; - if (tsHelper.hasGetAccessor(node, this.checker)) { + const hasGetAccessor = tsHelper.hasGetAccessor(node, this.checker); + if (hasGetAccessor === undefined) { + throw TSTLErrors.UnsupporteUnionAccessor(node); + } else if (hasGetAccessor) { return this.transpileGetAccessor(node); } From 3d5fefe9c0a83800aaa60a07721d4afc99e5e95b Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Fri, 18 Jan 2019 14:00:40 +0100 Subject: [PATCH 2/6] add test case --- test/unit/expressions.spec.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index 2e86beac7..59a880117 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -317,6 +317,33 @@ export class ExpressionTests { Expect(result).toBe(expected); } + @TestCase("x.value", 1) + @Test("Union accessors") + public unionAccessors(expression: string, expected: any): void { + const source = `class A{ get value(){ return 1; } } + class B{ get value(){ return 2; } } + class C{ value:number = 3; } + let x: A|B = new A(); + return ${expression};`; + + const lua = util.transpileString(source); + const result = util.executeLua(lua); + Expect(result).toBe(expected); + } + + @TestCase("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(); + return ${expression};`; + Expect(() => { util.transpileString(source); }).toThrowError( + TranspileError, + "Unsupported union of accessor with non-accessor types." + ); + } + @TestCase("i++", 10) @TestCase("i--", 10) @TestCase("++i", 11) From 98b84a1ad798f211680918287d17f7f8bfef9f55 Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Fri, 18 Jan 2019 14:05:29 +0100 Subject: [PATCH 3/6] update test --- test/unit/expressions.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index 59a880117..9a91c606d 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -322,7 +322,6 @@ export class ExpressionTests { public unionAccessors(expression: string, expected: any): void { const source = `class A{ get value(){ return 1; } } class B{ get value(){ return 2; } } - class C{ value:number = 3; } let x: A|B = new A(); return ${expression};`; From 4ec1e655db572773d5857078708444404d43ec35 Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Fri, 18 Jan 2019 14:40:03 +0100 Subject: [PATCH 4/6] handle setters --- src/Errors.ts | 2 +- src/TSHelper.ts | 13 ++++++++++++- src/Transpiler.ts | 11 +++++++---- test/unit/expressions.spec.ts | 14 ++++++++------ 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/Errors.ts b/src/Errors.ts index d89f41a77..116a24496 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -117,6 +117,6 @@ export class TSTLErrors { node); } - public static UnsupporteUnionAccessor = (node: ts.Node) => + public static UnsupportedUnionAccessor = (node: ts.Node) => new TranspileError(`Unsupported union of accessor with non-accessor types.`, node) } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 333edfd78..0cc296de9 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -272,11 +272,22 @@ export class TSHelper { } } + public static typeHasSetAccessor(type: ts.Type, name: ts.__String, checker: ts.TypeChecker): boolean | undefined { + if (type.isUnion()) { + if (type.types.some(t => this.typeHasSetAccessor(t, name, checker))) { + // undefined if only a subset of types implements the accessor + return type.types.every(t => this.typeHasSetAccessor(t, name, checker)) ? true : undefined; + } + return false; + } + return this.forTypeOrAnySupertype(type, checker, t => this.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 this.forTypeOrAnySupertype(type, checker, t => this.hasExplicitSetAccessor(t, name)); + return this.typeHasSetAccessor(type, name, checker); } return false; } diff --git a/src/Transpiler.ts b/src/Transpiler.ts index d35c6b16b..9ea324cad 100644 --- a/src/Transpiler.ts +++ b/src/Transpiler.ts @@ -965,8 +965,11 @@ export abstract class LuaTranspiler { } public transpileAssignment(node: ts.BinaryExpression, lhs: string, rhs: string): string { - if (tsHelper.hasSetAccessor(node.left, this.checker)) { + const hasSetAccessor = tsHelper.hasSetAccessor(node.left, this.checker); + if (hasSetAccessor) { return this.transpileSetAccessor(node.left as ts.PropertyAccessExpression, rhs); + } else if (hasSetAccessor === undefined) { + throw TSTLErrors.UnsupportedUnionAccessor(node); } // Validate assignment @@ -1612,10 +1615,10 @@ export abstract class LuaTranspiler { const property = node.name.text; const hasGetAccessor = tsHelper.hasGetAccessor(node, this.checker); - if (hasGetAccessor === undefined) { - throw TSTLErrors.UnsupporteUnionAccessor(node); - } else if (hasGetAccessor) { + if (hasGetAccessor) { return this.transpileGetAccessor(node); + } else if (hasGetAccessor === undefined) { + throw TSTLErrors.UnsupportedUnionAccessor(node); } // Check for primitive types to override diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index 9a91c606d..699778fd1 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -317,26 +317,28 @@ export class ExpressionTests { Expect(result).toBe(expected); } - @TestCase("x.value", 1) + @TestCase("return x.value;", 1) + @TestCase("x.value = 3; return x.value;", 3) @Test("Union accessors") public unionAccessors(expression: string, expected: any): void { - const source = `class A{ get value(){ return 1; } } - class B{ get value(){ return 2; } } + const source = `class A{ get value(){ return this.v || 1; } set value(v){ this.v = v; } v: number = 1; } + class B{ get value(){ return this.v || 2; } set value(v){ this.v = v; } v: number = 2; } let x: A|B = new A(); - return ${expression};`; + ${expression}`; const lua = util.transpileString(source); const result = util.executeLua(lua); Expect(result).toBe(expected); } - @TestCase("x.value") + @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(); - return ${expression};`; + ${expression}`; Expect(() => { util.transpileString(source); }).toThrowError( TranspileError, "Unsupported union of accessor with non-accessor types." From 86b313c3f8c44e819e46e5e48b58b29ca1adabdf Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Fri, 18 Jan 2019 14:40:03 +0100 Subject: [PATCH 5/6] update tests --- test/unit/expressions.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index 699778fd1..c77722d37 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -321,8 +321,8 @@ export class ExpressionTests { @TestCase("x.value = 3; return x.value;", 3) @Test("Union accessors") public unionAccessors(expression: string, expected: any): void { - const source = `class A{ get value(){ return this.v || 1; } set value(v){ this.v = v; } v: number = 1; } - class B{ get value(){ return this.v || 2; } set value(v){ this.v = v; } v: number = 2; } + const source = `class A{ get value(){ return this.v || 1; } set value(v){ this.v = v; } v: number; } + class B{ get value(){ return this.v || 2; } set value(v){ this.v = v; } v: number; } let x: A|B = new A(); ${expression}`; From f0556bfbe80fcdfe311a9015edd43e8d0a6483d7 Mon Sep 17 00:00:00 2001 From: Lars Melchior Date: Sat, 19 Jan 2019 19:29:21 +0100 Subject: [PATCH 6/6] consistent indentation --- src/Errors.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Errors.ts b/src/Errors.ts index 116a24496..b804c3905 100644 --- a/src/Errors.ts +++ b/src/Errors.ts @@ -117,6 +117,8 @@ export class TSTLErrors { node); } - public static UnsupportedUnionAccessor = (node: ts.Node) => - new TranspileError(`Unsupported union of accessor with non-accessor types.`, node) + public static UnsupportedUnionAccessor = (node: ts.Node) => { + return new TranspileError(`Unsupported union of accessor with non-accessor types.`, + node); + } }