lua-language-server icon indicating copy to clipboard operation
lua-language-server copied to clipboard

Function Overloading Overhaul (`@function` annotation)

Open carsakiller opened this issue 2 years ago • 16 comments

The Problem

I think function overloading needs some changes in order for it to really function in the way that most people would find useful. This is especially problematic with event systems, as I and others have encountered.

Example

Currently, let's say I have the following function that I want to provide an overload for:

function Shape:listen(event, callback) end

Using @overload

The first logical option is to use @overload:

---@class Shape
local Shape = {}

---Subscribe to an event on this shape
---@param event "Destroy"
---@param callback fun(self: Shape)
---@overload fun(event: "Repair", callback: fun(self: Shape, amount: number))
function Shape:listen(event, callback) end

But there is a problem, when using methods (:), the first parameter only gets completions for the first @param and ignores the @overload entirely. image Ok, so for testing, let's replace the method (:) with a static function (.):

---@class Shape
local Shape = {}

---Subscribe to an event on this shape
---@param event "Destroy"
---@param callback fun(self: Shape)
---@overload fun(event: "Repair", callback: fun(self: Shape, amount: number))
function Shape.listen(event, callback) end

This still isn't great, we are still offered both callbacks even though the info we have entered only matches the @overload. At least the first parameter was completed this time. image

Multiple Definitions

So then maybe we try defining multiple functions where each event param has the type set to the event name we are looking for:

---@class Shape
local Shape = {}

---Subscribe to an event on this shape
---@param event "Destroy"
---@param callback fun(self: Shape)
function Shape:listen(event, callback) end

---Subscribe to an event on this shape
---@param event "Repair"
---@param callback fun(self: Shape, amount: number)
function Shape:listen(event, callback) end

Now, even as methods (:) we are receiving correct completions for the first parameter... nice! However, we are still receiving two completions for the callback - there is no narrowing happening. The completion also shows the event as "Destroy", which is incorrect for our second definition as we have only allowed "Repair". image At least when defining the function twice, we are able to write a separate description for each of them as well as their @params and @returns. However, we receive a warning saying that we have a duplicate field.

Proposed Solution

See @flrgh's idea to add a @function annotation to add more in-depth support for defining functions overall.

carsakiller avatar Aug 09 '22 15:08 carsakiller

This would also fix another issue I'm having where classes that have both a static and method variant of the same function aren't narrowing, eg:

---@meta

---@class Foo
---@overload fun(): Foo
Foo = {}

---@param eventName string
---@param callback function
function Foo.Subscribe(eventName, callback) end

---@param eventName string
---@param callback function
function Foo:Subscribe(eventName, callback) end

...

local inst = Foo()
inst:Subscribe(" --> Provides autocompletion for the static method, treating the string as the callback

Derpius avatar Aug 09 '22 17:08 Derpius

Yeah, I don't know of a way where you can change the callback param given a certain first parameter, which is a very common use-case for an event system or asynchronous code. Hopefully changing how overloads work and improving the narrowing makes it possible 🙂

carsakiller avatar Aug 09 '22 18:08 carsakiller

This problem has been partially solved. However, the inconvenient thing is that it is impossible to display comments for each event separately.

---Registers a callback function for an event.\
---`function(client[, topic[, message]])`. The first parameter is always the client object itself.\
---Any remaining parameters passed differ by event:
--- @class Emit
--- @field on fun(self, event: string, cb: function)
--- @field on fun(self, event: '"connect"',  cb: fun(client))
--- @field on fun(self, event: '"connfail"', cb: fun(client, reason: integer))  #If the event is `"connfail"`, the 2nd parameter will be the connection failure code.
--- @field on fun(self, event: '"message"',  cb: fun(client, topic: string, message: string)) @If event is `"message"`, the 2nd and 3rd parameters are received topic and message, respectively, as Lua strings.
local emit = {}

emit:on("message", function (client, topic, message) end)

or

---comment**********
---@class EmitL
---@field listen fun(self, eventName: string, cb: function)
--- comment for e1
---@field listen fun(self, eventName: '"e1"', cb: fun(i:integer)):table
---@field listen fun( eventName: '"e2"', cb: fun(s:string))  @comment for e2
---@field listen fun(self, eventName: '"e3"', cb: fun(i:integer, s:string))  @comment for e3
local emitL = {}


emitL:listen("e1", function (i) end)
emitL.listen("e2", function (s) end)

serg3295 avatar Aug 10 '22 09:08 serg3295

Poking my head in to add ideas here (read: personal wish list features :wink:). I think a good solution/step would be to support functions as first-class objects alongside classes.

The issue with function aliases (fun(...))

The fun(...):... syntax is so terse/dense that it becomes cumbersome to use for all but the most simple cases. Also, if you're properly namespacing your custom types in order to not pollute the global ns, you wind up with reeeeeally long lines:

--- This is my function. If you pass it a callback, it does something different.
---
---@param foo string
---@return integer
---@overload fun(foo: string, callback: libraryname.functionname.callback):libraryname.functionname.result
local function do_the_thing(foo) end

---@alias libraryname.functionname.callback fun(result: string|nil, error: string|nil):boolean

An alternative: standalone function definitions

This would make things a lot more flexible.

The @function label

Example:

--- This is an optional callback.
--- Because it has it's own block (instead of `fun(...)`), I can write doc strings for everything (yay!).
---
--- The `@function` label comes first here for consistency with `@class`
---
---@function libraryname.functionname.callback
---@param result? string  # result text (`nil` if there was an error)
---@param error?  string  # an error message (always `nil` when `result` is not `nil`)
---@return boolean success

--- This is my function.
---
---@param foo string
---@return integer
local function do_the_thing(foo) end

--- This is also my function, but when passed a callback.
---
---@function libraryname.functionname-with-callback
---@param foo string
---@param callback libraryname.functionname.callback
---@return libraryname.functionname.result

Reusing @class

Alternatively, the @class label/decorator/keyword could be overloaded to support function annotations:

--- This is an optional callback.
--- Because it has it's own block (instead of `fun(...)`), I can write doc strings for everything (yay!).
---
---@class libraryname.functionname.callback : function
---@param result? string  # result text (`nil` if there was an error)
---@param error?  string  # an error message (always `nil` when `result` is not `nil`)
---@return boolean success

Applying the overload

Now, to apply libraryname.functionname-with-callback as an overload of do_the_thing, we have options...

Overload at the target by passing a type name to @overload

--- This is my function.
---
---@param foo string
---@return integer
---@overload libraryname.functionname-with-callback
local function do_the_thing(foo) en

Annotate at the source with a new @overloads label

--- This is also my function, but when passed a callback.
---
---@function libraryname.functionname-with-callback
---@param foo string
---@param callback libraryname.functionname.callback
---@return libraryname.functionname.result
---@overloads `do_the_thing` -- resolve "do_the_thing" as a lua identifier in local scope

If we allow @function to be applied to actual code, then we can take it a step further and allow overloading from different sources/files, which would be nice for anyone working with framework-y code

  • libraryname.lua
--- This is my function.
---
---@function libraryname.functionname
---@param foo string
---@return integer
local function do_the_thing(foo) en
  • other.lua
--- This is also my function, but when passed a callback.
---
---@function libraryname.functionname-with-callback
---@param foo string
---@param callback libraryname.functionname.callback
---@return libraryname.functionname.result
---
---@overloads libraryname.functionname -- resolve "libraryname.functionname" as a fully-qualified type name

The benefit of @function and @overloads is that now it's possible to create function annotations without having to render actual code, which would be a big win IMO.

flrgh avatar Aug 10 '22 16:08 flrgh

I think adding a @function annotation would be a nice replacement for the fun(...):... syntax. I too dislike the current way of defining functions because of all the information that is missing and because they have to be one line.

@overload could also be "replaced" with the functionality you explain with@overloads which refers to a @function or normal Lua function. For this to really work though, we would probably need to namespace annotations, like @flrgh said. It would be nice if this could be handled automatically so that an annotation on a function myFunction in myLibrary.lua could be referred to by using myLibrary.myFunction.

carsakiller avatar Aug 10 '22 16:08 carsakiller

Here is another issue I am experiencing with @overload, the returns are not narrowing and it results in a union type... no matter how I try to define it:

---@param event "A"
---@param callback fun()
---@return boolean
---@overload fun(event: "B", callback: fun()): string
local function listen(event, callback) end

local result = listen("A", function() end)
local result = listen("B", function() end)



---@overload fun(event: "A", callback: fun()): boolean
---@overload fun(event: "B", callback: fun()): string
local function listen2(event) end

local result = listen2("A", function() end)
local result = listen2("B", function() end)

result is boolean|string in all of these cases

carsakiller avatar Sep 13 '22 04:09 carsakiller

One other thing with this is that if you're trying to overload something that already exists, it will give you both definitions, which is not ideal (observed by @carsakiller and myself).

On top of that, if you try to nil out an existing function (which perhaps isn't really supported because why would you do it?) doesn't do anything. I'd like to be able to remove an existing documented function that's in the LÖVE documents, but doesn't exist for LÖVE Potion.

See #1834 for the discussion about this.

TurtleP avatar Jan 24 '23 15:01 TurtleP

  1. It's enough to introduce @function tag which would define a new instance function type that can be referenced by @type
  2. @overload per doc isn't intended to be used for function declaration, it's OOP concept for already declared funcs
  3. TypeScript is bad in most of the cases; don't bring anything from that language; you may learn how some things are done, but let's not borrow anything blindly; the TS has too many pitfalls and syntax allows you to write very unoptimized, completely dumb code.

hinell avatar Nov 23 '23 19:11 hinell