JohnnyMorganz / luau-lsp

Language Server Implementation for Luau
MIT License
242 stars 58 forks source link

Update requires when file renamed #124

Open JohnnyMorganz opened 2 years ago

JohnnyMorganz commented 2 years ago

We should support automatically updating relevant requires when a file has been moved / renamed

metrowaii commented 2 years ago

Coming from other IDE's you typically have the option to specify whether or not you want to update imports after renaming/moving a file, is there a way to accomplish this via VSCode's API? Or would we simply introduce a setting to toggle this on/off?

JohnnyMorganz commented 2 years ago

Yeah, I have also seen this, but I'm not sure how it's done. I know the typescript extension does it.

The main thing is it needs to be asynchronous - we should not block the whole LSP loop whilst waiting for the user to pick whether they want to update or not.

Note there is a window/showMessageRequest request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#window_showMessageRequest which can be sent on textDocument/didRenameFiles, and we can send a workspace/applyEdit request for changes.

There is also textDocument/willRenameFiles where the server can respond with text edits immediately instead, but if we wait on user input this will be blocking

JohnnyMorganz commented 2 years ago

For detecting files where we need to update the rename, note that Luau has a Frontend::markDirty method (https://github.com/Roblox/luau/blob/edb445392441582b722026dbfabed86706d6750d/Analysis/src/Frontend.cpp#L778) which populates a vector with the reverse deps/dependees that were also marked dirty (i.e. the files that required this file)

If we mark dirty the old module name (before rename), we can get this list. An important note however is that it only resolves reverse deps that the type checker has indexed and knows about (i.e. frontend.check was called on that module). By default, we do not do this except when computing workspace diagnostics, so we would need to make sure everything is indexed before computing reverse deps

JohnnyMorganz commented 1 year ago

Turns out this problem is more difficult than expected, due to the race condition between a file being renamed and the sourcemap being updated. We need the sourcemap to be updated before we receive the rename event to be able to track what the new path to the file should be