Introduce CodeActions

[chadhietala]

What Does This Do?

In practice Glint is language server that translates Ember templates and concepts into TypeScript code that then is typechecked using the off shelf TypeScript typechecker. If there are errors during the checking, text offsets of those errors are translated back into the original text offsets (in Ember templates and concepts) so that the developer can resolve the type errors.

This PR starts to plumb through more capabilities for the Glint language server, specifically we're starting with CodeActions. CodeActions Requests are sent from the client to server to get for fixes for problems in code including type fixes. Since Glint utilizes the underlying TypeScript infrastructure we can leverage it to ask for CodeFixes that can then be applied to Ember code. This PR addresses 3 tricky areas:

1. The TS Server does not use Language Server Protocol despite that is where LSP originated from so we must convert between ts.CodeFix and lsp.CodeAction
2. Translation of the fixes must occur before they are applied
3. We should leverage existing formatters that extension is aware of

This PR sets the ground work for these 3 areas.

[dfreeman]

I haven't gone deep into the implementation details yet since it's still WIP, but I've left a few comments/questions about the high level approach here. Thank you so much for taking this on, @chadhietala!

[chadhietala]

@dfreeman I think this is ready for review now. I ended up removing all the prettier things, simplified the config manager, consolidated the document related stuff.

I wrote smoke tests for all the environments except loose mode. It isn't clear to me if its even possible to correctly apply the text changes due to how transformation works.

Screen.Recording.2023-01-06.at.5.25.57.PM.mov

[dfreeman]

Great, thank you! I should have time to give this a review tomorrow.

[dfreeman]

I wrote smoke tests for all the environments except loose mode. It isn't clear to me if its even possible to correctly apply the text changes due to how transformation works.

Can you say more about what it is that makes actions for loose-mode templates complex? Is it that they're (usually) in a separate file from their backing class?

My uneducated guess would have been that doing things like inserting a {{! @glint-ignore }} would be possible, even if there are reasons that more complex actions like "add missing field bar" may be more difficult to account for.

[dfreeman]

@chadhietala I want to reiterate that I'm excited to see this land, and I'm super grateful for your work on it! 🙏

If there's anything here you want to talk through, feel free to ping me on Discord as well if that's easier 🙂

[chadhietala]

@dfreeman Sorry for the back and forth on this. I went back over all your comments and I think I've landed pretty close to what you had in mind. I was also able to get loose mode working. I have noted a couple areas that I think require your thoughts.

[chadhietala]

This is now just used for the binding. I noticed that this could be used for completions as well but it would need to be pulled through.

[chadhietala]

Generally curious about your thoughts here about passing the kind. For the vscode binding we only let QuickFixs requests but I'm trying to balance the fact that you can call getCodeActions programmatically. Since we don't support all codefix types it seems like we need something like this. Over time this can be dynamic.

[dfreeman]

If I'm reading this right, this means that when the editor sends a request for any kind of code actions, we'll always respond with quickfixes?

You've spent a lot more time thinking about this than I have, so I'm probably missing something, but my kneejerk reaction would be to say it's better to return an empty response for an action request we don't support than to give back something that the editor didn't ask for. Why doesn't the short-circuit you have in the language server already handle this the way we'd want?

[chadhietala]

@dfreeman we solicit to vscode that we will only support QuickFix so we should only get requests we can fulfill. The short circuit is there in the event you're using the server from analyzeProject directly.

[dfreeman]

If a well-behaved editor will only ever ask us for quickfixes, why do we need to hard-code QuickFix here?

[chadhietala]

I spent a day looking as to why context.only is optional and not always passed for CodeFix requests but found nothing. All implementations of language servers that I found (Svelte, Vue), that provide codefixes do a narrowing like:

let kind = '';

if (context.only === undefined) {
  kind = CodeActionKind.QuickFix;
} else if (context.only.includes(CodeActionKind.QuickFix)) {
  kind = CodeActionKind.QuickFix;
}

I can add this if you think it will be clearer.

[dfreeman]

Thanks, that's super helpful context! I think something along those lines would help make it clearer for anyone (read: me in the future when I've forgotten this conversation 😅) who comes along and reads it later

[chadhietala]

Sounds good. I pushed another commit that does ^ and explains whats going on with context.only

[dfreeman]

I think once we get #499 (comment) settled this will be ready to go. I should have better availability for a while from here forward, so hopefully we can land this in the next day or two—apologies again for how long it's taken!

[dfreeman]

🎉 Thank you again for all your hard work here!