feat: unused variable code action

[NJichev]

Very barebones draft for the code action

[NJichev]

I imagine we'll have several CodeActions, some basic ones for example:
NextLS.CodeAction.UnusedVar
NextLS.CodeAction.AddRequire

And each will have a new/1 function that converts a diagnostic/check/placeholder struct to the code action which will contain the necessary TextEdits inside for the client.

[mhanberg]

i assume you got this terminology from the credo language server.

there its called a check because there are credo checks. i'm not sure if the same concept applies here.

I think the thing to do would have the elixir extension have the code for converting its diagnostics into code actions.

with that, i'm not sure exactly if a protocol is going to be the best way to go here, as they are all going to be the same diagnostic struct.

[mhanberg]

so for this, I think you're going to one to loop through the diagnostics, then branch on the namespace and ask the appropriate extension to return code actions for it.

you'll also need to modify each extension to add the "data" property with the namespace. the data property is used to send data down to the client, then it sends it back up.

[NJichev]

Doing the latter, i.e. retrieving the diagnostic from the cache would probably be wrong, since as documented for the diagnostics field in %CodeActionContext, it says:

  * diagnostics: An array of diagnostics known on the client side overlapping the range provided to the
    `textDocument/codeAction` request. They are provided so that the server knows which
    errors are currently presented to the user for the given range. There is no guarantee
    that these accurately reflect the error state of the resource. The primary parameter
    to compute code actions is the provided range.

It seems to me that getting the diagnostics from the cache could lead to situations where the client isn't seeing a diagnostic but we send back a code action for it or the vice versa where the client sends a diagnostic that we ignore.

So I think it's better to store the extension namespace in the data attribute.

[mhanberg]

These will need to be dispatched to the extension processes dynamically, as eventually libraries can provide their own extensions and we won't statically know what extensions exist or have their code loaded.

This is fine for now, it maybe leave a todo comment

[NJichev]

Maybe we can store the module name to dispatch to in the data attribute of the diagnostic? And libraries will have to obey the contract for this to work.
I think we can do something like:

def from(:elixir, data), do: NextLS.ElixirExtension.CodeAction.from(data)
def from(:credo, data), do: NextLS.CredoExtension.CodeAction.from(data)
def from(namespace, data), do: Module.concat(namespace, CodeAction).from(data)

Then it's going to be clear for external libraries what behaviour they need to implement.

I still like the static part being there since it's probably faster for the BEAM

[mhanberg]

The code might not even be loaded on the NextLS node, which is by dispatching it to the extension process will be required.

[NJichev]

Ah I see, then I'll leave it as a TODO comment.

[mhanberg]

I think this looks good for a starting place, so feel free to implement the actual code action now 👍

[NJichev]

I think I can remove this in favour of just using schematic

[mhanberg]

this is dope.

i didn't realize my neovim extension would show a diff preview of what the code action would do

[mhanberg]

I wonder if elixir core would consider tagging diagnostics with IDs, like EX1 means "unused variable" 🤔

[NJichev]

That would be dope

[mhanberg]

you are a rockstar 🎸