Langium documents don't get removed after the containing directory is renamed

[lars-reimann]

After renaming a directory, all DSL files contained inside are listed twice in services.shared.workspace.LangiumDocuments. This can affect various downstream features, like linking, type computation, or validation.

Langium version: 2.0.2 Package name: langium

Steps To Reproduce

1. In a Langium project, observe the contents of services.shared.workspace.LangiumDocuments. In the repo linked below, it gets logged in the output panel.
2. Create a folder.
3. Create a DSL file inside (extension .hello for the repo linked below).
4. Rename the folder.

Link to code example: https://github.com/lars-reimann/langium-cst-range

The current behavior

Before renaming the directory, the document is included (correct):

After renaming (third array), the new document is shown (correct), but the old one is not deleted (wrong):

The expected behavior

Only the new document should be included.

[msujew]

@lars-reimann Thanks for the report. I believe this is part of a larger issue where we don't actually listen to a few of the workspace related LSP notifications, mainly:

• workspace/didChangeWorkspaceFolders
• workspace/willRenameFiles
• workspace/didRenameFiles
• workspace/willCreateFiles
• workspace/didCreateFiles
• workspace/willDeleteFiles
• workspace/didDeleteFiles

Some of these are not interesting for the default behavior of Langium, but we should definitely start handling the didChangeWorkspaceFolders, didRenameFiles, didCreateFiles and didDeleteFiles notifications.

[spoenemann]

I had a closer look at this. We already get notifications of most changes through workspace.didChangeWatchedFiles notifications. So the only case where we need additional notifications are when folders are renamed, as those are not covered by the glob pattern **/*.hello. We could do this by explicitly watching all folders in the language server, but what I don't like about it is that we have a distribution of responsibilities between language client and language server code. It would be better to find a solution where watched file system events are specified fully on the client or server side.

[cdietrich]

see also didClose in https://github.com/eclipse-langium/langium/discussions/1281

[spoenemann]

I experimented with adding / removing workspace folders with Langium documents and did not see any issues. So it seems we don't need to handle didChangeWorkspaceFolders for now.

A service for the file operations is introduced in #1286, but as mentioned in that PR, those notifications should not be used to trigger DocumentBuilder updates. They are rather meant to enhance the user experience when working with the IDE, e.g. by automatically changing some text content when you rename a file (we could make use of that for langium file renaming).

The remaining question is how to fix the issue reported here. My current understanding is that we should use the file system watcher for that by adding a suitable glob pattern and handling it.

[sailingKieler]

While reviewing and testing your changes of #1286 I could produce a corrupt state of `langiumDocuments by renaming a document's parent folder and then opening the document, see screenshots.

Thus listening to didChangeWorkspaceFolders is necessary I guess.

[msujew]

Thus listening to didChangeWorkspaceFolders is necessary I guess.

workspace/didChangeWorkspaceFolders is for the case when you actually change the workspace by adding a completely new workspace folder or remove one from the workspace. Note that workspace folders are not folders inside of a workspace but the directories that constitute a multi-root workspace.

But I agree. The language server should listen to it.

[sailingKieler]

workspace/didChangeWorkspaceFolders is for the case when you actually change the workspace by adding a completely new workspace folder or remove one from the workspace. Note that workspace folders are not folders inside of a workspace but the directories that constitute a multi-root workspace.

@msujew you're right, I didn't check the spec, which is saying "about workspace folder configuration changes"

[sailingKieler]

The remaining question is how to fix the issue reported here. My current understanding is that we should use the file system watcher for that by adding a suitable glob pattern and handling it.

This seems not to be possible in an adequate way, see https://github.com/microsoft/vscode/issues/60813

An explicit remark on that was deleted with https://github.com/microsoft/vscode/pull/139881/files#diff-0a75aed19c118603eb96332bc0b9c2d7867f4182346d16d18b7fc31b6ceeb321L10951-L10952 beginning of last year.

I could make notifications on folder name changes work with a glob pattern like ('**/{*,*.<extension>}, which however kills the file extension based filtering, and in case of a folder name change we would have to look for documents with the corresponding folder path prefix on our own... 😕