Feature: Support Inlay Hint

[aspeddro]

I used DocumentSymbol.ml to try to understand how the parser works. There's a lot of room for improvement.

I'll list some that I need help with:

• Update type hints when compilation is finished

• Find a better way to get the return type. I split the string at => and got the last item in the list.

• A better way to find the starting position of the variable.

• Get type hints for a range, currently gets the entire file.

• Type Hint for tuples destructuring

  let tuple = ("ReScript", "lol")
  
  let (lang, _) = tuple

  Output:

  let (lang: string, _) = tuple
  

  • Ignore type hint for _?

• Provide a setting to disable inlayHint

• Add type hint for pipe

• Type hint support in parameters

cc @zth

[cristianoc]

This is probably a good feature to make configurable with an opt-in. @zth what do you think?
Side comment, prob better to move the code cleanup/rename part to a separate PR, so it's not mixed up with the new functionality.

[cristianoc]

Unless it's clearly separated in the commits already.

[aspeddro]

Inlay Hint requires an updated version of vscode-languageserver. It is part of the latest 3.17 specification.

https://github.com/microsoft/vscode-languageserver-node#3170-protocol-800-json-rpc-800-client-and-800-server

[zth]

So, adding to @aspeddro and @cristianoc , I think we should do this:

• Change this PR so it only involves implementing inlay hints. Renaming and other refactoring (and even formatting of things that wasn't previously formatted, but has no other changes than just formatting) should go in a separate PR so this stays as on point as possible.
• Ensure there's an option for toggling inlay hints. I can fix that ✋
• Ensure the inlay hints are refreshed at compile finish. I can fix that too ✋
• Figure out the missing cases, like you mentioned @aspeddro. We'll need the expertise of @cristianoc here probably. But we'll get to that after we've done the other things.

I'll go ahead and fix the points I mention above asap. @aspeddro can you remove formatting, refactoring etc from the PR meanwhile?

Great work so far btw, this is really exciting and will be a great feature!

PS. I wonder if we could just reuse the full completion logic for finding the identifiers to give inlay hints? I have a feeling that will map quite well. It even works on unsaved sources, and we can easily filter out anything it doesn't find a type for yet. Thoughts @cristianoc ?

[aspeddro]

I'll go ahead and fix the points I mention above asap. @aspeddro can you remove formatting, refactoring etc from the PR meanwhile?

Done

[zth]

@aspeddro quick update - just like you said/did, we'll need to upgrade the VSCode client version for this to work out. I will do that in a separate PR that we can then rebase this on once merged. That'll make the diff a bit cleaner. You'll hear back from me in a little while.

[zth]

@aspeddro client is now upgraded, and inlay hints are now activated again (behind an option). So I think we're ready to continue working on the implementation parts in the analysis bin.

[aspeddro]

Updates, I made some advances in the analysis bin:

• Type hints in parameters.
• Tuple support

TODO:

• Type hint for pipe call
• Type hint for parameter names
• Show only type of return for function

[aspeddro]

About the return type, is it better to implement in syntax repo or in analysis?

It is similar to res_outcome_printer.ml.

[zth]

Curious, where would you show only the return type?

[aspeddro]

Should this do anything with interface files?

Type hint requests are now for .res files only.

[aspeddro]

There seems to be a range parameter sent with the inlay hint request that we're currently not taking into account: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#inlayHintParams

Let's try and implement that and see if it helps. I would assume VSCode leverage that to reduce the amount of hints asked for att once in large files.

I wonder how to handle the automatic refreshing on compile though since I don't think we have that information there. Let's think about that for a bit. scratch that, we don't need that information for a refresh, we're just telling the client things might need refreshing, and it'll issue the requests for hints that it needs to refresh

Added support for range. analysis/examples/large-project/src/res_core.res is no longer locking the extension

[zth]

Great work @aspeddro , and thank you for the quick turn around! I'll have another look asap.

[aspeddro]

The latest test results were not checked in. Done that. Could you check that this is still the expected result?

Tests Fixed.

[cristianoc]

Have you tried with res_core.res? What was your experience?

[aspeddro]

Have you tried with res_core.res? What was your experience?

Yup. When scrolling sharply to any part of the file, eg top (line 1) to middle (line 3k), it takes about 3 seconds to show the type hints. But if you scroll smoothly the result is responsive.

[cristianoc]

@zth how's your experience on that file?

[cristianoc]

Have you tried with res_core.res? What was your experience?

Yup. When scrolling sharply to any part of the file, eg top (line 1) to middle (line 3k), it takes about 3 seconds to show the type hints. But if you scroll smoothly the result is responsive.

Has something changed? I have the same experience now, instead of just "loading..." on any hover on anything.

[cristianoc]

I'd like to know what happens when one scrolls slowly.
I suspect the 3 seconds delay is still there, but you don't see it as the work happens on a bunch of not-yet-visible code.
So there's a lot of processing happening all the time.

[cristianoc]

Did some tests, and turning on this feature, on slow scroll, on my m1 Mac, pushes code renderer slightly above WindowServer, in terms of cpu utilisation. While it was slightly below beforehand.
So there's an observable difference, but not a huge one.

[cristianoc]

In terms of memory, vscode stays under every one of the 4 instances of ocamllsp I have on my machine at the moment (no idea why multiple instances).

[zth]

I tried inlay hints in TS on a huge file, and it's noticably faster than ours. Maybe we should investigate how many calls are made, and if there's a bottleneck somewhere for us. This might of course also simply be the result of the TS server keeping much more cached in memory. We don't know.

But I'm not sure if we should do that in this PR, or in a follow up (and also investigate pipe calls etc). We've marked this as an experimental feature, and it's behind a setting that's off by default. More inclined to merge now and continue in separate PRs so this isn't left hanging for too long. I think it's in a good state already, especially given it's experimental.

[cristianoc]

Yup let's merge. I only asked for one small clarification for the config. Then we can merge away.
I thought it's good to go in details now that it's fresh in everybody's mind. Easier than having to come back later.

[cristianoc]

@aspeddro thank you so much for doing this.