omnisharp-roslyn icon indicating copy to clipboard operation
omnisharp-roslyn copied to clipboard

[LSP] Invalid returned CodeAction

Open CGNonofr opened this issue 4 years ago • 7 comments

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

CGNonofr avatar Jan 22 '21 16:01 CGNonofr

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.

razzmatazz avatar Jan 24 '21 10:01 razzmatazz

Hi @razzmatazz

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

CGNonofr avatar Jan 25 '21 08:01 CGNonofr

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?

  • https://github.com/microsoft/vscode-languageserver-node/blob/dae62de921d25964e8732411ca09e532dde992f5/types/src/main.ts#L1201

razzmatazz avatar Jan 25 '21 09:01 razzmatazz

This function is not called if edit is undefined:

  • https://github.com/microsoft/vscode-languageserver-node/blob/dae62de921d25964e8732411ca09e532dde992f5/types/src/main.ts#L2956

CGNonofr avatar Jan 25 '21 09:01 CGNonofr

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..

razzmatazz avatar May 02 '21 12:05 razzmatazz

Thanks 👍 I'll give it a try!

CGNonofr avatar May 02 '21 16:05 CGNonofr

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

CGNonofr avatar Sep 17 '21 13:09 CGNonofr