From b8e76ad7f5cd712a6c6cba5e4783b0f85672b9ab Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sun, 28 May 2023 22:33:43 +0200 Subject: [PATCH 1/7] Tests for new decorators --- .../__snapshots__/decorators.spec.ts.snap | 21 +- test/unit/classes/decorators.spec.ts | 436 +++++++++++++++--- test/util.ts | 1 - 3 files changed, 385 insertions(+), 73 deletions(-) diff --git a/test/unit/classes/__snapshots__/decorators.spec.ts.snap b/test/unit/classes/__snapshots__/decorators.spec.ts.snap index e2f95d920..1a8407ccd 100644 --- a/test/unit/classes/__snapshots__/decorators.spec.ts.snap +++ b/test/unit/classes/__snapshots__/decorators.spec.ts.snap @@ -6,7 +6,7 @@ local __TS__Class = ____lualib.__TS__Class local __TS__Decorate = ____lualib.__TS__Decorate local ____exports = {} function ____exports.__main(self) - local function decorator(constructor) + local function decorator(constructor, context) end local TestClass = __TS__Class() TestClass.name = "TestClass" @@ -18,3 +18,22 @@ return ____exports" `; exports[`Throws error if decorator function has void context: diagnostics 1`] = `"main.ts(4,9): error TSTL: Decorator function cannot have 'this: void'."`; + +exports[`legacy experimentalDecorators Throws error if decorator function has void context: code 1`] = ` +"local ____lualib = require("lualib_bundle") +local __TS__Class = ____lualib.__TS__Class +local __TS__Decorate = ____lualib.__TS__Decorate +local ____exports = {} +function ____exports.__main(self) + local function decorator(constructor) + end + local TestClass = __TS__Class() + TestClass.name = "TestClass" + function TestClass.prototype.____constructor(self) + end + TestClass = __TS__Decorate({decorator}, TestClass) +end +return ____exports" +`; + +exports[`legacy experimentalDecorators Throws error if decorator function has void context: diagnostics 1`] = `"main.ts(4,13): error TSTL: Decorator function cannot have 'this: void'."`; diff --git a/test/unit/classes/decorators.spec.ts b/test/unit/classes/decorators.spec.ts index abc4b948b..ae54b2a89 100644 --- a/test/unit/classes/decorators.spec.ts +++ b/test/unit/classes/decorators.spec.ts @@ -3,25 +3,29 @@ import * as util from "../../util"; test("Class decorator with no parameters", () => { util.testFunction` - function setBool {}>(constructor: T) { + let classDecoratorContext; + + function classDecorator {}>(constructor: T, context: ClassDecoratorContext) { + classDecoratorContext = context; + return class extends constructor { decoratorBool = true; } } - @setBool + @classDecorator class TestClass { public decoratorBool = false; } - return new TestClass(); + return { decoratedClass: new TestClass(), classDecoratorContext }; `.expectToMatchJsResult(); }); test("Class decorator with parameters", () => { util.testFunction` function setNum(numArg: number) { - return {}>(constructor: T) => { + return {}>(constructor: T, context: ClassDecoratorContext) => { return class extends constructor { decoratorNum = numArg; }; @@ -39,13 +43,13 @@ test("Class decorator with parameters", () => { test("Multiple class decorators", () => { util.testFunction` - function setTen {}>(constructor: T) { + function setTen {}>(constructor: T, context: ClassDecoratorContext) { return class extends constructor { decoratorTen = 10; } } - function setNum {}>(constructor: T) { + function setNum {}>(constructor: T, context: ClassDecoratorContext) { return class extends constructor { decoratorNum = 410; } @@ -64,7 +68,7 @@ test("Multiple class decorators", () => { test("Class decorator with inheritance", () => { util.testFunction` - function setTen {}>(constructor: T) { + function setTen {}>(constructor: T, context: ClassDecoratorContext) { return class extends constructor { decoratorTen = 10; } @@ -89,7 +93,7 @@ test("Class decorators are applied in order and executed in reverse order", () = function pushOrder(index: number) { order.push("eval " + index); - return (constructor: new (...args: any[]) => {}) => { + return (constructor: new (...args: any[]) => {}, context: ClassDecoratorContext) => { order.push("execute " + index); }; } @@ -105,7 +109,7 @@ test("Class decorators are applied in order and executed in reverse order", () = test("Throws error if decorator function has void context", () => { util.testFunction` - function decorator(this: void, constructor: new (...args: any[]) => {}) {} + function decorator(this: void, constructor: new (...args: any[]) => {}, context: ClassDecoratorContext) {} @decorator class TestClass {} @@ -114,7 +118,7 @@ test("Throws error if decorator function has void context", () => { test("Exported class decorator", () => { util.testModule` - function decorator any>(Class: T): T { + function decorator any>(Class: T, context: ClassDecoratorContext): T { return class extends Class { public bar = "foobar"; }; @@ -127,65 +131,6 @@ test("Exported class decorator", () => { .expectToMatchJsResult(); }); -test.each([ - ["@decorator method() {}"], - ["@decorator property;"], - ["@decorator propertyWithInitializer = () => {};"], - ["@decorator ['evaluated property'];"], - ["@decorator get getter() { return 5 }"], - ["@decorator set setter(value) {}"], - ["@decorator static method() {}"], - ["@decorator static property;"], - ["@decorator static propertyWithInitializer = () => {}"], - ["@decorator static get getter() { return 5 }"], - ["@decorator static set setter(value) {}"], - ["@decorator static ['evaluated property'];"], - ["method(@decorator a) {}"], - ["static method(@decorator a) {}"], - ["constructor(@decorator a) {}"], -])("Decorate class member (%p)", classMember => { - util.testFunction` - let decoratorParameters: any; - - const decorator = (target, key, index?) => { - const targetKind = target === Foo ? "Foo" : target === Foo.prototype ? "Foo.prototype" : "unknown"; - decoratorParameters = { targetKind, key, index: typeof index }; - }; - - class Foo { - ${classMember} - } - - return decoratorParameters; - `.expectToMatchJsResult(); -}); - -describe("Decorators /w descriptors", () => { - test.each([ - ["return { writable: true }", "return { configurable: true }"], - ["desc.writable = true", "return { configurable: true }"], - ])("Combine decorators (%p + %p)", (decorateA, decorateB) => { - util.testFunction` - const A = (target, key, desc): any => { ${decorateA} }; - const B = (target, key, desc): any => { ${decorateB} }; - class Foo { @A @B static method() {} } - const { value, ...rest } = Object.getOwnPropertyDescriptor(Foo, "method"); - return rest; - `.expectToMatchJsResult(); - }); - - test.each(["return { value: true }", "desc.value = true"])( - "Use decorator to override method value", - overrideStatement => { - util.testFunction` - const decorator = (target, key, desc): any => { ${overrideStatement} }; - class Foo { @decorator static method() {} } - return Foo.method; - `.expectToMatchJsResult(); - } - ); -}); - // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1149 test("exported class with decorator", () => { util.testModule` @@ -195,7 +140,7 @@ test("exported class with decorator", () => { ` .addExtraFile( "other.ts", - `function myDecorator(target: {new(): any}) { + `function myDecorator(target: {new(): any}, context: ClassDecoratorContext) { return class extends target { foo() { return "overridden"; @@ -221,7 +166,7 @@ test("default exported class with decorator", () => { ` .addExtraFile( "other.ts", - `function myDecorator(target: {new(): any}) { + `function myDecorator(target: {new(): any}, context: ClassDecoratorContext) { return class extends target { foo() { return "overridden"; @@ -238,3 +183,352 @@ test("default exported class with decorator", () => { ) .expectToEqual({ result: "overridden" }); }); + +test("Class method decorator", () => { + util.testFunction` + let methodDecoratorContext; + + function methodDecorator(method: (v: number) => number, context: ClassMethodDecoratorContext) { + methodDecoratorContext = context; + + return (v: number) => { + return method(v) + 10; + }; + } + + class TestClass { + @methodDecorator + public myMethod(x: number) { + return x * 23; + } + } + + return { result: new TestClass().myMethod(3), methodDecoratorContext }; + `.expectToMatchJsResult(); +}); + +test("class getter decorator", () => { + util.testFunction` + let getterDecoratorContext; + + function getterDecorator(getter: () => number, context: ClassGetterDecoratorContext) { + getterDecoratorContext = context; + + return () => { + return getter() + 10; + }; + } + + class TestClass { + @getterDecorator + get getterValue() { return 10; } + } + + return { result: new TestClass().getterValue, getterDecoratorContext }; + `.expectToMatchJsResult(); +}); + +test("class setter decorator", () => { + util.testFunction` + let setterDecoratorContext; + + function setterDecorator(setter: (v: number) => void, context: ClassSetterDecoratorContext) { + setterDecoratorContext = context; + + return (v: number) => { + setter(v + 15); + }; + } + + class TestClass { + public value: number; + + @setterDecorator + set valueSetter(v: number) { this.value = v; } + } + + const instance = new TestClass(); + instance.valueSetter = 23; + return { result: instance.value, setterDecoratorContext }; + `.expectToMatchJsResult(); +}); + +test("class field decorator", () => { + util.testFunction` + let fieldDecoratorContext; + + function fieldDecorator(initialValue: number, context: ClassFieldDecoratorContext) { + fieldDecoratorContext = context; + + return initialValue => initialValue * 12; + } + + class TestClass { + @fieldDecorator + public value: number = 22; + } + + return { result: new TestClass(), fieldDecoratorContext }; +`.expectToMatchJsResult(); +}); + +describe("legacy experimentalDecorators", () => { + test("Class decorator with no parameters", () => { + util.testFunction` + function setBool {}>(constructor: T) { + return class extends constructor { + decoratorBool = true; + } + } + + @setBool + class TestClass { + public decoratorBool = false; + } + + return new TestClass(); + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test("Class decorator with parameters", () => { + util.testFunction` + function setNum(numArg: number) { + return {}>(constructor: T) => { + return class extends constructor { + decoratorNum = numArg; + }; + }; + } + + @setNum(420) + class TestClass { + public decoratorNum; + } + + return new TestClass(); + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test("Multiple class decorators", () => { + util.testFunction` + function setTen {}>(constructor: T) { + return class extends constructor { + decoratorTen = 10; + } + } + + function setNum {}>(constructor: T) { + return class extends constructor { + decoratorNum = 410; + } + } + + @setTen + @setNum + class TestClass { + public decoratorTen; + public decoratorNum; + } + + return new TestClass(); + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test("Class decorator with inheritance", () => { + util.testFunction` + function setTen {}>(constructor: T) { + return class extends constructor { + decoratorTen = 10; + } + } + + class TestClass { + public decoratorTen = 0; + } + + @setTen + class SubTestClass extends TestClass { + public decoratorTen = 5; + } + + return new SubTestClass(); + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test("Class decorators are applied in order and executed in reverse order", () => { + util.testFunction` + const order = []; + + function pushOrder(index: number) { + order.push("eval " + index); + return (constructor: new (...args: any[]) => {}) => { + order.push("execute " + index); + }; + } + + @pushOrder(1) + @pushOrder(2) + @pushOrder(3) + class TestClass {} + + return order; + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test("Throws error if decorator function has void context", () => { + util.testFunction` + function decorator(this: void, constructor: new (...args: any[]) => {}) {} + + @decorator + class TestClass {} + ` + .setOptions({ experimentalDecorators: true }) + .expectDiagnosticsToMatchSnapshot([decoratorInvalidContext.code]); + }); + + test("Exported class decorator", () => { + util.testModule` + function decorator any>(Class: T): T { + return class extends Class { + public bar = "foobar"; + }; + } + + @decorator + export class Foo {} + ` + .setReturnExport("Foo", "bar") + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test.each([ + ["@decorator method() {}"], + ["@decorator property;"], + ["@decorator propertyWithInitializer = () => {};"], + ["@decorator ['evaluated property'];"], + ["@decorator get getter() { return 5 }"], + ["@decorator set setter(value) {}"], + ["@decorator static method() {}"], + ["@decorator static property;"], + ["@decorator static propertyWithInitializer = () => {}"], + ["@decorator static get getter() { return 5 }"], + ["@decorator static set setter(value) {}"], + ["@decorator static ['evaluated property'];"], + ["method(@decorator a) {}"], + ["static method(@decorator a) {}"], + ["constructor(@decorator a) {}"], + ])("Decorate class member (%p)", classMember => { + util.testFunction` + let decoratorParameters: any; + + const decorator = (target, key, index?) => { + const targetKind = target === Foo ? "Foo" : target === Foo.prototype ? "Foo.prototype" : "unknown"; + decoratorParameters = { targetKind, key, index: typeof index }; + }; + + class Foo { + ${classMember} + } + + return decoratorParameters; + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + describe("Decorators /w descriptors", () => { + test.each([ + ["return { writable: true }", "return { configurable: true }"], + ["desc.writable = true", "return { configurable: true }"], + ])("Combine decorators (%p + %p)", (decorateA, decorateB) => { + util.testFunction` + const A = (target, key, desc): any => { ${decorateA} }; + const B = (target, key, desc): any => { ${decorateB} }; + class Foo { @A @B static method() {} } + const { value, ...rest } = Object.getOwnPropertyDescriptor(Foo, "method"); + return rest; + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + }); + + test.each(["return { value: true }", "desc.value = true"])( + "Use decorator to override method value", + overrideStatement => { + util.testFunction` + const decorator = (target, key, desc): any => { ${overrideStatement} }; + class Foo { @decorator static method() {} } + return Foo.method; + ` + .setOptions({ experimentalDecorators: true }) + .expectToMatchJsResult(); + } + ); + }); + + // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1149 + test("exported class with decorator", () => { + util.testModule` + import { MyClass } from "./other"; + const inst = new MyClass(); + export const result = inst.foo(); + ` + .addExtraFile( + "other.ts", + `function myDecorator(target: {new(): any}) { + return class extends target { + foo() { + return "overridden"; + } + } + } + + @myDecorator + export class MyClass { + foo() { + return "foo"; + } + }` + ) + .setOptions({ experimentalDecorators: true }) + .expectToEqual({ result: "overridden" }); + }); + + test("default exported class with decorator", () => { + util.testModule` + import MyClass from "./other"; + const inst = new MyClass(); + export const result = inst.foo(); + ` + .addExtraFile( + "other.ts", + `function myDecorator(target: {new(): any}) { + return class extends target { + foo() { + return "overridden"; + } + } + } + + @myDecorator + export default class { + foo() { + return "foo"; + } + }` + ) + .setOptions({ experimentalDecorators: true }) + .expectToEqual({ result: "overridden" }); + }); +}); diff --git a/test/util.ts b/test/util.ts index f2c73e1e2..35eda7168 100644 --- a/test/util.ts +++ b/test/util.ts @@ -163,7 +163,6 @@ export abstract class TestBuilder { lib: ["lib.esnext.d.ts"], moduleResolution: ts.ModuleResolutionKind.Node10, resolveJsonModule: true, - experimentalDecorators: true, sourceMap: true, }; public setOptions(options: tstl.CompilerOptions = {}): this { From 57349d3e2281bec737e9f5576ab98a73eabe5dc1 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 29 May 2023 16:26:26 +0200 Subject: [PATCH 2/7] Rename old decorate function to DecorateLegacy --- src/LuaLib.ts | 2 +- src/lualib/{Decorate.ts => DecorateLegacy.ts} | 2 +- src/transformation/visitors/class/decorators.ts | 2 +- test/unit/classes/__snapshots__/decorators.spec.ts.snap | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) rename src/lualib/{Decorate.ts => DecorateLegacy.ts} (94%) diff --git a/src/LuaLib.ts b/src/LuaLib.ts index b2f6383a6..986c34603 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -37,7 +37,7 @@ export enum LuaLibFeature { ClassExtends = "ClassExtends", CloneDescriptor = "CloneDescriptor", CountVarargs = "CountVarargs", - Decorate = "Decorate", + DecorateLegacy = "DecorateLegacy", DecorateParam = "DecorateParam", Delete = "Delete", DelegatedYield = "DelegatedYield", diff --git a/src/lualib/Decorate.ts b/src/lualib/DecorateLegacy.ts similarity index 94% rename from src/lualib/Decorate.ts rename to src/lualib/DecorateLegacy.ts index f9f8d083e..30f254927 100644 --- a/src/lualib/Decorate.ts +++ b/src/lualib/DecorateLegacy.ts @@ -5,7 +5,7 @@ import { __TS__ObjectGetOwnPropertyDescriptor } from "./ObjectGetOwnPropertyDesc import { __TS__SetDescriptor } from "./SetDescriptor"; import { Decorator } from "./Decorator"; -export function __TS__Decorate( +export function __TS__DecorateLegacy( this: void, decorators: Array>, target: TTarget, diff --git a/src/transformation/visitors/class/decorators.ts b/src/transformation/visitors/class/decorators.ts index 4b913485c..31d7192bc 100644 --- a/src/transformation/visitors/class/decorators.ts +++ b/src/transformation/visitors/class/decorators.ts @@ -35,5 +35,5 @@ export function createDecoratingExpression( trailingExpressions.push(isMethodOrAccessor ? lua.createBooleanLiteral(true) : lua.createNilLiteral()); } - return transformLuaLibFunction(context, LuaLibFeature.Decorate, undefined, ...trailingExpressions); + return transformLuaLibFunction(context, LuaLibFeature.DecorateLegacy, undefined, ...trailingExpressions); } diff --git a/test/unit/classes/__snapshots__/decorators.spec.ts.snap b/test/unit/classes/__snapshots__/decorators.spec.ts.snap index 1a8407ccd..a2deca171 100644 --- a/test/unit/classes/__snapshots__/decorators.spec.ts.snap +++ b/test/unit/classes/__snapshots__/decorators.spec.ts.snap @@ -3,7 +3,7 @@ exports[`Throws error if decorator function has void context: code 1`] = ` "local ____lualib = require("lualib_bundle") local __TS__Class = ____lualib.__TS__Class -local __TS__Decorate = ____lualib.__TS__Decorate +local __TS__DecorateLegacy = ____lualib.__TS__DecorateLegacy local ____exports = {} function ____exports.__main(self) local function decorator(constructor, context) @@ -12,7 +12,7 @@ function ____exports.__main(self) TestClass.name = "TestClass" function TestClass.prototype.____constructor(self) end - TestClass = __TS__Decorate({decorator}, TestClass) + TestClass = __TS__DecorateLegacy({decorator}, TestClass) end return ____exports" `; @@ -22,7 +22,7 @@ exports[`Throws error if decorator function has void context: diagnostics 1`] = exports[`legacy experimentalDecorators Throws error if decorator function has void context: code 1`] = ` "local ____lualib = require("lualib_bundle") local __TS__Class = ____lualib.__TS__Class -local __TS__Decorate = ____lualib.__TS__Decorate +local __TS__DecorateLegacy = ____lualib.__TS__DecorateLegacy local ____exports = {} function ____exports.__main(self) local function decorator(constructor) @@ -31,7 +31,7 @@ function ____exports.__main(self) TestClass.name = "TestClass" function TestClass.prototype.____constructor(self) end - TestClass = __TS__Decorate({decorator}, TestClass) + TestClass = __TS__DecorateLegacy({decorator}, TestClass) end return ____exports" `; From e1cf251cbd6d79e11de2ebb42999c572f835dd3e Mon Sep 17 00:00:00 2001 From: Perryvw Date: Mon, 29 May 2023 22:58:24 +0200 Subject: [PATCH 3/7] Method decorators and partially accessor decorators rewritten --- src/LuaLib.ts | 1 + src/lualib/Decorate.ts | 22 ++ src/lualib/DecorateLegacy.ts | 11 +- src/lualib/DecorateParam.ts | 4 +- src/lualib/Decorator.d.ts | 6 +- .../visitors/class/decorators.ts | 194 +++++++++++++++++- src/transformation/visitors/class/index.ts | 66 +++--- .../visitors/class/members/accessors.ts | 24 ++- .../visitors/class/members/fields.ts | 21 -- .../visitors/class/members/method.ts | 75 ++----- src/transformation/visitors/class/utils.ts | 4 + test/unit/classes/decorators.spec.ts | 49 ++++- 12 files changed, 330 insertions(+), 147 deletions(-) create mode 100644 src/lualib/Decorate.ts diff --git a/src/LuaLib.ts b/src/LuaLib.ts index 986c34603..4bf86aa5c 100644 --- a/src/LuaLib.ts +++ b/src/LuaLib.ts @@ -37,6 +37,7 @@ export enum LuaLibFeature { ClassExtends = "ClassExtends", CloneDescriptor = "CloneDescriptor", CountVarargs = "CountVarargs", + Decorate = "Decorate", DecorateLegacy = "DecorateLegacy", DecorateParam = "DecorateParam", Delete = "Delete", diff --git a/src/lualib/Decorate.ts b/src/lualib/Decorate.ts new file mode 100644 index 000000000..fd65f44e8 --- /dev/null +++ b/src/lualib/Decorate.ts @@ -0,0 +1,22 @@ +/** + * TypeScript 5.0 decorators + */ +import { Decorator } from "./Decorator"; + +export function __TS__Decorate( + this: TClass, + originalValue: TTarget, + decorators: Array>, + context: DecoratorContext +): TTarget { + let result = originalValue; + + for (let i = decorators.length; i >= 0; i--) { + const decorator = decorators[i]; + if (decorator !== undefined) { + result = decorator.call(this, result, context) ?? result; + } + } + + return result; +} diff --git a/src/lualib/DecorateLegacy.ts b/src/lualib/DecorateLegacy.ts index 30f254927..0369623d3 100644 --- a/src/lualib/DecorateLegacy.ts +++ b/src/lualib/DecorateLegacy.ts @@ -1,13 +1,18 @@ /** - * SEE: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/transformers/ts.ts#L3598 + * Old-style decorators, activated by enabling the experimentalDecorators flag */ import { __TS__ObjectGetOwnPropertyDescriptor } from "./ObjectGetOwnPropertyDescriptor"; import { __TS__SetDescriptor } from "./SetDescriptor"; -import { Decorator } from "./Decorator"; + +export type LegacyDecorator = ( + target: TTarget, + key?: TKey, + descriptor?: PropertyDescriptor +) => TTarget; export function __TS__DecorateLegacy( this: void, - decorators: Array>, + decorators: Array>, target: TTarget, key?: TKey, desc?: any diff --git a/src/lualib/DecorateParam.ts b/src/lualib/DecorateParam.ts index 2eb59bc96..0165294b2 100644 --- a/src/lualib/DecorateParam.ts +++ b/src/lualib/DecorateParam.ts @@ -1,4 +1,4 @@ -import { Decorator } from "./Decorator"; +import type { LegacyDecorator } from "./DecorateLegacy"; type ParamDecorator = ( target: TTarget, @@ -9,6 +9,6 @@ export function __TS__DecorateParam -): Decorator { +): LegacyDecorator { return (target: TTarget, key?: TKey) => decorator(target, key, paramIndex); } diff --git a/src/lualib/Decorator.d.ts b/src/lualib/Decorator.d.ts index 338b11b3f..d8f84febf 100644 --- a/src/lualib/Decorator.d.ts +++ b/src/lualib/Decorator.d.ts @@ -1,5 +1 @@ -export type Decorator = ( - target: TTarget, - key?: TKey, - descriptor?: PropertyDescriptor -) => TTarget; +export type Decorator = (target: TTarget, context: DecoratorContext) => TTarget; diff --git a/src/transformation/visitors/class/decorators.ts b/src/transformation/visitors/class/decorators.ts index 31d7192bc..f2fb6aebd 100644 --- a/src/transformation/visitors/class/decorators.ts +++ b/src/transformation/visitors/class/decorators.ts @@ -4,6 +4,10 @@ import { TransformationContext } from "../../context"; import { decoratorInvalidContext } from "../../utils/diagnostics"; import { ContextType, getFunctionContextType } from "../../utils/function-context"; import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib"; +import { isNonNull } from "../../../utils"; +import { transformMemberExpressionOwnerName, transformMethodName } from "./members/method"; +import { transformPropertyName } from "../literal"; +import { isPrivateNode, isStaticNode } from "./utils"; export function transformDecoratorExpression(context: TransformationContext, decorator: ts.Decorator): lua.Expression { const expression = decorator.expression; @@ -16,7 +20,161 @@ export function transformDecoratorExpression(context: TransformationContext, dec return context.transformExpression(expression); } -export function createDecoratingExpression( +export function createClassDecoratingExpression( + context: TransformationContext, + classDeclaration: ts.ClassDeclaration | ts.ClassExpression, + className: lua.Expression +): lua.Expression { + // If experimentalDecorators flag is set, decorate with legacy decorator logic + if (context.options.experimentalDecorators) { + return createLegacyDecoratingExpression( + context, + classDeclaration.kind, + ts.getDecorators(classDeclaration)?.map(d => transformDecoratorExpression(context, d)) ?? [], + className + ); + } + + // Else: TypeScript 5.0 decorator + return lua.createNilLiteral(); +} + +export function createClassMethodDecoratingExpression( + context: TransformationContext, + methodDeclaration: ts.MethodDeclaration, + originalMethod: lua.Expression, + className: lua.Identifier +): lua.Expression { + const parameterDecorators = getParameterDecorators(context, methodDeclaration); + const methodDecorators = + ts.getDecorators(methodDeclaration)?.map(d => transformDecoratorExpression(context, d)) ?? []; + + const methodName = transformMethodName(context, methodDeclaration); + + // If experimentalDecorators flag is set, decorate with legacy decorator logic + if (context.options.experimentalDecorators) { + const methodTable = transformMemberExpressionOwnerName(methodDeclaration, className); + return createLegacyDecoratingExpression( + context, + methodDeclaration.kind, + [...methodDecorators, ...parameterDecorators], + methodTable, + methodName + ); + } + + // Else: TypeScript 5.0 decorator + return createDecoratingExpression(context, className, originalMethod, methodDecorators, { + kind: lua.createStringLiteral("method"), + name: methodName, + private: lua.createBooleanLiteral(isPrivateNode(methodDeclaration)), + static: lua.createBooleanLiteral(isStaticNode(methodDeclaration)), + }); +} + +export function createClassAccessorDecoratingStatements( + context: TransformationContext, + accessor: ts.AccessorDeclaration, + originalAccessor: lua.Expression, + className: lua.Identifier +): lua.Expression { + const accessorDecorators = ts.getDecorators(accessor)?.map(d => transformDecoratorExpression(context, d)) ?? []; + const propertyName = transformPropertyName(context, accessor.name); + + // If experimentalDecorators flag is set, decorate with legacy decorator logic + if (context.options.experimentalDecorators) { + const propertyOwnerTable = transformMemberExpressionOwnerName(accessor, className); + + return createLegacyDecoratingExpression( + context, + accessor.kind, + accessorDecorators, + propertyOwnerTable, + propertyName + ); + } + + // Else: TypeScript 5.0 decorator + return createDecoratingExpression(context, className, originalAccessor, accessorDecorators, { + kind: lua.createStringLiteral("accessor"), + name: propertyName, + private: lua.createBooleanLiteral(isPrivateNode(accessor)), + static: lua.createBooleanLiteral(isStaticNode(accessor)), + }); +} + +export function createClassPropertyDecoratingStatements( + context: TransformationContext, + property: ts.PropertyDeclaration, + className: lua.Identifier +): lua.Statement[] { + const propertyDecorators = ts.getDecorators(property)?.map(d => transformDecoratorExpression(context, d)) ?? []; + if (propertyDecorators.length === 0) return []; + + // If experimentalDecorators flag is set, decorate with legacy decorator logic + if (context.options.experimentalDecorators) { + const propertyName = transformPropertyName(context, property.name); + const propertyOwnerTable = transformMemberExpressionOwnerName(property, className); + + return [ + lua.createExpressionStatement( + createLegacyDecoratingExpression( + context, + property.kind, + propertyDecorators, + propertyOwnerTable, + propertyName + ) + ), + ]; + } + + // Else: TypeScript 5.0 decorator + const decoratorTable = lua.createTableExpression(propertyDecorators.map(d => lua.createTableFieldExpression(d))); + const decoratorContext = lua.createTableExpression([ + lua.createTableFieldExpression(lua.createStringLiteral("field"), lua.createStringLiteral("kind")), + ]); + const decorateCall = transformLuaLibFunction( + context, + LuaLibFeature.Decorate, + undefined, + className, + decoratorTable, + decoratorContext + ); + + return [lua.createExpressionStatement(decorateCall)]; +} + +function createDecoratingExpression( + context: TransformationContext, + className: lua.Expression, + originalValue: TValue, + decorators: lua.Expression[], + decoratorContext: Record +): lua.Expression { + const decoratorTable = lua.createTableExpression(decorators.map(d => lua.createTableFieldExpression(d))); + const decoratorContextTable = objectToLuaTableLiteral(decoratorContext); + + return transformLuaLibFunction( + context, + LuaLibFeature.Decorate, + undefined, + className, + originalValue, + decoratorTable, + decoratorContextTable + ); +} + +function objectToLuaTableLiteral(obj: Record): lua.Expression { + return lua.createTableExpression( + Object.entries(obj).map(([key, value]) => lua.createTableFieldExpression(value, lua.createStringLiteral(key))) + ); +} + +// Legacy decorators: +function createLegacyDecoratingExpression( context: TransformationContext, kind: ts.SyntaxKind, decorators: lua.Expression[], @@ -37,3 +195,37 @@ export function createDecoratingExpression( return transformLuaLibFunction(context, LuaLibFeature.DecorateLegacy, undefined, ...trailingExpressions); } + +function getParameterDecorators( + context: TransformationContext, + node: ts.FunctionLikeDeclarationBase +): lua.CallExpression[] { + return node.parameters + .flatMap((parameter, index) => + ts + .getDecorators(parameter) + ?.map(decorator => + transformLuaLibFunction( + context, + LuaLibFeature.DecorateParam, + node, + lua.createNumericLiteral(index), + transformDecoratorExpression(context, decorator) + ) + ) + ) + .filter(isNonNull); +} + +export function createConstructorDecoratingExpression( + context: TransformationContext, + node: ts.ConstructorDeclaration, + className: lua.Identifier +): lua.Statement | undefined { + const parameterDecorators = getParameterDecorators(context, node); + + if (parameterDecorators.length > 0) { + const decorateMethod = createLegacyDecoratingExpression(context, node.kind, parameterDecorators, className); + return lua.createExpressionStatement(decorateMethod); + } +} diff --git a/src/transformation/visitors/class/index.ts b/src/transformation/visitors/class/index.ts index c5b4d9630..1b9dc36b9 100644 --- a/src/transformation/visitors/class/index.ts +++ b/src/transformation/visitors/class/index.ts @@ -11,19 +11,15 @@ import { import { createSelfIdentifier } from "../../utils/lua-ast"; import { createSafeName, isUnsafeName } from "../../utils/safe-names"; import { transformIdentifier } from "../identifier"; -import { createDecoratingExpression, transformDecoratorExpression } from "./decorators"; -import { transformAccessorDeclarations } from "./members/accessors"; -import { createConstructorName, transformConstructorDeclaration } from "./members/constructor"; -import { - createPropertyDecoratingExpression, - transformClassInstanceFields, - transformStaticPropertyDeclaration, -} from "./members/fields"; import { + createClassDecoratingExpression, + createClassPropertyDecoratingStatements, createConstructorDecoratingExpression, - createMethodDecoratingExpression, - transformMethodDeclaration, -} from "./members/method"; +} from "./decorators"; +import { transformAccessorDeclarations } from "./members/accessors"; +import { createConstructorName, transformConstructorDeclaration } from "./members/constructor"; +import { transformClassInstanceFields, transformStaticPropertyDeclaration } from "./members/fields"; +import { transformMethodDeclaration } from "./members/method"; import { getExtendedNode, getExtendedType, isStaticNode } from "./utils"; import { createClassSetup } from "./setup"; import { LuaTarget } from "../../../CompilerOptions"; @@ -121,6 +117,7 @@ function transformClassLikeDeclaration( if (constructorResult) result.push(constructorResult); + // Legacy constructor decorator const decoratingExpression = createConstructorDecoratingExpression(context, constructor, localClassName); if (decoratingExpression) result.push(decoratingExpression); } else if (!extendedType) { @@ -165,51 +162,36 @@ function transformClassLikeDeclaration( ); } - // Transform accessors - for (const member of classDeclaration.members) { - if (!ts.isAccessor(member)) continue; - const accessors = context.resolver.getAllAccessorDeclarations(member); - if (accessors.firstAccessor !== member) continue; - - const accessorsResult = transformAccessorDeclarations(context, accessors, localClassName); - if (accessorsResult) { - result.push(accessorsResult); - } - } - - const decorationStatements: lua.Statement[] = []; - + // Transform class members for (const member of classDeclaration.members) { if (ts.isAccessor(member)) { - const expression = createPropertyDecoratingExpression(context, member, localClassName); - if (expression) decorationStatements.push(lua.createExpressionStatement(expression)); + // Accessors + const accessors = context.resolver.getAllAccessorDeclarations(member); + if (accessors.firstAccessor !== member) continue; + + const accessorsResult = transformAccessorDeclarations(context, accessors, localClassName); + if (accessorsResult) { + result.push(accessorsResult); + } } else if (ts.isMethodDeclaration(member)) { + // Methods const statement = transformMethodDeclaration(context, member, localClassName); if (statement) result.push(statement); - if (member.body) { - const statement = createMethodDecoratingExpression(context, member, localClassName); - if (statement) decorationStatements.push(statement); - } } else if (ts.isPropertyDeclaration(member)) { + // Properties if (isStaticNode(member)) { const statement = transformStaticPropertyDeclaration(context, member, localClassName); - if (statement) decorationStatements.push(statement); + if (statement) result.push(statement); } - const expression = createPropertyDecoratingExpression(context, member, localClassName); - if (expression) decorationStatements.push(lua.createExpressionStatement(expression)); + + // Add decorating statements + result.push(...createClassPropertyDecoratingStatements(context, member, className)); } } - result.push(...decorationStatements); - // Decorate the class if (ts.canHaveDecorators(classDeclaration) && ts.getDecorators(classDeclaration)) { - const decoratingExpression = createDecoratingExpression( - context, - classDeclaration.kind, - ts.getDecorators(classDeclaration)?.map(d => transformDecoratorExpression(context, d)) ?? [], - localClassName - ); + const decoratingExpression = createClassDecoratingExpression(context, classDeclaration, localClassName); const decoratingStatement = lua.createAssignmentStatement(localClassName, decoratingExpression); result.push(decoratingStatement); diff --git a/src/transformation/visitors/class/members/accessors.ts b/src/transformation/visitors/class/members/accessors.ts index fe34a3d8a..da09fc90c 100644 --- a/src/transformation/visitors/class/members/accessors.ts +++ b/src/transformation/visitors/class/members/accessors.ts @@ -7,11 +7,27 @@ import { transformFunctionBody, transformParameters } from "../../function"; import { transformPropertyName } from "../../literal"; import { isStaticNode } from "../utils"; import { createPrototypeName } from "./constructor"; +import { createClassAccessorDecoratingStatements } from "../decorators"; -function transformAccessor(context: TransformationContext, node: ts.AccessorDeclaration): lua.FunctionExpression { +function transformAccessor( + context: TransformationContext, + node: ts.AccessorDeclaration, + className: lua.Identifier +): lua.Expression { const [params, dot, restParam] = transformParameters(context, node.parameters, createSelfIdentifier()); const body = node.body ? transformFunctionBody(context, node.parameters, node.body, restParam)[0] : []; - return lua.createFunctionExpression(lua.createBlock(body), params, dot, lua.NodeFlags.Declaration); + const accessorFunction = lua.createFunctionExpression( + lua.createBlock(body), + params, + dot, + lua.NodeFlags.Declaration + ); + + if (ts.getDecorators(node)?.length) { + return createClassAccessorDecoratingStatements(context, node, accessorFunction, className); + } else { + return accessorFunction; + } } export function transformAccessorDeclarations( @@ -23,12 +39,12 @@ export function transformAccessorDeclarations( const descriptor = lua.createTableExpression([]); if (getAccessor) { - const getterFunction = transformAccessor(context, getAccessor); + const getterFunction = transformAccessor(context, getAccessor, className); descriptor.fields.push(lua.createTableFieldExpression(getterFunction, lua.createStringLiteral("get"))); } if (setAccessor) { - const setterFunction = transformAccessor(context, setAccessor); + const setterFunction = transformAccessor(context, setAccessor, className); descriptor.fields.push(lua.createTableFieldExpression(setterFunction, lua.createStringLiteral("set"))); } diff --git a/src/transformation/visitors/class/members/fields.ts b/src/transformation/visitors/class/members/fields.ts index 0e7c3e51f..9d9009ae3 100644 --- a/src/transformation/visitors/class/members/fields.ts +++ b/src/transformation/visitors/class/members/fields.ts @@ -4,27 +4,6 @@ import { TransformationContext } from "../../../context"; import { createSelfIdentifier } from "../../../utils/lua-ast"; import { transformInPrecedingStatementScope } from "../../../utils/preceding-statements"; import { transformPropertyName } from "../../literal"; -import { createDecoratingExpression, transformDecoratorExpression } from "../decorators"; -import { transformMemberExpressionOwnerName } from "./method"; - -export function createPropertyDecoratingExpression( - context: TransformationContext, - node: ts.PropertyDeclaration | ts.AccessorDeclaration, - className: lua.Identifier -): lua.Expression | undefined { - if (!ts.canHaveDecorators(node)) return; - const decorators = ts.getDecorators(node); - if (!decorators) return; - const propertyName = transformPropertyName(context, node.name); - const propertyOwnerTable = transformMemberExpressionOwnerName(node, className); - return createDecoratingExpression( - context, - node.kind, - decorators.map(d => transformDecoratorExpression(context, d)), - propertyOwnerTable, - propertyName - ); -} export function transformClassInstanceFields( context: TransformationContext, diff --git a/src/transformation/visitors/class/members/method.ts b/src/transformation/visitors/class/members/method.ts index 0c3c10672..47eae9877 100644 --- a/src/transformation/visitors/class/members/method.ts +++ b/src/transformation/visitors/class/members/method.ts @@ -4,10 +4,8 @@ import { TransformationContext } from "../../../context"; import { transformFunctionToExpression } from "../../function"; import { transformPropertyName } from "../../literal"; import { isStaticNode } from "../utils"; -import { createDecoratingExpression, transformDecoratorExpression } from "../decorators"; import { createPrototypeName } from "./constructor"; -import { transformLuaLibFunction, LuaLibFeature } from "../../../utils/lualib"; -import { isNonNull } from "../../../../utils"; +import { createClassMethodDecoratingExpression } from "../decorators"; export function transformMemberExpressionOwnerName( node: ts.PropertyDeclaration | ts.MethodDeclaration | ts.AccessorDeclaration, @@ -36,66 +34,17 @@ export function transformMethodDeclaration( const methodName = transformMethodName(context, node); const [functionExpression] = transformFunctionToExpression(context, node); - return lua.createAssignmentStatement( - lua.createTableIndexExpression(methodTable, methodName), - functionExpression, - node - ); -} - -export function getParameterDecorators( - context: TransformationContext, - node: ts.FunctionLikeDeclarationBase -): lua.CallExpression[] { - return node.parameters - .flatMap((parameter, index) => - ts - .getDecorators(parameter) - ?.map(decorator => - transformLuaLibFunction( - context, - LuaLibFeature.DecorateParam, - node, - lua.createNumericLiteral(index), - transformDecoratorExpression(context, decorator) - ) - ) - ) - .filter(isNonNull); -} - -export function createConstructorDecoratingExpression( - context: TransformationContext, - node: ts.ConstructorDeclaration, - className: lua.Identifier -): lua.Statement | undefined { - const parameterDecorators = getParameterDecorators(context, node); - - if (parameterDecorators.length > 0) { - const decorateMethod = createDecoratingExpression(context, node.kind, parameterDecorators, className); - return lua.createExpressionStatement(decorateMethod); - } -} - -export function createMethodDecoratingExpression( - context: TransformationContext, - node: ts.MethodDeclaration, - className: lua.Identifier -): lua.Statement | undefined { - const methodTable = transformMemberExpressionOwnerName(node, className); - const methodName = transformMethodName(context, node); - - const parameterDecorators = getParameterDecorators(context, node); - const methodDecorators = ts.getDecorators(node)?.map(d => transformDecoratorExpression(context, d)) ?? []; - - if (methodDecorators.length > 0 || parameterDecorators.length > 0) { - const decorateMethod = createDecoratingExpression( - context, - node.kind, - [...methodDecorators, ...parameterDecorators], - methodTable, - methodName + if (ts.getDecorators(node)?.length) { + return lua.createAssignmentStatement( + lua.createTableIndexExpression(methodTable, methodName), + createClassMethodDecoratingExpression(context, node, functionExpression, className), + node + ); + } else { + return lua.createAssignmentStatement( + lua.createTableIndexExpression(methodTable, methodName), + functionExpression, + node ); - return lua.createExpressionStatement(decorateMethod); } } diff --git a/src/transformation/visitors/class/utils.ts b/src/transformation/visitors/class/utils.ts index d5b960662..0cd4384bb 100644 --- a/src/transformation/visitors/class/utils.ts +++ b/src/transformation/visitors/class/utils.ts @@ -1,6 +1,10 @@ import * as ts from "typescript"; import { TransformationContext } from "../../context"; +export function isPrivateNode(node: ts.HasModifiers): boolean { + return node.modifiers?.some(m => m.kind === ts.SyntaxKind.PrivateKeyword) === true; +} + export function isStaticNode(node: ts.HasModifiers): boolean { return node.modifiers?.some(m => m.kind === ts.SyntaxKind.StaticKeyword) === true; } diff --git a/test/unit/classes/decorators.spec.ts b/test/unit/classes/decorators.spec.ts index ae54b2a89..1a880ce24 100644 --- a/test/unit/classes/decorators.spec.ts +++ b/test/unit/classes/decorators.spec.ts @@ -184,7 +184,7 @@ test("default exported class with decorator", () => { .expectToEqual({ result: "overridden" }); }); -test("Class method decorator", () => { +test("class method decorator", () => { util.testFunction` let methodDecoratorContext; @@ -203,7 +203,34 @@ test("Class method decorator", () => { } } - return { result: new TestClass().myMethod(3), methodDecoratorContext }; + return { result: new TestClass().myMethod(4), context: { + kind: methodDecoratorContext.kind, + name: methodDecoratorContext.name, + private: methodDecoratorContext.private, + static: methodDecoratorContext.static + } }; + `.expectToMatchJsResult(); +}); + +test("this in decorator points to class being decorated", () => { + util.testFunction` + function methodDecorator(method: (v: number) => number, context: ClassMethodDecoratorContext) { + return function() { + const thisCallTime = this.myInstanceVariable; + return thisCallTime; + }; + } + + class TestClass { + constructor(protected myInstanceVariable: number) { } + + @methodDecorator + public myMethod() { + return 0; + } + } + + return new TestClass(5).myMethod(); `.expectToMatchJsResult(); }); @@ -224,7 +251,12 @@ test("class getter decorator", () => { get getterValue() { return 10; } } - return { result: new TestClass().getterValue, getterDecoratorContext }; + return { result: new TestClass().getterValue, context: { + kind: getterDecoratorContext.kind, + name: getterDecoratorContext.name, + private: getterDecoratorContext.private, + static: getterDecoratorContext.static + } }; `.expectToMatchJsResult(); }); @@ -235,8 +267,8 @@ test("class setter decorator", () => { function setterDecorator(setter: (v: number) => void, context: ClassSetterDecoratorContext) { setterDecoratorContext = context; - return (v: number) => { - setter(v + 15); + return function(v: number) { + setter.call(this, v + 15); }; } @@ -249,7 +281,12 @@ test("class setter decorator", () => { const instance = new TestClass(); instance.valueSetter = 23; - return { result: instance.value, setterDecoratorContext }; + return { result: instance.value, context: { + kind: setterDecoratorContext.kind, + name: setterDecoratorContext.name, + private: setterDecoratorContext.private, + static: setterDecoratorContext.static + } }; `.expectToMatchJsResult(); }); From b62db353724e192486677543a8dea80978ca6f98 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Tue, 30 May 2023 22:21:24 +0200 Subject: [PATCH 4/7] Modern decorators except field decorators working but with some regression to legacy --- .../visitors/class/decorators.ts | 63 ++++++++----------- src/transformation/visitors/class/index.ts | 9 +-- .../visitors/class/members/accessors.ts | 4 +- .../visitors/class/members/fields.ts | 11 +++- .../__snapshots__/decorators.spec.ts.snap | 4 +- test/unit/classes/decorators.spec.ts | 32 ++++++---- 6 files changed, 61 insertions(+), 62 deletions(-) diff --git a/src/transformation/visitors/class/decorators.ts b/src/transformation/visitors/class/decorators.ts index f2fb6aebd..e249fb87a 100644 --- a/src/transformation/visitors/class/decorators.ts +++ b/src/transformation/visitors/class/decorators.ts @@ -25,18 +25,19 @@ export function createClassDecoratingExpression( classDeclaration: ts.ClassDeclaration | ts.ClassExpression, className: lua.Expression ): lua.Expression { + const classDecorators = + ts.getDecorators(classDeclaration)?.map(d => transformDecoratorExpression(context, d)) ?? []; + // If experimentalDecorators flag is set, decorate with legacy decorator logic if (context.options.experimentalDecorators) { - return createLegacyDecoratingExpression( - context, - classDeclaration.kind, - ts.getDecorators(classDeclaration)?.map(d => transformDecoratorExpression(context, d)) ?? [], - className - ); + return createLegacyDecoratingExpression(context, classDeclaration.kind, classDecorators, className); } // Else: TypeScript 5.0 decorator - return lua.createNilLiteral(); + return createDecoratingExpression(context, className, className, classDecorators, { + kind: lua.createStringLiteral("class"), + name: lua.createStringLiteral(classDeclaration.name?.getText() ?? ""), + }); } export function createClassMethodDecoratingExpression( @@ -72,7 +73,7 @@ export function createClassMethodDecoratingExpression( }); } -export function createClassAccessorDecoratingStatements( +export function createClassAccessorDecoratingExpression( context: TransformationContext, accessor: ts.AccessorDeclaration, originalAccessor: lua.Expression, @@ -96,54 +97,42 @@ export function createClassAccessorDecoratingStatements( // Else: TypeScript 5.0 decorator return createDecoratingExpression(context, className, originalAccessor, accessorDecorators, { - kind: lua.createStringLiteral("accessor"), + kind: lua.createStringLiteral(accessor.kind === ts.SyntaxKind.SetAccessor ? "setter" : "getter"), name: propertyName, private: lua.createBooleanLiteral(isPrivateNode(accessor)), static: lua.createBooleanLiteral(isStaticNode(accessor)), }); } -export function createClassPropertyDecoratingStatements( +export function createClassPropertyDecoratingExpression( context: TransformationContext, property: ts.PropertyDeclaration, + originalValue: lua.Expression, className: lua.Identifier -): lua.Statement[] { +): lua.Expression { const propertyDecorators = ts.getDecorators(property)?.map(d => transformDecoratorExpression(context, d)) ?? []; - if (propertyDecorators.length === 0) return []; // If experimentalDecorators flag is set, decorate with legacy decorator logic if (context.options.experimentalDecorators) { const propertyName = transformPropertyName(context, property.name); const propertyOwnerTable = transformMemberExpressionOwnerName(property, className); - return [ - lua.createExpressionStatement( - createLegacyDecoratingExpression( - context, - property.kind, - propertyDecorators, - propertyOwnerTable, - propertyName - ) - ), - ]; + return createLegacyDecoratingExpression( + context, + property.kind, + propertyDecorators, + propertyOwnerTable, + propertyName + ); } // Else: TypeScript 5.0 decorator - const decoratorTable = lua.createTableExpression(propertyDecorators.map(d => lua.createTableFieldExpression(d))); - const decoratorContext = lua.createTableExpression([ - lua.createTableFieldExpression(lua.createStringLiteral("field"), lua.createStringLiteral("kind")), - ]); - const decorateCall = transformLuaLibFunction( - context, - LuaLibFeature.Decorate, - undefined, - className, - decoratorTable, - decoratorContext - ); - - return [lua.createExpressionStatement(decorateCall)]; + return createDecoratingExpression(context, className, originalValue, propertyDecorators, { + kind: lua.createStringLiteral("field"), + name: lua.createStringLiteral(property.name.getText()), + private: lua.createBooleanLiteral(isPrivateNode(property)), + static: lua.createBooleanLiteral(isStaticNode(property)), + }); } function createDecoratingExpression( diff --git a/src/transformation/visitors/class/index.ts b/src/transformation/visitors/class/index.ts index 1b9dc36b9..1ca760e32 100644 --- a/src/transformation/visitors/class/index.ts +++ b/src/transformation/visitors/class/index.ts @@ -11,11 +11,7 @@ import { import { createSelfIdentifier } from "../../utils/lua-ast"; import { createSafeName, isUnsafeName } from "../../utils/safe-names"; import { transformIdentifier } from "../identifier"; -import { - createClassDecoratingExpression, - createClassPropertyDecoratingStatements, - createConstructorDecoratingExpression, -} from "./decorators"; +import { createClassDecoratingExpression, createConstructorDecoratingExpression } from "./decorators"; import { transformAccessorDeclarations } from "./members/accessors"; import { createConstructorName, transformConstructorDeclaration } from "./members/constructor"; import { transformClassInstanceFields, transformStaticPropertyDeclaration } from "./members/fields"; @@ -183,9 +179,6 @@ function transformClassLikeDeclaration( const statement = transformStaticPropertyDeclaration(context, member, localClassName); if (statement) result.push(statement); } - - // Add decorating statements - result.push(...createClassPropertyDecoratingStatements(context, member, className)); } } diff --git a/src/transformation/visitors/class/members/accessors.ts b/src/transformation/visitors/class/members/accessors.ts index da09fc90c..165d3fa88 100644 --- a/src/transformation/visitors/class/members/accessors.ts +++ b/src/transformation/visitors/class/members/accessors.ts @@ -7,7 +7,7 @@ import { transformFunctionBody, transformParameters } from "../../function"; import { transformPropertyName } from "../../literal"; import { isStaticNode } from "../utils"; import { createPrototypeName } from "./constructor"; -import { createClassAccessorDecoratingStatements } from "../decorators"; +import { createClassAccessorDecoratingExpression } from "../decorators"; function transformAccessor( context: TransformationContext, @@ -24,7 +24,7 @@ function transformAccessor( ); if (ts.getDecorators(node)?.length) { - return createClassAccessorDecoratingStatements(context, node, accessorFunction, className); + return createClassAccessorDecoratingExpression(context, node, accessorFunction, className); } else { return accessorFunction; } diff --git a/src/transformation/visitors/class/members/fields.ts b/src/transformation/visitors/class/members/fields.ts index 9d9009ae3..1e623c381 100644 --- a/src/transformation/visitors/class/members/fields.ts +++ b/src/transformation/visitors/class/members/fields.ts @@ -4,6 +4,7 @@ import { TransformationContext } from "../../../context"; import { createSelfIdentifier } from "../../../utils/lua-ast"; import { transformInPrecedingStatementScope } from "../../../utils/preceding-statements"; import { transformPropertyName } from "../../literal"; +import { createClassPropertyDecoratingExpression } from "../decorators"; export function transformClassInstanceFields( context: TransformationContext, @@ -42,5 +43,13 @@ export function transformStaticPropertyDeclaration( const fieldName = transformPropertyName(context, field.name); const value = context.transformExpression(field.initializer); const classField = lua.createTableIndexExpression(lua.cloneIdentifier(className), fieldName); - return lua.createAssignmentStatement(classField, value); + + if (ts.getDecorators(field)?.length) { + return lua.createAssignmentStatement( + classField, + createClassPropertyDecoratingExpression(context, field, value, className) + ); + } else { + return lua.createAssignmentStatement(classField, value); + } } diff --git a/test/unit/classes/__snapshots__/decorators.spec.ts.snap b/test/unit/classes/__snapshots__/decorators.spec.ts.snap index a2deca171..027441192 100644 --- a/test/unit/classes/__snapshots__/decorators.spec.ts.snap +++ b/test/unit/classes/__snapshots__/decorators.spec.ts.snap @@ -3,7 +3,7 @@ exports[`Throws error if decorator function has void context: code 1`] = ` "local ____lualib = require("lualib_bundle") local __TS__Class = ____lualib.__TS__Class -local __TS__DecorateLegacy = ____lualib.__TS__DecorateLegacy +local __TS__Decorate = ____lualib.__TS__Decorate local ____exports = {} function ____exports.__main(self) local function decorator(constructor, context) @@ -12,7 +12,7 @@ function ____exports.__main(self) TestClass.name = "TestClass" function TestClass.prototype.____constructor(self) end - TestClass = __TS__DecorateLegacy({decorator}, TestClass) + TestClass = __TS__Decorate(TestClass, TestClass, {decorator}, {kind = "class", name = "TestClass"}) end return ____exports" `; diff --git a/test/unit/classes/decorators.spec.ts b/test/unit/classes/decorators.spec.ts index 1a880ce24..49a7c314c 100644 --- a/test/unit/classes/decorators.spec.ts +++ b/test/unit/classes/decorators.spec.ts @@ -18,7 +18,10 @@ test("Class decorator with no parameters", () => { public decoratorBool = false; } - return { decoratedClass: new TestClass(), classDecoratorContext }; + return { decoratedClass: new TestClass(), context: { + kind: classDecoratorContext.kind, + name: classDecoratorContext.name, + } }; `.expectToMatchJsResult(); }); @@ -292,21 +295,26 @@ test("class setter decorator", () => { test("class field decorator", () => { util.testFunction` - let fieldDecoratorContext; + let fieldDecoratorContext; - function fieldDecorator(initialValue: number, context: ClassFieldDecoratorContext) { - fieldDecoratorContext = context; + function fieldDecorator(_: undefined, context: ClassFieldDecoratorContext) { + fieldDecoratorContext = context; - return initialValue => initialValue * 12; - } + return (initialValue: number) => initialValue * 12; + } - class TestClass { - @fieldDecorator - public value: number = 22; - } + class TestClass { + @fieldDecorator + public value: number = 22; + } - return { result: new TestClass(), fieldDecoratorContext }; -`.expectToMatchJsResult(); + return { result: new TestClass(), context: { + kind: fieldDecoratorContext.kind, + name: fieldDecoratorContext.name, + private: fieldDecoratorContext.private, + static: fieldDecoratorContext.static, + } }; + `.expectToMatchJsResult(); }); describe("legacy experimentalDecorators", () => { From 664a475b3def235f0cefc9f728c0fb009d3746d2 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Thu, 1 Jun 2023 22:30:38 +0200 Subject: [PATCH 5/7] All decorators legacy and modern work, except for field decorators --- .../visitors/class/members/method.ts | 22 ++++++++++++----- test/unit/classes/decorators.spec.ts | 2 +- test/util.ts | 24 ++++++++++++------- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/transformation/visitors/class/members/method.ts b/src/transformation/visitors/class/members/method.ts index 47eae9877..d7dcf6d9e 100644 --- a/src/transformation/visitors/class/members/method.ts +++ b/src/transformation/visitors/class/members/method.ts @@ -34,12 +34,22 @@ export function transformMethodDeclaration( const methodName = transformMethodName(context, node); const [functionExpression] = transformFunctionToExpression(context, node); - if (ts.getDecorators(node)?.length) { - return lua.createAssignmentStatement( - lua.createTableIndexExpression(methodTable, methodName), - createClassMethodDecoratingExpression(context, node, functionExpression, className), - node - ); + const methodHasDecorators = (ts.getDecorators(node)?.length ?? 0) > 0; + const methodHasParameterDecorators = node.parameters.some(p => (ts.getDecorators(p)?.length ?? 0) > 0); // Legacy decorators + + if (methodHasDecorators || methodHasParameterDecorators) { + if (context.options.experimentalDecorators) { + // Legacy decorator statement + return lua.createExpressionStatement( + createClassMethodDecoratingExpression(context, node, functionExpression, className) + ); + } else { + return lua.createAssignmentStatement( + lua.createTableIndexExpression(methodTable, methodName), + createClassMethodDecoratingExpression(context, node, functionExpression, className), + node + ); + } } else { return lua.createAssignmentStatement( lua.createTableIndexExpression(methodTable, methodName), diff --git a/test/unit/classes/decorators.spec.ts b/test/unit/classes/decorators.spec.ts index 49a7c314c..ad4d1cb1c 100644 --- a/test/unit/classes/decorators.spec.ts +++ b/test/unit/classes/decorators.spec.ts @@ -509,7 +509,7 @@ describe("legacy experimentalDecorators", () => { }); test.each(["return { value: true }", "desc.value = true"])( - "Use decorator to override method value", + "Use decorator to override method value %s", overrideStatement => { util.testFunction` const decorator = (target, key, desc): any => { ${overrideStatement} }; diff --git a/test/util.ts b/test/util.ts index 35eda7168..75b477849 100644 --- a/test/util.ts +++ b/test/util.ts @@ -122,35 +122,35 @@ export abstract class TestBuilder { // TODO: Use testModule in these cases? protected tsHeader = ""; public setTsHeader(tsHeader: string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setTsHeader"); this.tsHeader = tsHeader; return this; } private luaHeader = ""; public setLuaHeader(luaHeader: string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setLuaHeader"); this.luaHeader += luaHeader; return this; } protected jsHeader = ""; public setJsHeader(jsHeader: string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setJsHeader"); this.jsHeader += jsHeader; return this; } protected abstract getLuaCodeWithWrapper(code: string): string; public setLuaFactory(luaFactory: (code: string) => string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setLuaFactory"); this.getLuaCodeWithWrapper = luaFactory; return this; } private semanticCheck = true; public disableSemanticCheck(): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("disableSemanticCheck"); this.semanticCheck = false; return this; } @@ -166,7 +166,7 @@ export abstract class TestBuilder { sourceMap: true, }; public setOptions(options: tstl.CompilerOptions = {}): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setOptions"); Object.assign(this.options, options); return this; } @@ -183,25 +183,31 @@ export abstract class TestBuilder { protected mainFileName = "main.ts"; public setMainFileName(mainFileName: string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setMainFileName"); this.mainFileName = mainFileName; return this; } protected extraFiles: Record = {}; public addExtraFile(fileName: string, code: string): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("addExtraFile"); this.extraFiles[fileName] = normalizeSlashes(code); return this; } private customTransformers?: ts.CustomTransformers; public setCustomTransformers(customTransformers?: ts.CustomTransformers): this { - expect(this.hasProgram).toBe(false); + this.throwIfProgramExists("setCustomTransformers"); this.customTransformers = customTransformers; return this; } + private throwIfProgramExists(name: string) { + if (this.hasProgram) { + throw new Error(`${name}() should not be called after an .expect() or .debug()`); + } + } + // Transpilation and execution public getTsCode(): string { From 2717c34930544757a5195deeb9caa02321c92739 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sat, 3 Jun 2023 19:36:04 +0200 Subject: [PATCH 6/7] Finalize property decorators --- src/transformation/utils/diagnostics.ts | 4 +++ .../visitors/class/decorators.ts | 19 +++++++--- src/transformation/visitors/class/index.ts | 7 ++++ .../visitors/class/members/fields.ts | 10 +----- .../__snapshots__/decorators.spec.ts.snap | 23 ++++++++++++ test/unit/classes/decorators.spec.ts | 36 ++++++++++++++++--- 6 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/transformation/utils/diagnostics.ts b/src/transformation/utils/diagnostics.ts index 8fe4a22f5..137bd15b8 100644 --- a/src/transformation/utils/diagnostics.ts +++ b/src/transformation/utils/diagnostics.ts @@ -168,3 +168,7 @@ export const invalidSpreadInCallExtension = createErrorDiagnosticFactory( export const cannotAssignToNodeOfKind = createErrorDiagnosticFactory( (kind: lua.SyntaxKind) => `Cannot create assignment assigning to a node of type ${lua.SyntaxKind[kind]}.` ); + +export const incompleteFieldDecoratorWarning = createWarningDiagnosticFactory( + "You are using a class field decorator, note that tstl ignores returned value initializers!" +); diff --git a/src/transformation/visitors/class/decorators.ts b/src/transformation/visitors/class/decorators.ts index e249fb87a..11e4b75c7 100644 --- a/src/transformation/visitors/class/decorators.ts +++ b/src/transformation/visitors/class/decorators.ts @@ -1,7 +1,7 @@ import * as ts from "typescript"; import * as lua from "../../../LuaAST"; import { TransformationContext } from "../../context"; -import { decoratorInvalidContext } from "../../utils/diagnostics"; +import { decoratorInvalidContext, incompleteFieldDecoratorWarning } from "../../utils/diagnostics"; import { ContextType, getFunctionContextType } from "../../utils/function-context"; import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib"; import { isNonNull } from "../../../utils"; @@ -107,10 +107,10 @@ export function createClassAccessorDecoratingExpression( export function createClassPropertyDecoratingExpression( context: TransformationContext, property: ts.PropertyDeclaration, - originalValue: lua.Expression, className: lua.Identifier ): lua.Expression { - const propertyDecorators = ts.getDecorators(property)?.map(d => transformDecoratorExpression(context, d)) ?? []; + const decorators = ts.getDecorators(property) ?? []; + const propertyDecorators = decorators.map(d => transformDecoratorExpression(context, d)); // If experimentalDecorators flag is set, decorate with legacy decorator logic if (context.options.experimentalDecorators) { @@ -127,7 +127,18 @@ export function createClassPropertyDecoratingExpression( } // Else: TypeScript 5.0 decorator - return createDecoratingExpression(context, className, originalValue, propertyDecorators, { + + // Add a diagnostic when something is returned from a field decorator + for (const decorator of decorators) { + const signature = context.checker.getResolvedSignature(decorator); + const decoratorReturnType = signature?.getReturnType(); + // If return type of decorator is NOT void + if (decoratorReturnType && (decoratorReturnType.flags & ts.TypeFlags.Void) === 0) { + context.diagnostics.push(incompleteFieldDecoratorWarning(property)); + } + } + + return createDecoratingExpression(context, className, lua.createNilLiteral(), propertyDecorators, { kind: lua.createStringLiteral("field"), name: lua.createStringLiteral(property.name.getText()), private: lua.createBooleanLiteral(isPrivateNode(property)), diff --git a/src/transformation/visitors/class/index.ts b/src/transformation/visitors/class/index.ts index 1ca760e32..e19e2ccc5 100644 --- a/src/transformation/visitors/class/index.ts +++ b/src/transformation/visitors/class/index.ts @@ -20,6 +20,7 @@ import { getExtendedNode, getExtendedType, isStaticNode } from "./utils"; import { createClassSetup } from "./setup"; import { LuaTarget } from "../../../CompilerOptions"; import { transformInPrecedingStatementScope } from "../../utils/preceding-statements"; +import { createClassPropertyDecoratingExpression } from "./decorators"; export const transformClassDeclaration: FunctionVisitor = (declaration, context) => { // If declaration is a default export, transform to export variable assignment instead @@ -179,6 +180,12 @@ function transformClassLikeDeclaration( const statement = transformStaticPropertyDeclaration(context, member, localClassName); if (statement) result.push(statement); } + + if (ts.getDecorators(member)?.length) { + result.push( + lua.createExpressionStatement(createClassPropertyDecoratingExpression(context, member, className)) + ); + } } } diff --git a/src/transformation/visitors/class/members/fields.ts b/src/transformation/visitors/class/members/fields.ts index 1e623c381..2308eaac8 100644 --- a/src/transformation/visitors/class/members/fields.ts +++ b/src/transformation/visitors/class/members/fields.ts @@ -4,7 +4,6 @@ import { TransformationContext } from "../../../context"; import { createSelfIdentifier } from "../../../utils/lua-ast"; import { transformInPrecedingStatementScope } from "../../../utils/preceding-statements"; import { transformPropertyName } from "../../literal"; -import { createClassPropertyDecoratingExpression } from "../decorators"; export function transformClassInstanceFields( context: TransformationContext, @@ -44,12 +43,5 @@ export function transformStaticPropertyDeclaration( const value = context.transformExpression(field.initializer); const classField = lua.createTableIndexExpression(lua.cloneIdentifier(className), fieldName); - if (ts.getDecorators(field)?.length) { - return lua.createAssignmentStatement( - classField, - createClassPropertyDecoratingExpression(context, field, value, className) - ); - } else { - return lua.createAssignmentStatement(classField, value); - } + return lua.createAssignmentStatement(classField, value); } diff --git a/test/unit/classes/__snapshots__/decorators.spec.ts.snap b/test/unit/classes/__snapshots__/decorators.spec.ts.snap index 027441192..681e4fa3d 100644 --- a/test/unit/classes/__snapshots__/decorators.spec.ts.snap +++ b/test/unit/classes/__snapshots__/decorators.spec.ts.snap @@ -19,6 +19,29 @@ return ____exports" exports[`Throws error if decorator function has void context: diagnostics 1`] = `"main.ts(4,9): error TSTL: Decorator function cannot have 'this: void'."`; +exports[`class field decorator warns the return value is ignored: code 1`] = ` +"local ____lualib = require("lualib_bundle") +local __TS__Class = ____lualib.__TS__Class +local __TS__Decorate = ____lualib.__TS__Decorate +local ____exports = {} +function ____exports.__main(self) + local fieldDecoratorContext + local function fieldDecorator(self, _, context) + fieldDecoratorContext = context + return function(____, initialValue) return initialValue * 12 end + end + local TestClass = __TS__Class() + TestClass.name = "TestClass" + function TestClass.prototype.____constructor(self) + self.value = 22 + end + __TS__Decorate(TestClass, nil, {fieldDecorator}, {kind = "field", name = "value", private = false, static = false}) +end +return ____exports" +`; + +exports[`class field decorator warns the return value is ignored: diagnostics 1`] = `"main.ts(11,13): warning TSTL: You are using a class field decorator, note that tstl ignores returned value initializers!"`; + exports[`legacy experimentalDecorators Throws error if decorator function has void context: code 1`] = ` "local ____lualib = require("lualib_bundle") local __TS__Class = ____lualib.__TS__Class diff --git a/test/unit/classes/decorators.spec.ts b/test/unit/classes/decorators.spec.ts index ad4d1cb1c..d7399f058 100644 --- a/test/unit/classes/decorators.spec.ts +++ b/test/unit/classes/decorators.spec.ts @@ -1,4 +1,7 @@ -import { decoratorInvalidContext } from "../../../src/transformation/utils/diagnostics"; +import { + decoratorInvalidContext, + incompleteFieldDecoratorWarning, +} from "../../../src/transformation/utils/diagnostics"; import * as util from "../../util"; test("Class decorator with no parameters", () => { @@ -299,8 +302,6 @@ test("class field decorator", () => { function fieldDecorator(_: undefined, context: ClassFieldDecoratorContext) { fieldDecoratorContext = context; - - return (initialValue: number) => initialValue * 12; } class TestClass { @@ -314,7 +315,34 @@ test("class field decorator", () => { private: fieldDecoratorContext.private, static: fieldDecoratorContext.static, } }; - `.expectToMatchJsResult(); + `.expectToEqual({ + result: { + value: 22, // Different from JS because we ignore the value initializer + }, + context: { + kind: "field", + name: "value", + private: false, + static: false, + }, + }); +}); + +test("class field decorator warns the return value is ignored", () => { + util.testFunction` + let fieldDecoratorContext; + + function fieldDecorator(_: undefined, context: ClassFieldDecoratorContext) { + fieldDecoratorContext = context; + + return (initialValue: number) => initialValue * 12; + } + + class TestClass { + @fieldDecorator + public value: number = 22; + } + `.expectDiagnosticsToMatchSnapshot([incompleteFieldDecoratorWarning.code]); }); describe("legacy experimentalDecorators", () => { From ea94d24fa220af4a5ff821e79e9c58be990a96a3 Mon Sep 17 00:00:00 2001 From: Perryvw Date: Sun, 4 Jun 2023 16:58:09 +0200 Subject: [PATCH 7/7] Fix unit tests --- test/unit/classes/classes.spec.ts | 4 ++-- test/unit/identifiers.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/classes/classes.spec.ts b/test/unit/classes/classes.spec.ts index f9ab4fe61..2e447ab54 100644 --- a/test/unit/classes/classes.spec.ts +++ b/test/unit/classes/classes.spec.ts @@ -749,9 +749,9 @@ test("default exported anonymous class has 'default' name property", () => { }); // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/584 -test("constructor class name available with constructor", () => { +test("constructor class name available with decorator", () => { util.testModule` - const decorator = any>(constructor: T) => class extends constructor {}; + const decorator = any>(constructor: T, context: ClassDecoratorContext) => class extends constructor {}; @decorator class MyClass {} diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index e3acf8530..6e13a5e12 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -196,7 +196,7 @@ test.each(validTsInvalidLuaNames)("class with invalid lua name has correct name test.each(validTsInvalidLuaNames)("decorated class with invalid lua name", name => { util.testFunction` - function decorator any>(Class: T): T { + function decorator any>(Class: T, context: ClassDecoratorContext): T { return class extends Class { public bar = "foobar"; }; @@ -210,7 +210,7 @@ test.each(validTsInvalidLuaNames)("decorated class with invalid lua name", name test.each(validTsInvalidLuaNames)("exported decorated class with invalid lua name", name => { util.testModule` - function decorator any>(Class: T): T { + function decorator any>(Class: T, context: ClassDecoratorContext): T { return class extends Class { public bar = "foobar"; };