search

eclipse-langium / langium

Next-gen language engineering / DSL framework
https://langium.org/
MIT License
725 stars 65 forks source link

Ensure expected document state in LSP responses #1334

Closed msujew closed 7 months ago

msujew commented 9 months ago

Related to https://github.com/eclipse-langium/langium/discussions/1290

In the discussion in particular, the console warning appears due to the GLSP handler not awaiting the linking phase. However, all of our own LSP handlers haven't been awaiting that phase, instead opting to respond as quickly as possible. This sometimes leads to unexpected behavior in the response or tries to resolve a cross reference before it is ready.

This change attempts to mitigate the risk of this issue appearing by adding a waitUntil method to the DocumentBuilder. We use that method in the language server to wait until the required data is available for the respective service to do their work. This is usually either IndexedReferences (for any service that requires to resolve cross references) and Parsed (for services that operate on the AST/CST directly).

The new waitUntil method can also be interrupted in case it is no longer required. It will then reject (see tests).

msujew commented 8 months ago

@sailingKieler I've also thought about the waitUntil API placement. However, I don't think it makes a lot of sense in the LanguageServer service interface. While the API isn't used outside of the LSP context (yet), it's own functionality isn't really related to language servers at all. Assuming we merge the LSP split, that would make it hard for users to use the method without importing langium/lsp. I don't really see a reason to move it outside of the DocumentBuilder.

On the other hand, it's kind of a sugar API based on the build listeners, right, with the additional support for cancelation checking?

Also, I wouldn't say it's API sugar. It has two separate use cases:

  1. onBuildPhase triggers on every call of the build/update methods. The idea is to be able to intercept the builder process and run your own code in addition to that.
  2. waitUntil only observes the document builder once. It is supposed to be used to ensure that outside processes deal with a valid state of a document/workspace.

With that in mind, I believe it's worth to have it's own API.