[PATCH] D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics

Thu Mar 4 05:01:59 PST 2021

sammccall added a comment.

In D96245#2602896 <https://reviews.llvm.org/D96245#2602896>, @kadircet wrote:

> Agreed, I was going this way because currently there was no way to provide a code action to clangd::diagnostic apart from storing it in the fixes map while generating it. I suppose it is more sensible to just go with a new hook on modules that can provide a codeaction given a clangd::diagnostic.

I think this is what you meant, but just to be sure... there's no fundamental need in LSP to make any association between code actions and diagnostics until a codeAction request comes in.
There's no need to put anything into FixesMap. (We do have an extension/C++ API that exposes a different model, but as discussed offline I think we should just not expose this through the extension for now)

> Does `std::vector<CodeAction> contributeActions(const CodeActionParams&);` look like a good signature for the module hook?

A couple of issues:

- this will require modules to duplicate protocol details (e.g. CodeActionContext.only)
- many modules that want to provide code actions will want an AST, and we build one for the codeAction request, we should be able to share it
- it's not obvious how to use these without ClangdLSPServer if the CodeAction wraps a Command whose handler must be bound

We have the Tweak infrastructure that's basically designed to solve all these problems. It has some issues (like you can only return one fix per "class") but these are fixable and would be nice to have fixed in any case.
This means we'd be using a fixed LSP applyTweak command rather than binding a custom command for this action as planned, but it might work well?

Interface would just look like `contributeTweaks(vector<unique_ptr<Tweak>>&)` or something

WDYT

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96245/new/

https://reviews.llvm.org/D96245