clangd / clangd

clangd language server
https://clangd.llvm.org
Apache License 2.0
1.51k stars 63 forks source link

unexpected empty list returned from `textDocument/codeAction` request #1126

Open siegel opened 2 years ago

siegel commented 2 years ago

I'm working out implementation of Code Actions from first principles (and the spec, for as much good as it does), and have gotten to the point where the communications seem to be working, but I'm not getting back expected results from clangd. (Which is, in honesty, the server I'm using to test and debug with, since it's proven to be the most reliable by far.)

Given this source file (its name ends in .mm so it's Objective-C++, but the test case contains only C++ code):

#include <climits>

class   SomeCppClass
{
private:
    int someIvar = 0;

public:
    SomeCppClass();
    ~SomeCppClass();

    int getIVar(void) const
        { return someIvar; }

    void setIVar(const int newVal)
        { someIvar = newVal; }
};

int
main (int argc, char **argv)
{
    SomeCppClass    *bar = new SomeCppClass;

    bar->setIVar(INT_MAX);
    delete bar;
}

If I select the first occurrence of "SomeCppClass" or place the insertion point in it and send a textDocument/codeAction request, I would expect clangd to respond with some non-empty list of things it can do at that point. (Perhaps "refactor".)

I immediately ran into a bug because I wasn't including the only parameter. The spec says that this parameter is optional, but looking at the clangd source code I see this:

  // Checks whether a particular CodeActionKind is included in the response.
  auto KindAllowed = [Only(Params.context.only)](llvm::StringRef Kind) {
    if (Only.empty())
      return true;
    return llvm::any_of(Only, [&](llvm::StringRef Base) {
      return Kind.consume_front(Base) && (Kind.empty() || Kind.startswith("."));
    });
  };

To me this reads as though clangd will return no code actions unless a non-empty only list is included; this seems like a bug to me but I'm not sure of what is expected here.

Continuing: even after sending a non-empty only parameter, I'm finding that clangd is continuing to return an empty list of actions.

The request I send is:

2022-04-25 14:17:46.836: LSPSendRequest(sync): textDocument/codeAction
    document:file:///Users/siegel/Desktop/LSPTest/main.mm
    parameters: {
    context =     {
        diagnostics =         (
        );
    };
    only =     (
        quickfix,
        info,
        source,
        refactor
    );
    range =     {
        end =         {
            character = 18;
            line = 2;
        };
        start =         {
            character = 6;
            line = 2;
        };
    };
}

I can verify that in other cases, my textDocument/codeAction requests return expected output; for example, when I have an overlapping diagnostic with an available quickfix action, I can request code actions and I'll get back something applicable. That tells me that in general the request I'm sending is well formed; but clearly it's lacking something that clangd is expecting to see, or else my expectations are completely out of whack.

Logs

clangd-codeaction.txt

System information

Output of clangd --version:

Homebrew clangd version 13.0.1
Features: mac+xpc
Platform: arm64-apple-darwin21.4.0

Editor/LSP plugin: BBEdit 14, engineering build

Operating system: macOS 12.3.1 (21E258), arm64

sam-mccall commented 2 years ago

We just don't have any actions here to show on C++ class names :-) Or rather, we do in trunk ("declare implicit copy/move members"), but not in the 13 or even 14 release.

Our set of code actions is not very extensive (apart from diagnostic fixes). You can see them here. Some examples to try:

To me this reads as though clangd will return no code actions unless a non-empty only list is included; this seems like a bug to me but I'm not sure of what is expected here.

I don't think that's right: KindAllowed evaluates to true if only is empty (we don't distinguish between empty-list and unset here). This means that code actions will be accepted regardless of their Kind, i.e. no filtering is performed in this case. We have a test that requests code actions without setting only.

siegel commented 2 years ago

We just don't have any actions here to show on C++ class names :-)

That's fair. :-) I tried some simple test cases: Objective-C class names, and argument names, and local variables.

I don't think that's right: KindAllowed evaluates to true if only is empty[...]

Great, thanks — I appreciate the explanation.

auto bar = new SomeCppClass, with cursor on auto

This did indeed return a proposed action.

a switch statement over an enum, without handling all cases, cursor on switch

This did not, but I may be missing something obvious.

At the very least I was expecting to see some refactoring options available, as shown in the "Refactoring" section of the Features page, which says:

Most symbols are renameable, such as classes, variables, functions and methods.

Is the documentation wrong there, or am I holding it wrong?

sam-mccall commented 2 years ago

a switch statement over an enum, without handling all cases, cursor on switch

This did not, but I may be missing something obvious.

Try this:

enum X {A, B, C};
void foo(X x) {
  switch(x) { } // cursor on 'switch'
}

At the very least I was expecting to see some refactoring options available, as shown in the "Refactoring" section of the Features page

The text there is under a "rename" subheading, this is a separate method textDocument/rename rather than code actions.

(These docs are out of date, we do have some other minor refactorings as code actions, like extract-variable, but they're not documented there yet).

siegel commented 2 years ago

Thanks for the explanation and pointers. I've got textDocument/rename working quite nicely now, so at least there's some progress. :-)

Is there a repository of samples/test cases for the tweaks that I can use to verify intended behavior?