Support optional Diagnostic.tags

[leungbk]

https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#diagnosticTag

A DiagnosticTag can be either 1 (DiagnosticTag.Unnecessary) or
2 (DiagnosticTag.Deprecated). Following the rendering suggestions in
the protocol, we fade out Unnecessary code and strike-through
Deprecated code.

• eglot.el (eglot--lsp-interface-alist): Add Diagnostic.tags.
  (eglot-client-capabilities): Advertise supported tags.
  (eglot-handle-notification): Assign the appropriate properties.

[skangas]

Thanks, this looks like a very useful feature. I would suggest that we should have faces instead of a defconst here, to make it easier for users to customize this with the full power of customize-option+defface. WDYT?

[leungbk]

Thanks, this looks like a very useful feature. I would suggest that we should have faces instead of a defconst here, to make it easier for users to customize this with the full power of customize-option+defface. WDYT?

Done.

[dgutov]

FWIW, company-capf uses the :company-deprecated property for the latter. And has a matching face definition.

The "unnecessary" tag never came up before.

[joaotavora]

I generally think this PR is good to go (after the usual git commit tweaks maybe).

@dgutov can you explain what your suggestion is? How can :company-deprecated help here? AFAIU this is for diagnostics, nor completions

[leungbk]

Oh yeah, do we want a test for this before merging? Pyright sends the Unused property, and that was how I checked my commit. I'm not at home right now, but I can get around to writing a test by tomorrow.

[dgutov]

Ah, sorry.

I thought you were going to add "deprecated" support for completions, which is also missing in Eglot.

[skangas]

@leungbk A test case would be useful, I think.

[leungbk]

@skangas Done. My test case (requires npm i -g pyright) passes after running make check.

[leungbk]

Actually...hold on, I need to spend some time debugging the test.

[leungbk]

@skangas OK, now it works. I used rust-analyzer instead since pyright-langserver behaved unexpectedly when running tests.

[skangas]

This seems to lead to a byte-compiler warning:

eglot.el:1822:1: Warning: Destructuring for Diagnostic has extraneous (:tags)

[joaotavora]

Nice. That just means that the Diagnostic type further up in the file must also be changed, informing Eglot that tags is a new optional field.

[leungbk]

This seems to lead to a byte-compiler warning:

eglot.el:1822:1: Warning: Destructuring for Diagnostic has extraneous (:tags)

I can't reproduce this with make clean && make check in the root directory. The Diagnostic type seems to be appropriately extended with the new optional :tags field; you might be looking at one of my earlier (botched) attempts at fixing a merge conflict, when I made a typo extending the Diagnostic type.

[skangas]

I can't reproduce this with make clean && make check in the root directory.

Hmm, I can reproduce this consistently... but it seems like I can't reproduce this after restarting Emacs, nor can I reproduce it from emacs -Q. I guess something was going on with the Emacs session where I'm seeing this. Sorry for the noise.

[skangas]

I had to increase the test timeout to 5 seconds, but now everything is working here. I see this diagnostic when replicating the test case manually:

rustc: unused variable: `test`
`#[warn(unused_variables)]` on by default
rustc: if this is intentional, prefix it with an underscore: `_test`

So I fixed up the commit message slightly (feel free to take a look at the changes) and merged this to master. Thanks for your contribution!

[theothornhill]

I can't reproduce this with make clean && make check in the root directory.

Hmm, I can reproduce this consistently... but it seems like I can't reproduce this after restarting Emacs, nor can I reproduce it from emacs -Q. I guess something was going on with the Emacs session where I'm seeing this. Sorry for the noise.

This can happen if you eval-buffer eglot. I've seen this sometimes as well, and I believe it has with some of the eval-and-compiles running around.

[joaotavora]

This can happen if you eval-buffer eglot. I've seen this sometimes as well, and I believe it has with some of the eval-and-compiles running around.

Yes, Eglot is meant to be compiled, then loaded. If you just interpret it you will get some of that trouble when you do try to compile it. So I recommend always M-x emacs-lisp-byte-compile-and-load when loading the whole file. Even for individual definitions, I tend to prefer compile-defun. But of course C-M-x and C-x C-e also work 99% of the time.

Nice test, btw, @leungbk! Seems very clean. And @skangas now that there's a rust-analyzer test there, maybe the "watches files" test can also migrate from rls to rust-analyzer?

[skangas]

@skangas now that there's a rust-analyzer test there, maybe the "watches files" test can also migrate from rls to rust-analyzer?

Yup, I'll look into it.