eclipse-jdtls
Allow clients to control when publishDiagnostics notifications are sent
Hello,
Is there a way to only send diagnostics on save? In Emacs JSON is a bottleneck, and being able to limit when diagnostics will be sent will be a great improvement.
Thanks, Theo
I think the LSP has support for this, but JDT-LS don't support this. You're describing textDocument/diagnostic, which is a message a client sends to the server to request diagnostics. So a client could certainly make the request as part of a document save only.
Currently we only support textDocument/publishDiagnostics, which is a notification the server sends to the client, at any time. We tend to send and compute diagnostics whenever there's a change that would indicate that should be recalculated. We mostly send them on initialize, didOpen, didChange, didSave etc.
Yeah, that's what I'm seeing too. It causes a flood of diagnostics, where many are just not needed, because I know the code is in a broken state. It is hard adding support for such an option in the init options? Because java projects tend to be pretty big it would be a huge boost in Emacs due to its single threaded nature.
I think it should be fine using textDocument/publishDiagnostics for this, right? I don't think it's needed a specific client request for this. Maybe a variable where you can state save, change or all, and let the clients configure that.
If the project is broken do you get any benefit from trying to import it at all ? If not then have you considered trying to run the syntax language server as opposed to standard one ? We provide a distribution called the syntax server which has fewer features but might help in this case.
Otherwise, I guess we could create some custom extendedClientCapability option that clients would merely need to set, and then we'd refrain from publishing diagnostics except for certain cases. Going forward, the better approach would probably be to implement textDocument/diagnostics and as long as the clients implemented the new protocol, they'd be able to control when diagnostics get sent.
My only opposition to using extendedClientCapability is that the protocol clearly has a proper way to do this, so using this option is only to make it easier for clients, in case they don't support the new methods.
The project isn't broken - just syntax-wise until you complete a statement etc. There is something to be gained for the more limited lsp clients to control this.
I know of the syntax server, but that's not the issue here. The issue is just that resending diagnostics for the whole project on almost every keypress is too excessive compute-wise for clients like emacs' eglot. Both approaches are okay from my pov, but I'm thinking not using the extendedClientCapability is best. But there's something to be said for a simple config option too.
What do you think?
The specs you linked to me reads a bit differently than what you suggested. It seems the textDocument/diagnostic is meant as a client pull request for the diagnostics of the current document, whereas the textDocument/publishDiagnostics is the one that will be "project-aware", in lack of a better term. So it would make sense to be able to decide when these diagnostics should be sent, but we should still get diagnostics for the whole project. This is something we could change in eglot or lsp-mode, by doing some trickery to ignore messages not sent right after a didSave, but that is unfortunately not enough in Emacs' world. Every message that is sent over the connection will be parsed and read, so the perf issue will still be there. If we were able to limit how many messages are sent, however :)
I hope the motivation for the change is a little clearer now - I was in a rush earlier, so my messages weren't the most detailed - apologies.
I can try to create a PR for the extendedClientCapabilities version if you want?
I didn't include this in my initiali post mentioning textDocument/diagnostic, but there is also workspace/diagnostic which handles the workspace case. This is probably a longer term solution (along with textDocument/diagnostic), as the easy thing to do now is just guard our existing diagnostic publishing entry points with a new extended client capability.
It's just a matter of finding the correct places. I think there's a spot within WorkspaceDiagnosticsHandler, a spot in BaseDocumentLifeCycleHandler where we publish diagnostics for the individual file, and a spot in project importing (initialize). Not sure if there are others. Might involve some investigation.
Hey @rgrunber ,
Hope you're doing well. I'm just checking in on the progress of implementing the textDocument/diagnostics method. As someone who's not deeply involved in eclipse.jdt.ls development but really benefits from its features, I'm pretty excited about the potential improvements this could bring, especially for static code analysis. Any update or insights you can share would be super helpful!
Cheers!
Currently the server is entirely responsible for diagnostics by sending them over to the client. We do this as part of project import, and also based on opened documents (ie. changing something in one document could affect others). By forcing the client to make the request, we could benefit from certain performance improvements. Maybe some clients don't care to fetch all diagnostics on project import, or want to customize the behaviour more than we're able to provide, or simply place a limit on how many diagnostics should be reported. I agree it would be good to implement the new methods.