Language-server-protocol: Add support for explicitly requesting diagnostics

[w0rp]

The Language Server Protocol sends diagnostics to clients as notifications. These notifications are typically sent in response to textDocument/didChange or textDocument/didSave, but they could arrive at any time. Clients have no control over when or if diagnostics will be sent to them. This lack of control causes problems for clients.

1. Clients cannot indicate if there there are pending diagnostics. Because they can arrive at any time, or may never arrive, you cannot indicate if there are diagnostics being calculated or not.
2. Clients cannot easily control when diagnostics should be displayed. A document update in order to retrieve accurate completion results may result in diagnostics being sent, for example.
3. Servers can send clients diagnostics for files they might not be interested in, such as files that are not open in client editors. This results in clients having to read more data than they need.

I think it would be a good idea to add an optional mode to the protocol to disable sending diagnostics to clients unless they are explicitly requested for a file. An optional attribute could be added to InitializeParams which when enabled would tell servers not to send diagnostics until a new message type textDocument/getDiagonistics is sent. ServerCapabilities could include an attribute for indicating if this mode is supported. Servers would then be required to respond to clients with textDocument/publishDiagnostics with either an empty diagnostics array, or previously computed results.

For my LSP client ALE, this would mean that ALE will be able to update the Vim status bar to indicate that diagnostic results from language servers are pending, and to avoid requesting diagnostics when the user only wants to get some completion results. Currently ALE is able to do this for tsserver, because the tsserver protocol requires the use of a geterr command, but this is unfortunately not possible with language servers.

Let me know what you think. Thank you for the protocol.

[dbaeumer]

@aeschli we discussed something like this awhile back as well.

@w0rp the guarding of such a capability would need to be done in the client and server capabilities, not in the InitializeParams. However if servers support that mode they shouldn't push any diagnostics anymore.

One tricky part could be is how to clear diagnostics on document close. Clients would need to send a corresponding request in this case.

[aeschli]

IMO we need:
a. a new request textDocument/getDiagonistics for the client to request diagnostics for a specific URL from the server
b. a way to configure the server whether and for which URLs the server it should publish diagnostics. This will likely be a selector as the client doesn't know all URIs that can get diagnostics.

However if servers support that mode they shouldn't push any diagnostics anymore.

I don't think a. and b. should be coupled. The user can configure it that way, but you can also think of a usage where the problems view is fed by published diagnostics (as now) but there are also programmatic requests for diagnosics of a specific document for other another use cases.

[dbaeumer]

I don't think a. and b. should be coupled.

After reading this again I agree.

[w0rp]

I agree that the explicit diagnostic requests and disabling automatic publishing of diagnostics in response to notifications like textDocument/didChange can be implemented as two separate features.

[kdvolder]

FWIW: The setup where server 'pushes' diagnostics works just fine from our point of view. And this also fits with situations where the server performs something like workspace builds, and then pushes any build-related problems as 'diagnostics' to the client. Such workflow is not driven by the client, but by the server.

I do also agree that there may be situations where a pull model makes more sense. I'm fine with adding that as long as its a 'opt-in' sort of thing and we also keep the 'push' model as it exists now.

[w0rp]

I think both are valid use cases, for different purposes.

[dbaeumer]

Just to clarify: it is not our intention to deprecate the push model. Pull would be simply another way to get to diagnostics.

[apexskier]

I have another use case for this. For https://github.com/apexskier/nova-typescript/, the editor manages the language server entirely, and intercepts diagnostic notifications. When requesting code actions, I need to pass diagnostics to the server to receive all actions, but I can't because I currently don't have a way to know the diagnostics. I'd say this is more of a Nova issue - it should provide a way for me to get a list of diagnostics, since it's already managing them, but this would be another way to solve it.

[gwerbin]

Would it make sense if the client could "filter" diagnostics, in addition to adding the "client pull" capability?

For example, the client could ask the server to not pull diagnostics matching tag = "Linting" and code ∈ (E328, E089), and then could later specifically pull an unfiltered diagnostic when the user wants it.

This is like the difference between a "signal" and a "method" in D-Bus.

[w0rp]

If there's ever the ability to filter which type of diagnostics are received, I think it's worth implementing that after being able to just pull everything at first, so we can see that by itself finished sooner.

[sam-mccall]

It made the VSCode release notes now, and we're getting some questions about it!
https://code.visualstudio.com/updates/v1_54#_language-server-protocol

It would be great to have some details in the upcoming docs :-)

From reading https://github.com/microsoft/vscode-languageserver-node/blob/main/protocol/src/common/proposed.diagnostic.ts, a couple of questions:

• the request returns a diagonstic list | null, or an error as usual. What's the effect of returning null? (when reparsing includes, clangd goes into a low-fidelity state where it doesn't publish diagnostics. It would be useful to us to return null and then request refresh once we're done, but I can't tell if that design is valid)
• as a language server that supports real-time diagnostics, I'm not really sure how to populate the pull flags. We'd prefer to push if possible (as it gives us more flexibility in scheduling), so I'm not sure whether we should set all the flags or none of them.

[dbaeumer]

@sam-mccall as said this is currently proposed and might very likely change again. To clarify some things:

• if a server can't fulfil a request right now because it is busy the server can cancel the request and the client is supposed to retrigger it.
• we also will provide a workspace pull model which will allow to compute all errors for the workspace. We are working on details but it will consider the scheduling of the server.

Push will not be removed. So if we server wants to use push it will be able to do so. However currently it is hard to tell for a server if and when it should compute diagnostic for a single document.

[sam-mccall]

Thanks!

if a server can't fulfil a request right now because it is busy the server can cancel the request and the client is supposed to retrigger it.

If the client retriggers, we'll likely (quickly) cancel the request again, and it can take up to a minute to become ready. It'd be nice to be able to have the client wait for a refresh request to retrigger to avoid a retry loop, but probably not a huge deal in practice.

FWIW we'd been (independently) looking at extensions for pull-diagnostics to better support distributed web clients, where pull seems to work better than push. I'm not sure if you've had this case in mind, but the pull+refresh model in this extension (and semanticTokens) looks good from that perspective.

[dbaeumer]

Yes, being able to explicitly pull for diagnostics (even programmatically via API) is the use case we want to support as well.