Use Server side Diagnostics

[vidit-od]

Fixes #4805

This PR aims to deliver server-side diagnostics to codeactions.
The Comment below linked issue by @fendor stated

Ideally, we come up with some API or API changes to the plugin handlers that allow us to omit the step of manually calling activeDiagnosticsInRange* and automatically do the right thing... But I don't know right away how.

This is the approach i followed, hopefully it aligns with the expectations :

• exposed a new API named mkCodeActionHandlerWithDiagnostics
• previously users called mkPluginHandler SMethod_TextDocumentCodeAction
• now users need to call this API instead,
• internally activeDiagnosticsInRange is used to fetch server-side diagnostics
• if file not published / server-side diagnostics inaccessible then use client side diagnostics
• call mkPluginHandler SMethod_TextDocumentCodeAction with this fresh diagnostics ( if available ).

Edit :
the above mentioned approach has been discarded. The new approach follows :

• making an injectDiagnostics function that uses activeDiagnosticsinRange to fetch server side diagnostics.
• if no server side diagnostics are found then return an empty list.
• this function is later used after intercepting codeactions requests and used to inject server diagnostics.

[vidit-od]

Below is a small documentation of a different API Design that i tried exploring.

Origin :

if we notice the use of our new API in change-type-signature-plugin. we can observe the use of ActiveDiagnosticinRange as well as our API.

• Our API uses this same function internally as well
• i.e for each call we will be using it 2x
• This is inefficient, but done just to justify the goal of this issue.

attempted solution

TLDR : case (File Not published ) => empty codeaction on first load :)

• To fix the above issue i tried to update our API to accept [FileDiagnostic]
• This leads to a conflict in fallback case ( when file is not published and we have to rely on clint side )
• client-supplied diagnostics are of type [LSP.Diagnostic], and since FileDiagnostic is a richer server-side type, we cannot reconstruct it from [LSP.Diagnostic]
• essentially forcing us to return Nothing here

Pros :

More versatile API ( and efficient )

Cons :

Fall back case results to no codeaction

[fendor]

Using a separate primitive for writing CodeAction handlers does the job, but can we do better?
Could we, for example, interject in ghcide all CodeAction requests and replace the Diagnostics once and for all, without plugin authors having to remember that they should use something else for CodeAction handlers only?

I think we perform some similar special logic in ghcide for certain requests?

[vidit-od]

I began working with this same logic that you present.

I tried updating the f' helper that is responsible for intercepting and calling for codeaction in mkpluginhandler.

Later discarded it because that felt too complicated.

But yea, i can surly work on it. Just so that we are on the same page, our expectation is :

Basically, update mkpluginhandler such that it does everything our new API does. Helping users to remain as they were before on the caller site.

[vidit-od]

I think this does what we were expecting. Surprisingly simpler than what i was trying to do before so thanks for the direction :)

[fendor]

Could you also expand the commit message to contain more information? Server Diagnostic is really not enough, a proper commit message would be good for such an internal and somewhat tricky change.

[fendor]

Are there any call sites of activeDiagnosticsInRange we can now get rid of?

Also, you have to rebase this PR as there is a conflict.

[vidit-od]

Are there any call sites of activeDiagnosticsInRange we can now get rid of?

I don't think we can replace any of the activeDiagnosticsInRange. All of the plugins that use it expect [Filediagnostics]. This is later used to extract fdStructuredMessageL.

Below is the list of all plugins that uses activeDiagnosticsInRange and i investigated :

• hls-change-type-signature-plugin
• hls-class-plugin
• hls-pragmas-plugin

Also, you have to rebase this PR as there is a conflict.

I have Rebased it on top of master;

[fendor]

Some doc changes, and one thing to discuss, otherwise looks good to me!

[fendor]

LGTM, thank you!

[vidit-od]

I have rebased on top of Master;
should pass all of the previous failing tests now !

Thanks a lot for reviewing and co-authoring this PR 🚀