Hidden DiagnosticSeverity?

[david-driscoll]

Roslyn has a hidden diagnostic severity that's used to fade out text when they can be optional removed. Things like unused usings, or when this. is not required.

It would be great to have a way to say a diagnostic is hidden for LSP servers.

[DustinCampbell]

FWIW, Hidden doesn't necessarily mean that a diagnostic is faded. The fading is handled in another way. Instead, a hidden diagnostic is actually hidden from the user but is used to help drive the IDE experience. For example, Roslyn creates a hidden diagnostic for "Remove Unnecessary Usings" over the entire block of using directives at the top of the file so that the user can invoke a code fix without having to move the caret into faded using directive.

[david-driscoll]

I stand corrected! 👍

[dbaeumer]

This is the idea of the hint severity which for example we in VS Code don't render in the UI as squiggles. Would that work for you.

[DustinCampbell]

@dbaemer, IIRC, that still causes the diagnostic to appear in the VS Code Problems list. When we used "hint" and that happened, we received a lot of complaints.

[dbaeumer]

@DustinCampbell that is correct. Reason was that hint are often inserted for code actions and people want to trigger them from the problems view.

So when exactly would you put a hidden severity on something. They aren't trustable anyways so when the server executes a specific functions diagnostics must be recomputed.

[DustinCampbell]

Hidden diagnostics are used within Roslyn to improve the user experience by indicating spans where a code action should appear but the diagnostic squiggle appears elsewhere. Here are a couple of examples where we've used hidden diagnostics:

1. Unnecessary using directives. There is a diagnostic created for every unnecessary using directive. This because overwhelming for users when it appears in the Problems list. So, Roslyn produces a hidden diagnostic. In Visual Studio, Roslyn will alter the presentation of the using directive so that it's faded in the editor. In addition, Roslyn produces a hidden diagnostic for the entire list of using directives, so that the user doesn't have to place their caret directly within an unused using directive to invoke the code action. They just have to get the caret into the block of using directives.

2. Implement Interface. This is a case where Roslyn produces an error diagnostic with a squiggle. However, that might not be the natural place for the user to invoke a code action. Consider the following code:

interface IB
{
    void M1();
}

interface IC
{
    void M2();
}

class A : IB, IC
{
}

In the code above, we want the user to be able to use a code action to implement an interface from the name of class A and the references to IB and IC. However, the error diagnostics only appear on IB and IC.

[dbaeumer]

@jrieken have you heard of such a request in your land as well ?

@DustinCampbell So you basically use them as markers for code action trigger (showing for example the light bulb). What kind of diagnostic severities does Roslyn support.

[DustinCampbell]

Roslyn supports the following severities with the given behavior in Visual Studio.

• Error - Red squiggle in the editor and appears in the Error List.
• Warning - Green squiggle in the editor and appears in the Error List.
• Suggestion - Short underline of gray dots in the editor and does not appear in the Error List. These are often used for code style suggestions.
• Info - No squiggle in the editor and appears in the Error List.
• Hidden - No squiggle in the editor and does not appear in the Error List

In addition, Roslyn can provide additional presentation, such as fading code, which is orthogonal to the severities above.

[jrieken]

@jrieken have you heard of such a request in your land as well ?

Kind of... I know that Roslyn uses that trick and it would work for us because we forward diagnostics to the code action provider. Tho, it does feel a little weird and complicated for just sending information from one part of the system to another... Especially given our open approach, so a FooCodeProvider will be able to access the hidden diagnostics of the BarCodeProvider

[DustinCampbell]

@jrieken : I'm not sure I understand that last bit. What is the "open approach"?

Note that, in Roslyn, a CodeFixProvider is registered with the particular diagnostic IDs that it responds to.

[DustinCampbell]

FWIW, I agree that hidden diagnostics are weird in general. Really, their trigger points for particular code actions. IMO, this should have been handled differently in Roslyn.

[jrieken]

Note that, in Roslyn, a CodeFixProvider is registered with the particular diagnostic IDs that it responds to.

In VS Code you register your project for a language but not more fine grained. They call have access to the same set of diagnostics. Basically those that intersect the range being asked for...

[DustinCampbell]

Yup! it's not that really that big of a deal, but "hidden" diagnostics are even weirder in VS Code. It's like seeing private implementation bits. Is there some other mechanism we would map to in LSP to achieve a similar user experience to what I described above?

[dbaeumer]

Two things:

• now that I understand this better I think we should not show Hint in the problems list (@jrieken @sandy081). For me it would be unclear what the difference between a hint and and info is
• for the C# case I would simply not use diagnostics to control code actions. IMO they can't be trusted anyways (since they might be outdated due to edits (even in other files) or the editor not adjusting them as expected). So if the server is asked for code actions at a particular location the AST has to be reconciled and the actions need to be computed on the reconciled state. Would this be a problem for OmniSharp ?

[DustinCampbell]

for the C# case I would simply not use diagnostics to control code actions. IMO they can't be trusted anyways (since they might be outdated due to edits (even in other files) or the editor not adjusting them as expected). So if the server is asked for code actions at a particular location the AST has to be reconciled and the actions need to be computed on the reconciled state. Would this be a problem for OmniSharp ?

Perhaps I'm misunderstanding, but Roslyn code actions are driven off of diagnostics, so they couldn't be completely disconnected. In VS, I believe we force diagnostics to recompute when Roslyn is queried for code actions. So, everything is up-to-date before we even return anything. Of course, the same arguments about diagnostic not being trustworthy applies to code actions as well doesn't it? Just because a code action has been returned, there's no guarantee that running it is valid.

[dbaeumer]

This is true and given all the async behavior it can't be fully guaranteed to be correct except we always include ALL open document version is edit requests. However to make the window as small as possible I implement that in the following way in my servers:

• I do not trust diagnostics :-)
• If ask for a code action I return an tuple [id, document version] to the client
• when selected I send the code action to the server using the execute command request
• the server checks the code action and computes the edits
• the server sends a applyWorkspaceEdit request to the client. This request contains the edits and the document including the version number to apply
• if the document on the client side has moved forward I drop the edits (done by the LSP client)
• if it is correct I apply the edits (done by the LSP client)

This still leaves room that editing another file would have invalidated a code action. However that approach doesn't apply invalid edits to an open document.

[DustinCampbell]

That seems reasonable. You shouldn't trust diagnostics 😄 and having a validation step before a code action is applied is the right thing IMO.

We do something similar in Roslyn. Because we have an immutable snapshot model for what is effectively the entire workspace, we can version the contents of the entire workspace, not just each document. So, when a code action is applied, Roslyn checks to see if the workspace has the same version that the code action was created for and fails if the version has changed.