Lua table by hazzard993 · Pull Request #516 · TypeScriptToLua/TypeScriptToLua

hazzard993 · 2019-04-10T11:50:45Z

@LuaTable can be used to mark a Map-like object to transpile down to a simple Lua table.

/** @LuaTable */
declare class Table<K extends {} = {}, V = any> {
    readonly length: number;
    set(key: K, value: V): void;
    get(key: K): V;
}

const tbl = new Table(); // local tbl = {}

const foo = {};
tbl.set(foo, "bar"); // tbl[foo] = "bar"
print(tbl.get(foo)); // print(tbl[foo])

tbl.set(1, "baz"); // tbl[1] = "baz"
print(tbl.length); // print(#tbl)

get, set and length are the only properties the transpiler pays special attention to. Other properties are treated the same as any other property.

I did make a decision to disallow tbl.set(...) to be used in place of an expression since it never returns a value and transpiles to an ExpressionStatement, not an Expression.

This could help with reducing the amount of Lua outputted through its use in LuaLibs and other source code.

tomblind

Need to make sure other classes don't extend LuaTables
Need to make sure non-declared classes don't use this as it results in unexpected behavior

src/LuaTransformer.ts

hazzard993 · 2019-04-14T02:06:24Z

If a class is declared with the LuaTable directive without declare modifier the transpiler will error
Classes extending a LuaTable will cause the transpiler to error
set() takes two parameters, get() takes one

Also made the length property not re-assignable to avoid extra code gen

tomblind · 2019-04-14T20:35:00Z

One more issue I see is explicit constructors. I can declare a constructor for the class, but it's basically ignored. We should throw an error or do something with the arguments.

Also, it would be good to have tests for the directive when used on an interface instead of a class.

As for assigning length, we now support that for array types with a lib function __TS_ArraySetLength. This could be used here as well.

hazzard993 · 2019-04-15T12:53:34Z

Now if a LuaTable is constructed with arguments the transpiler will throw an error saying no parameters are allowed.

I chose not to use __TS_ArraySetLength since it seems like the goal of a LuaTable object is to reduce code gen.

test/unit/luaTable.spec.ts

tomblind

This is looking good - but I thought of one more edge-case 😄 We should probably dis-allow using instanceof on @luaTable objects since it will give incorrect results.

Perryvw

Missing functional test(s) for table get/set.

src/LuaTransformer.ts

Perryvw · 2019-04-26T19:46:08Z

src/LuaTransformer.ts

+                        "One parameter is required for get() on a '@LuaTable' object."
+                    );
+                }
+                if (isWithinExpressionStatement) {


I don't think I understand the reasoning here. If you have a table.get(abc) statement it is transformed to ____ = table.get(abc)? That sounds like asking for all kinds of strange bugs.

Since table.get(abc) transforms to table[abc] it becomes an invalid expression in Lua. In this case I chose to then assign this to an anonymous variable the same way x; is transformed to local ____ = x;

This sounds like a more fundamental issue. Table.get(abc) is an expression but you are returning a statement. This is related to my other comment on the function signature, I think it might automatically be resolved if you separate the assignment and expression functions.

I've separated this logic into three functions atm to keep this logic more consistent. I've grouped the error checking in validateLuaTableCall which takes isWithinExpressionStatement for validation purposes, transformLuaTableExpressionStatement and transformLuaTableExpression.

Would this be something more appropriate?

Seems okay, these methods should probably be private instead of public though.

src/TSTLErrors.ts

ark120202 · 2019-05-02T02:17:39Z

src/LuaTransformer.ts

+        }
+
+        // LuaTable classes must be declared
+        if (decorators.has(DecoratorKind.LuaTable) && !tsHelper.isDeclared(statement)) {


Class may considered declared because of it's context

Suggested change

if (decorators.has(DecoratorKind.LuaTable) && !tsHelper.isDeclared(statement)) {

if (decorators.has(DecoratorKind.LuaTable) && (ts.getCombinedModifierFlags(statement) & ts.ModifierFlags.Ambient) === 0) {

Not sure how to test this but good pickup

ark120202 · 2019-05-02T02:17:55Z

src/LuaTransformer.ts

+
+        switch (methodName) {
+            case "get":
+                if (expression.arguments.length !== 1) {


It also should check that it's argument isn't a SpreadElement

ark120202 · 2019-05-02T02:18:32Z

src/LuaTransformer.ts

+                if (expression.arguments.length !== 1) {
+                    throw TSTLErrors.ForbiddenLuaTableUseException(
+                        expression,
+                        "One parameter is required for get() on a '@LuaTable' object."


Suggested change

"One parameter is required for get() on a '@LuaTable' object."

"One parameter is required for get() on a '@luaTable' object."

ark120202 · 2019-05-02T02:18:34Z

src/LuaTransformer.ts

+                if (params.length !== 2) {
+                    throw TSTLErrors.ForbiddenLuaTableUseException(
+                        expression,
+                        "Two parameters are required for set() on a '@LuaTable' object."


Suggested change

"Two parameters are required for set() on a '@LuaTable' object."

"Two parameters are required for set() on a '@luaTable' object."

ark120202 · 2019-05-02T02:18:37Z

src/TSTLErrors.ts

        new TranspileError(`Iterating over arrays with 'for ... in' is not allowed.`, node);

+    public static ForbiddenLuaTableSetExpression = (node: ts.Node) => new TranspileError(
+        `A '@LuaTable' object's 'set()' method can only be used as a Statement, not an Expression.`,


Suggested change

`A '@LuaTable' object's 'set()' method can only be used as a Statement, not an Expression.`,

`A '@luaTable' object's 'set()' method can only be used as a Statement, not an Expression.`,

src/LuaTransformer.ts

test/util.ts

hazzard993 added 7 commits April 10, 2019 07:39

LuaTable draft

e8aff2b

Refactor and added tests

59d25a7

Fixed tripped watch file

174f191

Missing semicolon

600616f

Prettified

aea50a8

Merge remote-tracking branch 'upstream/master' into lua-table

ce82a03

Merge remote-tracking branch 'upstream/master' into lua-table

9a55666

tomblind requested changes Apr 13, 2019

View reviewed changes

src/LuaTransformer.ts Outdated Show resolved Hide resolved

src/LuaTransformer.ts Outdated Show resolved Hide resolved

hazzard993 added 7 commits April 14, 2019 07:11

Merge remote-tracking branch 'upstream/master' into lua-table

94c8217

Cannot extend LuaTable classes

fe81a01

luaTable classes can only be declared

65fea6f

Restricted LuaTable parameter length

2d0300c

string -> LuaTable correction

6848f11

LuaTable length cannot be re-assigned

213a0a4

Prettier

152bf96

hazzard993 added 3 commits April 15, 2019 19:17

Merge remote-tracking branch 'upstream/master' into lua-table

de40026

LuaTable interface tests

6e849e1

LuaTables cannot be constructed with arguments

cc81891

tomblind requested changes Apr 17, 2019

View reviewed changes

test/unit/luaTable.spec.ts Show resolved Hide resolved

test/unit/luaTable.spec.ts Outdated Show resolved Hide resolved

hazzard993 added 2 commits April 18, 2019 13:15

Merge remote-tracking branch 'upstream/master' into lua-table

c51c7df

Test splitting and more interface tests

eb15d5c

tomblind reviewed Apr 23, 2019

View reviewed changes

instanceof LuaTable not allowed

54e4aac

tomblind approved these changes Apr 24, 2019

View reviewed changes

Perryvw requested changes Apr 26, 2019

View reviewed changes

Purpose in life for exception

32ad53d

ark120202 reviewed May 2, 2019

View reviewed changes

hazzard993 added 9 commits May 4, 2019 10:19

No more other methods

781fd95

Broke down transformLuaTableCallExpression into other methods

ed5c4c5

LuaTable -> luaTable

3f9de1d

Merge remote-tracking branch 'upstream/master' into lua-table

0762631

luaTables must be within an ambient context

49ff675

Added isAmbient and disallowed argument spreading

8691eee

Unsupported method consistency

17f1f13

tsHelper.isDeclared is unused

f3d832b

public -> private

b22c364

Perryvw reviewed May 4, 2019

View reviewed changes

src/LuaTransformer.ts Outdated Show resolved Hide resolved

hazzard993 added 3 commits May 4, 2019 20:04

Missed one more public

baca079

Allowed transpileAndExecute to use ambient code

b75f0ef

luaTable functional tests

0756141

Perryvw reviewed May 4, 2019

View reviewed changes

test/util.ts Outdated Show resolved Hide resolved

hazzard993 added 2 commits May 4, 2019 22:36

transpileString signature change

2a48670

Revert util.ts changes

a2dd7ab

Perryvw approved these changes May 4, 2019

View reviewed changes

Perryvw merged commit 3de430d into TypeScriptToLua:master May 4, 2019

hazzard993 deleted the lua-table branch May 4, 2019 13:10

lolleko mentioned this pull request Mar 6, 2021

Explore using custom types instead of tuple and iterator decorators #580

Closed

	if (decorators.has(DecoratorKind.LuaTable) && !tsHelper.isDeclared(statement)) {
	if (decorators.has(DecoratorKind.LuaTable) && (ts.getCombinedModifierFlags(statement) & ts.ModifierFlags.Ambient) === 0) {

	"One parameter is required for get() on a '@LuaTable' object."
	"One parameter is required for get() on a '@luaTable' object."

	"Two parameters are required for set() on a '@LuaTable' object."
	"Two parameters are required for set() on a '@luaTable' object."

	`A '@LuaTable' object's 'set()' method can only be used as a Statement, not an Expression.`,
	`A '@luaTable' object's 'set()' method can only be used as a Statement, not an Expression.`,

Uh oh!

Conversation

hazzard993 commented Apr 10, 2019

Uh oh!

tomblind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hazzard993 commented Apr 14, 2019

Uh oh!

tomblind commented Apr 14, 2019

Uh oh!

hazzard993 commented Apr 15, 2019

Uh oh!

Uh oh!

Uh oh!

tomblind left a comment

Choose a reason for hiding this comment

Uh oh!

Perryvw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants