Diagnostics for the full project?

[JesusTorrado]

Hi there,

Is there any chance that pylsp will at some point implement a switch for diagnostics for the full workspace, as mentioned for pyright in this comment #143 (comment) ?

E.g. for static type checkers like mypy it does make sense to see possible bad side effects of changes in type declarations in other files. And it would be useful in general too, e.g. for activating it just before pushing a large change.

Thanks for the great work!

[ccordoba12]

Thanks for the suggestion. This idea is interesting but an implementation for it will have to come from the community because we don't have time for it right now, sorry.

[JesusTorrado]

Thanks for the answer, @ccordoba12! After taking a look at the code, I am tempted to give it a try. I will probably ask you a few things here until I feel confident enough to submit a PR, if you don't mind.

[ccordoba12]

Sure, no problem with that. And thanks a lot for your interest in adding this feature.

[JesusTorrado]

Hi @ccordoba12

I have been taking a look at this and I think there is some ambiguity about where to solve this:

• At the editor/client side, when the language server is initialised, by sending a didOpen signal for all files in the project (conditioned to some user setting).
• At pylsp side by exploring at initialisation the root tree and calling Workspace.put_document for every python file.

Not sure what is the right way to proceed, to be honest. The first one looks simpler, but maybe more memory-expensive, since more files need to be open? (or maybe the signal is sent but the file is not actually loaded in memory by the editor)

Since I am using eglot as a client, maybe @joaotavora has an opinion regarding this, or knows at what level this is addressed for other language servers?

[joaotavora]

Have you tried, M-x flymake-show-project-diagnostics? Some servers work with it just fine. Servers don't need to be notified that the client opened all the files to be able to analyze them.

[JesusTorrado]

Thanks, @joaotavora! For the python-lsp-server it shows the open files only, not all files in the project.

We'll go the sever-route then. The question now would be whether to put all the documents in the workspace at initialisation of the server, as suggested above, or whether to let the server plugins handle that (e.g. by not passing them a file name argument). I guess the first one makes more sense.

@ccordoba12@joaotavora Do you have any idea whose responsibility in LSP (whether the server or the client) is to gather the names and paths of these files?

I am sure project.el can do that in emacs, but not sure whether that list is passed at the moment (or can be passed) to the server in eglot.

Otherwise, if @ccordoba12 thinks that would be a good idea, we could scan the project root for .py files and put them into the workspace (conditioned to some user-defined setting about all-files vs open-files project diagnostics).

[albilu]

@JesusTorrado : From LSP spec there is a specific hook for this on server side: workspaceDiagnostic

But i think the better solution will be to enable the textDocument/diagnostic capability on server side textDocumentDiagnostic so that any LSP client can gather the diagnostic for each file.

So Option 1 is fine but instead of sending didOpen for all files at startup the client can request at any time the diagnostics for a set of files with textDocument/diagnostic

[ccordoba12]

In order for any client to easily take advantage of this functionality, I'd be in favor of implementing the workspaceDiagnostic response instead, which seems to be added to the protocol to cover precisely this use case.

[joaotavora]

Carlos Cordoba that could make sense, but eglot doesn't issue these new kinds of requests. It could. Suggest you submit a feature report to Emacs's bug tracker, link to the LSP spec page, and CC: me in the report email. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics Also please list one of two servers you know that support this.

[JesusTorrado]

Thanks, @albilu!

I'll try to get to this in 1-2 weeks.

@joaotavora I will take care of the emacs side when this is done.

[joaotavora]

OK, see also related https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59824 This bug report, which will soon be fixed (already have a fix) concerns a server that already supplies workspace-wide diagnostics without making use of the new workspaceDiagnostics "pull" request. So, with some servers, this functionality is already available in Eglot. Doesn't mean workspaceDiagnostics shouldn't also be added to Eglot, of course, but it's not essential to get this functionality to the end user. João

…

On Wed, Dec 7, 2022 at 8:20 AM Jesús Torrado ***@***.***> wrote: Thanks, @albilu <https://github.com/albilu>! I'll try to get to this in 1-2 weeks. @joaotavora <https://github.com/joaotavora> I will take care of the emacs side when this is done. — Reply to this email directly, view it on GitHub <#280 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6PQ2S3UKXIYQAPYX7USDWMBCEVANCNFSM6AAAAAARHDZJIA> . You are receiving this because you were mentioned.Message ID: ***@***.*** .com>

-- João Távora

[albilu]

@JesusTorrado any progress on this feature?

[JesusTorrado]

I am afraid not, and I don't expect to make any for the next few months. I hope to come back to this soon, but on my side feel free to close if you prefer.

[albilu]

i will rather convert this into a feature request if no one is against as this is a needed feature.

[simonlearnscoding]

hey guys any news on the workspace diagnostics feature?