WhatsApp / erlang-language-platform

Erlang Language Platform. LSP server and CLI.
https://whatsapp.github.io/erlang-language-platform/
Apache License 2.0
242 stars 19 forks source link

Help message about variable's type is broken when source file has unsaved edits #32

Open dszoboszlay opened 4 months ago

dszoboszlay commented 4 months ago

Describe the bug

Hovering over a variable shows type information about the variable. However, if the source file has unsaved edits that moved the variable's declaration around, the help message is rendered incorrectly.

To Reproduce**

Write a module with this function in it and save it:

test(<<X:32>>) ->
  X + 1.

Hovering over X in the X = 1 expression displays the tooltip X :: number().

Now insert a space in the previous line before X, but do not save the document:

test(<< X:32>>) ->
  X + 1.

Hovering over X in the X = 1 expression now gives the tooltip :: number().

Expected behavior

Unsaved edits that only move the declaration around shouldn't impact the help message.

Actual behavior

It looks like the variable's name is taken from the same position where it was declared in the saved file, even though it may now be moved around. E.g. adding some space before a variable with a long name will chop off characters from the end of its name, and deleting white space from before the variable will chop off characters from the beginning of its name.

Interestingly, renaming the variable would display the type help with the correct variable name, and no compilation errors are reported until the file is saved. For example with this unsaved edit:

test(<<Y:32>>) ->
  X + 1.

Context

alanz commented 4 months ago

Currently eqwalizer on runs when you save a file.

dszoboszlay commented 4 months ago

Currently eqwalizer on runs when you save a file.

That's fine, the issue is that while the file is modified, the help texts take a wrong section of the file somehow. Which is strange, because as the last example of renaming the variable shows, the original, correct text is still available to elp.

alanz commented 4 months ago

Sorry for not checking the detail before.

Currently eqwalizer is loosely coupled to ELP, and we get type information by querying against a position. This is memoized, and only updated when eqwalizer processes the file again. We generate the hover string using the text at the location we stored, and this has now changed.

I suspect we may have to just put up with it until we have a tighter integration between ELP and eqwalizer, at some unspecified future point.