OmniSharp / omnisharp-roslyn

OmniSharp server (HTTP, STDIO) based on Roslyn workspaces
MIT License
1.78k stars 419 forks source link

[LSP] Invalid returned CodeAction #2068

Open CGNonofr opened 3 years ago

CGNonofr commented 3 years ago

Here an example of a serialized CodeAction response provided by omnisharp-roslyn:

{
  "jsonrpc": "2.0",
  "id": 9,
  "result": [
    {
      "title": "Remove Unnecessary Usings",
      "kind": "refactor",
      "isPreferred": false,
      "diagnostics": [],
      "edit": {},
      "command": {
        "title": "Remove Unnecessary Usings",
        "command": "omnisharp/executeCodeAction",
        "arguments": [
          {
            "Uri": "file:///tmp/project/Main.cs",
            "Identifier": "Remove Unnecessary Usings",
            "Name": "Remove Unnecessary Usings",
            "Range": {
              "Start": {
                "Line": 6,
                "Character": 19
              },
              "End": {
                "Line": 6,
                "Character": 19
              }
            }
          }
        ]
      },
      "data": {
        "$$__handler_id__$$": "2d2f9bc2-5beb-4bb3-9b34-bd823e2cb425"
      }
    }
  ]
}

Refering to the documentation, edit should either be undefined or of type WorkspaceEdit

WorkspaceEdit A workspace edit represents changes to many resources managed in the workspace. The edit should either provide changes or documentChanges. If the client can handle versioned document edits and if documentChanges are present, the latter are preferred over changes.

but in the provided example, it's an empty object.

vscode then doesn't consider it as a CodeAction and then think it's a legacy Command instead of a CodeAction, starting from here, nothing is working

It seems related to https://github.com/OmniSharp/omnisharp-roslyn/blob/ebabfb88bebd2989bc5348f9c022241314f064b1/src/OmniSharp.LanguageServerProtocol/Handlers/OmniSharpCodeActionHandler.cs#L102 but I don't know much C#

In was probably broken in https://github.com/OmniSharp/omnisharp-roslyn/commit/96600de10ba16caed0379b1439d1cf5ff382e468#diff-8b5480f9e5910e05693e4a02d4ab416f6b006ec30673dea7bc16e2b919ab9f37L97

razzmatazz commented 3 years ago

HI @CGNonofr

The intent of 96600de was to workaround the issue where generating changesets for all the actions took too long on large projects. The design is such that we expect for the client to issue the Command specified to the server so the server can compute and push the changes back to the client when this command is executed (omnisharp/executeCodeAction). This workaround works on a couple of clients, emacs-lsp and nvim, IIRC. Is there something in vscode LSP client or w.r.t. this workaround that makes it unable to execute this Command?

The proper support for delayed code action resolution was added in 3.16#codeAction_resolve. We are waiting for the #2044 PR to be merged before we can start using it in omnisharp-roslyn.

CGNonofr commented 3 years ago

Hi @razzmatazz

It seems that not providing the edit field at all (instead of an empty object) should be fine

razzmatazz commented 3 years ago

I might not know the context, but I see the code is checking (short-circuiting) on the presence of the actual edit object too, in the line above, so providing it with a null or even undefined value would still not fix anything, right?

CGNonofr commented 3 years ago

This function is not called if edit is undefined:

razzmatazz commented 3 years ago

FYI @CGNonofr -- I have added a PR that does away with omnisharp/executeCodeAction and uses the new "Standard" codeAction/resolve functionality introduced in LSP 3.16 which should fix this issue..

CGNonofr commented 3 years ago

Thanks 👍 I'll give it a try!

CGNonofr commented 3 years ago

I just tried the v1.37.15 and the issue still doesn't seem to be fixed