Typehinting luasnip

[michaeldebetaz]

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 😅).

[bew]

Note that class annotations are usually only used when there are fields with values, here it is not really a class but a module with only functions.

Also please don't duplicate function signature between here and the docstring above each function, it'd be very annoying to maintain.

[michaeldebetaz]

Thanks for the feedback! I used class indeed in order to restrict the fields you can invoke from a module. Is there a way to the same with a table?

That's very helpful, because the starting point was pretty annoying to get done and I am still new to Lua and to leveraging the lua-ls LSP. I would be happy to implement your requests if you want 👍

For the docstrings, I tried not to touch the original code too much. I was hoping someone more expert would help tidy that up.

If there is a better way than submitting PRs to improve the type hints incrementally, I would be happy to follow. But I don't think we can avoid making those kind of roundtrips, because I am too much of a LuaSnoob for understanding what's going on in the code base.

You can count on me however to produce some starting point and do the chores, as it's time-consuming. I hope my contribution can be useful to you and to the users! Let me know how I can be of better help 😊

[bew]

I think this generic type can be restricted to be a list:

Suggested change

	---@generic T : table
	---@generic T : any[]

[michaeldebetaz]

As I understand it, Lua only have table as a data structure. Like for your previous comment about class vs. table, I would say it boils down to how it is more convenient for you to typehint your code.

any[] is the same as table<integer, any> (which is better than table and I prefer too the array type). But I think any should be avoided if we can narrow the type.

Do you have a better type? Usually if I let any or unknown, that means that I couldn't guess the type.

[bew]

Well in this case deduplicate is very generic and handles any kind of list without value type restriction, there is not much we can do.
Maybe it's only used with some specific types, but that's beside the point of type hinting, typing a function should represent the function's interface and limitation not how it's going to be used.

When you have T: table it's basically table<any, any> so restricting to lists of any is an improvement even if we still have any in the description (it's one less than before!), and it clearly explains that the function is limited to processing lists and not any kind of tables (as it's using ipairs to iterate and not pairs)
¯\_(ツ)_/¯

[michaeldebetaz]

Alright, let's change for any[] then if there is nothing better 👍

[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. 🤔

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?) :/

[L3MON4D3]

Oh, this gave me a few thoughts:

• I think the annotations should only communicate official API, which for a snippet means relatively few fields, I think trigger, name, description, captures and env would be official, or are so commonly-used that they won't be changed, while stuff like id and wordTrig should not (yet) be relied upon.
  Also, there are a few interesting functions for snippet, see here
• This is kind of a weird place to define Snippet, we should probably use a separate file for basic classes like that (see lua/luasnip/_types.lua or lua/luasnip/extras/_extra_types.lua for examples of this) (also, looking at them, it may make sense to namespace our classes 😅)
• Not sure if you recall my little digression on snippet vs expandable vs addable in the discussion you opened, but snip_expand would take an expandable. But since this distinction exists in very few places, and is yet another thing newcomers would have to deal with, it's probably fine, especially for now, to just make most things Snippet

[L3MON4D3]

(correction: id can also be official 😅 Just saw that we have get_id_snippet, and that'd be pretty stupid without some way to get the id)

[L3MON4D3]

Yeah, I think we'll also need a Node-type, first for arguments like this, and second to communicate API (which is currently described here)

[L3MON4D3]

Isn't that one - too many? Or intentional to hide them?
I think luals can identify that a function belongs to a module if you do something like

---@class ExtendDecorator
local M = {}

---@return any
function M.foo() ... end

(see for example here)
so the original annotation should be fine?

[michaeldebetaz]

Oh yeah, I don't know where comes from. Maybe because lua-ls was hinting an error? lua-ls uses ---@ to understand the types, so it throws an error if the declaration doesn't match what is expected.

I'll check it once I run my code in my NeoVim.

[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 (#1117 (comment)) 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 😬

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 😅
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 :)

[tmillr]

Ahhh here it is. I was wondering why this plugin didn't have any types/completion. Since it's June now and there still aren't any types, perhaps this could be addressed in an incremental fashion? E.g. for the time being just start by implementing the most common/central interfaces (that are external/public) for creating snippets (like s(), i(), and t())?

Also FWIW, I have a decent amount of experience with emmylua/luals type annotations and can help or give an opinion if needed (though the luals impl can be a bit tricky and quirky at times, so the result is practically never going to be 100% perfect, especially in comparison to other languages that are statically-typed by default like rust or typescript). With that said, having types or some types/hints is usually better than having none at all (in which case you have to constantly cross-reference with a browser window or another document containing documentation).

---

Edit: hey @michaeldebetaz thanks for bringing light to this issue and opening this pr. I see you just closed it. Just for clarity, I'm not associated with this repo or anything (I'm just another user and thought this might be a good place to comment on the topic of lack of types in the plugin) and my comment above was mainly addressed to @L3MON4D3 to try to gain some insight into what his plans were for this.

[L3MON4D3]

I'm still planning to finish this, but I don't have that much time for it right now :/
Sorry for not giving some kind of update @michaeldebetaz

@tmillr, feel free to take a look, I'd appreciate input from someone who has experience with lua-annotations :) (mine is limited to what I've done here in luasnip, which isn't much :D)