Typehinting luasnip

[michaeldebetaz]

trafficstars

See #1110

As Luasnip uses lazy loading in the code logic, the type hinting chain breaks before getting to the user. This is the first step towards a better type hinted Luasnip, leveraging the lua-ls LSP server and its use of the annotations (e. g. ---@params name string).

I don't see another way to ensure type hinting than to cast a type to the lazy table at the end of lua/luasnip/init.lua. But I am sure we can still improve the type hiniting from functions that are using too ofter an any type. Typically for the opts or for the snippets.

This first try put the type hints a bit all over the place, so the author can better understand to what part of the code they are linked. It would be advisable to put the ---@alias in a types.lua file in the future, so that the type hints don't pollute too much of the code.

I apologize if it is sometimes messy, I wanted to do this first iteration quickly before I get lost into the type hinting madness ^^ I am of course available if you have questions or if I can help in any way (I'm sure you will 😅).

[L3MON4D3]

Hey @michaeldebetaz, really happy about the quick PR and you tackling this at all, a big Thank You! already for that :D I'm a bit busy this week and the next and don't have much time to spend on luasnip, but I'll check your changes as soon as I have free time on my hands :)

[michaeldebetaz]

Thank you for your kind words and thank you for helping me getting started.

Take your time of course, there is no rush.

If my code is too messy (I rushed it sometimes I think), we can speak on discord and pair program. Anyway, I'm here to help 👍

[bew]

Hey, I've been reading up on LuaLS annotations, and found @module did you try to use it?

Check the 2nd example @ https://luals.github.io/wiki/annotations/#module :

---@module 'http'
local http

--The above provides the same as
local http = require 'http'
--within the language server

It looks like you could define an empty local var representing the target module (LuaLS would consider it as the target module itself even though at runtime it's currently empty), and later fill it and use it. :thinking:

This way the 'complexity' of lazyness could be only where the lazyness happens, and all other modules would be clean without the need to define endless aliases and pseudo classes for modules.

[michaeldebetaz]

@bew Yeah! I saw it but didn't find a useful application in our case. Maybe you see something that I missed 💡 I'll try to play with this.

Anyway, I don't see a way around type casting/forcing when we use the lazy_table. It would be easier if we could capture a type with some kind of typeof, but no idea how to do that.

About the "endless aliases", I don't think we should discard them, as they should be declared in the different modules in order to better typehint the code. There are too many any or unknown to ensure more useful completion with the LSP.

[L3MON4D3]

I've looked a bit into the lazy_table-issue, and it seems like @generic, used as shown here, is a good solution.

---@generic T1: table<string, fun(): any>
---@generic T2: table<string, fun(): any>
---@param lazy_t T1
---@param lazy_defs T2
---@return T1 | T2
return function(lazy_t, lazy_defs)
...
end

One issue with this is that the function-arguments (to eg. insert_node in require("luasnip").insert_node), are not passed through, maybe it needs some more tricks, or luals just can't do that (yet?) :/

[michaeldebetaz]

Hi @L3MON4D3 👋 I was away the past weeks, but I am still available if I can help! What's the course of action so far? does my PR bring any help?

I tried to merge incrementally the solution from @bew (https://github.com/L3MON4D3/LuaSnip/pull/1117#discussion_r1468457076) but I see it creates a conflict with the autogenerated docs.

In any case, I am still here if I can help :)

[L3MON4D3]

does my PR bring any help?

Oh yeah, it certainly does :D

I see it creates a conflict with the autogenerated docs.

Oh, don't worry about that, those will be generated anew anyway :)

Did you try my suggestion for the lazy_table on your end? Did you run into the same issue? It seems like it would reduce the burden of manually keeping things in sync a bit :D

[michaeldebetaz]

I did work on it, but I can't find a way with my knowledge of leveraging lua-ls :/ I don't think it's avoidable. Type casting still is the best solution I found.

[L3MON4D3]

Oh hey, sorry, I totally forgot to actually answer you :grimacing:

I'm fine with just typecasting, it's not very maintainable, but then again, there probably won't be many more big additions, so it won't cause too much additional work in the future, and will give us the (probably) best results right now :D

[michaeldebetaz]

Okay, no problem at all :) what's the next step then? Should I go on with the PR? Or will you take it and adapt the changes with with your preferences before merging?

[L3MON4D3]

Hey, back again after a break :sweat_smile: I think I'll do a few of the changes that need more intimate knowledge of luasnip, the low-hanging but tedious fruits you've addressed nicely :)

[michaeldebetaz]

That's great! Let me know if I can do anything to help. Otherwise we can close this PR after you're done with the implementation :)