Feature request: syntax for forward declaration of colonmethods

[JR-Mitchell]

Request

Introduce a syntax allowing the forward declaration of colon methods in records.

For example, a possible syntax could be:

record Point
    x: integer
    y: integer
    distance: method(other: Point): number
end

such that calling myPoint:distance(otherPoint) (where myPoint and otherPoint are of type Point) compiles successfully, but myPoint.distance(otherPoint) throws the error "invoked method as a regular function: use ':' instead of '.'".

Premise

When declaring a custom type in code:

record Point
    x: integer
    y: integer
end

you can, within scope, declare methods with the colon operator:

function Point:distance(other: Point): number
    local xDistance = self.x - other.x
    local yDistance = self.y - other.y
    return math.sqrt(xDistance*xDistance + yDistance*yDistance)
end

and then, at call sites, teal will throw an error if you call with a dot as opposed to colon:

local distance = myPoint.distance(otherPoint) -- invoked method as a regular function: use ':' instead of '.'

This is a really handy feature to have.

However, if you need to declare a record that then has its method definition set in a different scope, or if you need to declare a type using a .d.ts file, the only way to do this (as far as I am aware) is

record Point
    x: integer
    y: integer
    distance: function(self: Point, other: Point): number
end

And then, if you accidentally write myPoint.distance(otherPoint) rather than myPoint:distance(otherPoint), it will not throw an error at compile time, but instead at run time, when trying to access other.y on a nil argument.

It would be useful to allow forward declaration of such methods with some alternative syntax which would allow the compiler to determine if a method intended for calling using colon syntax was instead called with dot syntax.

[hishamhm]

I think the addition of interfaces will require us to introduce a self type. I believe that should address this issue (by making any function whose first argument is of type self become a method).

[hishamhm]

(for backwards-compatibility, we could even extend this behavior to functions whose first argument is the record itself and the argument name is self... self is already a special name in the language, anyway)

[JR-Mitchell]

Thanks for the speedy response @hishamhm :) Is there a timeline for the introduction of interfaces?

(for backwards-compatibility, we could even extend this behavior to functions whose first argument is the record itself and the argument name is self... self is already a special name in the language, anyway)

From my understanding, this could be implemented before interfaces are. Is it worth me trying to make a PR that implements this behaviour?

[hishamhm]

Is there a timeline for the introduction of interfaces?

No timeline, sorry, but it's high in my priority list.

From my understanding, this could be implemented before interfaces are.

Now that you say it, yes!

Is it worth me trying to make a PR that implements this behaviour?

Sure! The code can be a bit confusing at first, but the things to look for are the is_method attribute and parse_function_type.

[JR-Mitchell]

Hi again @hishamhm, in writing logic and test cases for this feature, I found something else that seems like it might be a bug, but just wanted to check with you whether it is intended behaviour or not.

The code:

local record Foo
end
function Foo:first_method(argument: string)
end
function Foo:second_method()
    self.first_method("hello")
end

Will throw the error invoked method as a regular function: use ':' instead of '.'

However, if the type of the argument is the same as the receiver type, for example:

local record Foo
end
function Foo:first_method(argument: Foo)
end
function Foo:second_method()
    local argument: Foo = {}
    self.first_method(argument)
end

This error won't be thrown, and it will compile successfully (presumably because the compiler recognises the passed argument to be of the correct type).

Is this the intended behaviour, or should this second case throw the same error invoked method as a regular function: use ':' instead of '.' (or at least a similar warning)?

[JR-Mitchell]

I've opened a separate PR (#640) for the above issue, as it is related but separate behaviour.

[overcrook]

IMHO, the current behavior can be useful where we want to call the record method as a "static classmethod". Given the fact that all "record instances" are just ordinary tables with a corresponding metatable inserted, and all record functions are placed in the shared record, in some cases we can use the record object itself.

Consider the following:

local record M
    val: integer
end

function M:add(other: M): integer
    if not other then return self.val end

    return self.val + other.val
end

function M.new(value: integer): M
    return setmetatable({val = value}, {__index = M})
end

local a:M = M.new(2)
local b:M = M.new(3)

print(a:add(b))
print(M.add(a, b))
print(M.add(b))

The first 2 print calls are equivalent in terms of Lua.

With the third line, things get interesting. All of the following assumptions may be correct:

1. This is a static method call with the last parameter omitted, which is the correct form in this case.
2. This is a method call, and in this case it is an error.

I think Teal does not complain about the third line due to the assumption that it MAY be correct, AND there is currently no way to mark function parameters as nullable or non-nullable.

Can Teal distinguish between record objects and record instances? In this case, perhaps we should allow methods to be called like regular function, but only if they are used from within the record object itself?

[JR-Mitchell]

That's a good point. M.add(b) is equivalent in syntax to b:add(), but in this case I think there could be situations where the former syntax is clearer. I will update #640 with a test case for this situation (call as a function on the record itself) and see if I can find a way to suppress the warning for this particular case.

[hishamhm]

I think Teal does not complain about the third line due to the assumption that it MAY be correct, AND there is currently no way to mark function parameters as nullable or non-nullable.

That's exactly right (including the emphasis on AND :) ) — with non-nullable arguments the arity itself would give us a more reliable hint on whether the use of . was a mistake or not.

[hishamhm]

Can Teal distinguish between record objects and record instances?

Internally, yes. A record instance is of type record, a record declaration is of type typetype, whose typetype.def is a record.

For using a declaration as an instance (a common Lua pattern for getting the equivalent of Java's static), there are a number of automatic coercions in place, so the typetype just behaves like a record.

So, to the outside view they're pretty much equivalent, except for things like: you can only declare new methods (function MyRecord:my_method()) on the typetype itself, and in the same scope where you declared it (this is so that a record definition can be known statically after a module is typechecked, ensuring that no other modules will add extra methods after the fact).

In this case, perhaps we should allow methods to be called like regular function, but only if they are used from within the record object itself?

Not sure, because for functions that are intended to be used as "class methods" a la Java static (such as M.new above), those are usually declared with . rather than : anyway (so they don't have the is_method bit set and don't get this check, they're just "functions in a record").

[JR-Mitchell]

Not sure, because for functions that are intended to be used as "class methods" a la Java static (such as M.new above), those are usually declared with . rather than : anyway (so they don't have the is_method bit set and don't get this check, they're just "functions in a record").

This is a good point, and additionally calling a function like this can still be done with colon syntax - for example

local record M
end
function M.add(first: M, second: M)
    -- some logic here
end
local function do_things(a: M, b: M)
    print(a:add(b))
    print(M.add(a, b))
    print(M.add(b))
end

will throw no warnings.

However, programmers might either:

• Not know that the warning can be suppressed by using . rather than : in the declaration
• Not want to make this syntax change (for whatever reason - maybe they mostly use it as a method and only occasionally as a static function)

Also, once we introduce forward declarations with self: Parent as the first argument, the only way for programmers to suppress this would be to rename the first argument to something else (e.g first, as in my example above).

So, I still do think it might be worth making an exception from the warning if the receiver is the record type itself.

[JR-Mitchell]

https://github.com/teal-language/tl/pull/640/commits/7668e030f63d5a97373f547e7398e560a9a9462d suppresses the warning if the method is called directly from the typetype (e.g M.add(b)). I've added a test case for it, and it's passing all tests, but I'm not fully confident that there aren't any edge cases that it doesn't account for. Let me know what you think :)

[hishamhm]

For future reference: #640 says "closed" above but I merged it manually!

[JR-Mitchell]

Hi @hishamhm, I was wondering if you could advise on the implementation of this behaviour for self indicating that a function in a record is a method.

I've been able to (relatively easily) set up behaviour that checks that the parameter name is self. However, at the time that a function type within a record is being parsed, the record type itself hasn't been fully parsed, and so I've been struggling to check whether this parameter type is the record itself - and this becomes even tricker if it is a record with a generic type parameter, or a nested record.

The only possible approach I've thought of is adding an additional step after a top-level record has been parsed which iterates through all fields of the method which are either functions or nested records, and removes is_method for any functions with the first parameter named self but which don't match the type name and generics of the record that they are in.

But this seems like there might be a more elegant approach - I was wondering if you could think of a better place to do this first parameter type checking?

[hishamhm]

I see what you mean. From a quick glance at the code, I think one way to keep it single-pass is to trickle down the type name as an extra argument, all the way from parse_type_declaration, through parse_newtype then into parse_record_body. Then you'd have the record name available when you're about to store_field_in_record.

Off the top of my head, I don't think anything special would need to be done for nested records, since these trigger a new parse_newtype and you would then pass the nested record's name, and a first argument would only count as a "method self" if it's the same one from the nesting level you're currently on.

Does that sound reasonable?

[JR-Mitchell]

That sounds alright to me, I'll give it a go.

I feel like I might encounter an issue with records with generics - for example, with the code:

record MyRecord<T>
    value: T,
    add: function(self: MyRecord<integer>, arg: integer)
end

local int_record: MyRecord<integer> = {}
local str_record: MyRecord<string> = {}

int_record.add(12)
str_record.add(21)

the warnings/errors might not work how they should.

However, that's something I can test (and try to fix) once I've got the basic behaviour working.

[hishamhm]

@JR-Mitchell To deal with generics, within parse_record_body you have def.typeargs available. If your function's self type has typ.typevals, you can check one against the other.

[JR-Mitchell]

@hishamhm can this issue be closed now?