[LSP] Fix fading diagnostics

[dibarbet]

The logic for fading diagnostics changed to use additional locations instead of always creating multiple diagnostics with different spans. This updates LSP to read the new diagnostic data and itself create multiple diagnostic reports for the unnecessary locations. LSP does not yet support having additional locations with different tags.

LSP diagnostics should now appropriately respect severities and tools options for fading. Also added tests to verify LSP behavior.

In the screenshot, usings and unreachable code are fully faded. Unnecessary parens fades the parens and warns on the first line when configured to warning severity.

Resolves #63147

[dibarbet]

this code was just moved into a common layer out of the tagger.

[mavasani]

@dibarbet So the approach in general looks good to me, and matches what we discussed offline couple of weeks ago. However, I am wondering if it is simpler to just remove this encoding logic in all such fading analyzers and instead have the analyzers themselves report separate hidden, non-configurable, unnecessary tagged diagnostics, maybe with a separate diagnostic ID. This way we avoid this unnecessary round-trip. I don't recall now why we originally decided to move from separate fading diagnostics to this encoding logic in additional locations - maybe design wise it is better to have a single diagnostic with additional locations for fading, but it seems performance and maintenance wise it might be better to go with separate diagnostics in the analyzer. What do you think?

[Youssef1313]

I don't recall now why we originally decided to move from separate fading diagnostics to this encoding logic in additional locations

@mavasani A bit of history here. This was introduced in #39649. There is no associated issues to the PR, but branch name is "dupe_error_list". We were using same ID for both descriptors s_diagnosticWithFade and s_diagnosticWithoutFade and both were configurable, so I think this can indeed lead to duplicate diagnostics. Using isConfigurable: false to one of these descriptors and changing its ID should be good to fix the duplicate diagnostics in a more performant way as you suggested.

One possible downside is that now these descriptors will appear in EditorConfig UI, but it's not much important I think.

[dibarbet]

maybe design wise it is better to have a single diagnostic with additional locations for fading, but it seems performance and maintenance wise it might be better to go with separate diagnostics in the analyzer.

IMHO the right solution is to have LSP support tags on additional locations. The problem with multiple diagnostics is its pretty easy to accidentally start showing them in places they aren't supposed to be, e.g. duplicate hover info, duplicate error list info, configuring hidden diagnostics accidentally, etc.

LSP already has the concept of diagnostic related information, I think we just need to extend it to add tags - https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticRelatedInformation

[dibarbet]

Forgot to mention one thing as well - VSCode doesn't have as complex support for diagnostics as VS does and doesn't have diagnostics that can be 'hidden'. IMHO it will be much easier to expand tags to additional diagnostic locations that it will be to introduce the concept of a hidden diagnostic in VSCode.

[CyrusNajmabadi]

Sort :)

[CyrusNajmabadi]

nit: i'd prfer a space above here.