Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No delegateCommandHandler for java.apply.workspaceEdit #376

Open
idanarye opened this issue Oct 3, 2017 · 20 comments · Fixed by #1256 or #1278 · May be fixed by #2962
Open

No delegateCommandHandler for java.apply.workspaceEdit #376

idanarye opened this issue Oct 3, 2017 · 20 comments · Fixed by #1256 or #1278 · May be fixed by #2962

Comments

@idanarye
Copy link

idanarye commented Oct 3, 2017

I'm using jdt.ls via LanguageClient-neovim, and I'm trying to use code actions. This is my file:

public class App {
    public static void main(String[] args) {
        JFrame frame = new JFrame();
    }
}

JFrame is linted because I did not import it, and the LS suggests some code actions(this is from the log of the client - I just formatted the JSON lines to make them readable):

17:06:19 INFO    [MainThread] Begin textDocument/codeAction
17:06:19 DEBUG   [MainThread] =>
{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "textDocument/codeAction",
  "params": {
    "textDocument": {
      "uri": "file:///files/tests/java/45/src/main/java/App.java"
    },
    "range": {
      "start": {
        "line": 2,
        "character": 8
      },
      "end": {
        "line": 2,
        "character": 14
      }
    },
    "context": {
      "diagnostics": [
        {
          "range": {
            "start": {
              "line": 2,
              "character": 8
            },
            "end": {
              "line": 2,
              "character": 14
            }
          },
          "severity": 1,
          "code": "16777218",
          "source": "Java",
          "message": "JFrame cannot be resolved to a type"
        }
      ]
    }
  }
}
17:06:19 DEBUG   [RPC-java  ] <= {"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"Oct 3, 2017 5:05:52 PM \u003e\u003e document/codeAction"}}
17:06:19 DEBUG   [RPC-java  ] <=
{
  "jsonrpc": "2.0",
  "id": "1",
  "result": [
    {
      "title": "Change to 'JobName' (javax.print.attribute.standard)",
      "command": "java.apply.workspaceEdit",
      "arguments": [
        {
          "changes": {
            "file:///files/tests/java/45/src/main/java/App.java": [
              {
                "range": {
                  "start": {
                    "line": 0,
                    "character": 0
                  },
                  "end": {
                    "line": 0,
                    "character": 0
                  }
                },
                "newText": "import javax.print.attribute.standard.JobName;"
              },
              {
                "range": {
                  "start": {
                    "line": 0,
                    "character": 0
                  },
                  "end": {
                    "line": 0,
                    "character": 0
                  }
                },
                "newText": "\n\n"
              },
              {
                "range": {
                  "start": {
                    "line": 2,
                    "character": 8
                  },
                  "end": {
                    "line": 2,
                    "character": 8
                  }
                },
                "newText": "JobName"
              },
              {
                "range": {
                  "start": {
                    "line": 2,
                    "character": 8
                  },
                  "end": {
                    "line": 2,
                    "character": 14
                  }
                },
                "newText": ""
              }
            ]
          }
        }
      ]
    },
    {
      "title": "Import 'JFrame' (javax.swing)",
      "command": "java.apply.workspaceEdit",
      "arguments": [
        {
          "changes": {
            "file:///files/tests/java/45/src/main/java/App.java": [
              {
                "range": {
                  "start": {
                    "line": 0,
                    "character": 0
                  },
                  "end": {
                    "line": 0,
                    "character": 0
                  }
                },
                "newText": "import javax.swing.JFrame;"
              },
              {
                "range": {
                  "start": {
                    "line": 0,
                    "character": 0
                  },
                  "end": {
                    "line": 0,
                    "character": 0
                  }
                },
                "newText": "\n\n"
              }
            ]
          }
        }
      ]
    },
    {
      "title": "Add type parameter 'JFrame' to 'main(String[])'",
      "command": "java.apply.workspaceEdit",
      "arguments": [
        {
          "changes": {
            "file:///files/tests/java/45/src/main/java/App.java": [
              {
                "range": {
                  "start": {
                    "line": 1,
                    "character": 18
                  },
                  "end": {
                    "line": 1,
                    "character": 18
                  }
                },
                "newText": "<"
              },
              {
                "range": {
                  "start": {
                    "line": 1,
                    "character": 18
                  },
                  "end": {
                    "line": 1,
                    "character": 18
                  }
                },
                "newText": "JFrame"
              },
              {
                "range": {
                  "start": {
                    "line": 1,
                    "character": 18
                  },
                  "end": {
                    "line": 1,
                    "character": 18
                  }
                },
                "newText": "> "
              }
            ]
          }
        }
      ]
    }
  ]
}
17:06:19 DEBUG   [MainThread] state.codeActionCommands: [] =>
[
  {
    "title": "Change to 'JobName' (javax.print.attribute.standard)",
    "command": "java.apply.workspaceEdit",
    "arguments": [
      {
        "changes": {
          "file:///files/tests/java/45/src/main/java/App.java": [
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "import javax.print.attribute.standard.JobName;"
            },
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "\n\n"
            },
            {
              "range": {
                "start": {
                  "line": 2,
                  "character": 8
                },
                "end": {
                  "line": 2,
                  "character": 8
                }
              },
              "newText": "JobName"
            },
            {
              "range": {
                "start": {
                  "line": 2,
                  "character": 8
                },
                "end": {
                  "line": 2,
                  "character": 14
                }
              },
              "newText": ""
            }
          ]
        }
      }
    ]
  },
  {
    "title": "Import 'JFrame' (javax.swing)",
    "command": "java.apply.workspaceEdit",
    "arguments": [
      {
        "changes": {
          "file:///files/tests/java/45/src/main/java/App.java": [
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "import javax.swing.JFrame;"
            },
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "\n\n"
            }
          ]
        }
      }
    ]
  },
  {
    "title": "Add type parameter 'JFrame' to 'main(String[])'",
    "command": "java.apply.workspaceEdit",
    "arguments": [
      {
        "changes": {
          "file:///files/tests/java/45/src/main/java/App.java": [
            {
              "range": {
                "start": {
                  "line": 1,
                  "character": 18
                },
                "end": {
                  "line": 1,
                  "character": 18
                }
              },
              "newText": "<"
            },
            {
              "range": {
                "start": {
                  "line": 1,
                  "character": 18
                },
                "end": {
                  "line": 1,
                  "character": 18
                }
              },
              "newText": "JFrame"
            },
            {
              "range": {
                "start": {
                  "line": 1,
                  "character": 18
                },
                "end": {
                  "line": 1,
                  "character": 18
                }
              },
              "newText": "> "
            }
          ]
        }
      }
    ]
  }
]
17:06:19 INFO    [MainThread] End textDocument/codeAction

So I pick Import 'JFrame' (javax.swing):

17:06:44 INFO    [MainThread] Begin workspace/executeCommand
17:06:44 DEBUG   [MainThread] =>
{
  "jsonrpc": "2.0",
  "id": 2,
  "method": "workspace/executeCommand",
  "params": {
    "command": "java.apply.workspaceEdit",
    "arguments": [
      {
        "changes": {
          "file:///files/tests/java/45/src/main/java/App.java": [
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "import javax.print.attribute.standard.JobName;"
            },
            {
              "range": {
                "start": {
                  "line": 0,
                  "character": 0
                },
                "end": {
                  "line": 0,
                  "character": 0
                }
              },
              "newText": "\n\n"
            },
            {
              "range": {
                "start": {
                  "line": 2,
                  "character": 8
                },
                "end": {
                  "line": 2,
                  "character": 8
                }
              },
              "newText": "JobName"
            },
            {
              "range": {
                "start": {
                  "line": 2,
                  "character": 8
                },
                "end": {
                  "line": 2,
                  "character": 14
                }
              },
              "newText": ""
            }
          ]
        }
      }
    ]
  }
}
17:06:44 DEBUG   [RPC-java  ] <= {"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"Oct 3, 2017 5:05:52 PM \u003e\u003e workspace/executeCommand"}}
17:06:44 DEBUG   [RPC-java  ] <= {"jsonrpc":"2.0","id":"2","error":{"code":-32601,"message":"No delegateCommandHandler for java.apply.workspaceEdit"}}
17:06:44 ERROR   [MainThread] {"jsonrpc": "2.0", "id": "2", "error": {"code": -32601, "message": "No delegateCommandHandler for java.apply.workspaceEdit"}}
17:06:44 INFO    [MainThread] End workspace/executeCommand

And get the error message - No delegateCommandHandler for java.apply.workspaceEdit

The LSP specs does not seem to refer to delegateCommandHandlers, and from #346 it looks like it's jdt.ls' extension - but I don't see any instructions on how to use it.

So... what do I need to do to add support for it in a client?

@fbricon
Copy link
Contributor

fbricon commented Oct 3, 2017

Yeah documenting jdt.ls specific commands is in our backlog (#314), sorry about that.

java.apply.workspaceEdit is not related to delegateCommandHandlers actually (those are server side extensions). It's a command that needs to be implemented by clients. See how it's done in vscode-java:
https://github.com/redhat-developer/vscode-java/blob/9b0f0aca80cbefabad4c034fb5dd365d029f6170/src/extension.ts#L155-L160

Basically, the server sends workspace edits to the client, and the client is responsible for applying them.

@idanarye
Copy link
Author

idanarye commented Oct 3, 2017

Isn't that how code actions work? Why do you need a special command for that?

@autozimu
Copy link

autozimu commented Nov 4, 2017

I think the issue is that protocol doesn't specify clearly how client is going to consume a Command(https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#command) after user select one, which could have at least two cases here,

  1. client sends the selected Command to server, server responds back with workspaceEdit, then client applies those edits
  2. client itself figures out how to apply edits based on selected Command

sourcegraph/javascript-typescript-langserver chosen option 1 and LanguageClient-neovim followed the same pattern. And it looks here eclipse.jdt did option 2.

@idanarye
Copy link
Author

idanarye commented Nov 4, 2017

So much for having a unified protocol that solves the matrix problem...

@gorkem
Copy link
Contributor

gorkem commented Feb 13, 2018

The main difference between 1 and 2 is when the TextEdits are the calculated otherwise both require a custom command to be implemented by the client. 2nd way works well for jdt.ls at the moment because the way we determine code actions gets us very close to calculating TextEdits anyway hence more efficient.

@amiramw
Copy link
Contributor

amiramw commented Feb 14, 2018

But option 2 doesn't conform with the protocol.
https://microsoft.github.io/language-server-protocol/specification#textDocument_codeAction

When the command is selected the server should be contacted again (via the workspace/executeCommand) request to execute the command.

If the need for option 2 is real shouldn't it be pushed to the protocol?

@natebosch
Copy link

There is a proposal to support returning an edit instead of a command: microsoft/language-server-protocol#178

@tsmaeder
Copy link
Contributor

But option 2 doesn't conform with the protocol.
https://microsoft.github.io/language-server-protocol/specification#textDocument_codeAction

When the command is selected the server should be contacted again (via the workspace/executeCommand) request to execute the command.

If the need for option 2 is real shouldn't it be pushed to the protocol?

This provision was added AFTER jdt.ls implemented code actions this way. So "option 2" was at one time in the protocol, but was removed. We should probably move to using WorkspaceEdits, now that the can contain resource changes, but that would break existing clients.

@gns26
Copy link

gns26 commented Jan 17, 2019

Hi guys - any update on this one? I see that lots of other clients are busy implementing custom commands (i.e. autozimu/LanguageClient-neovim#158) which as mentioned above is unnecessary.

Won't most clients be able to handle WorkspaceEdits now? If so then moving across should be fine?

Cheers

@puremourning
Copy link
Contributor

Won't most clients be able to handle WorkspaceEdits now?

What makes you think that?

@gns26
Copy link

gns26 commented Jan 17, 2019

Won't most clients be able to handle WorkspaceEdits now?

What makes you think that?

I saw the linked ticket had been closed back in August. But if most have not then it would have to wait, yes. (https://github.com/prabirshrestha/vim-lsp this one for instance does seem to)

@AObuchow
Copy link

Was testing JDT-LS integration with LSP4E and ran into this issue as well.
Would it be possible to replace usage of "java.apply.workspaceEdit" in codeAction with workspaceEdit?
It seems to be mentioned in CodeActionHandler.java:

if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(proposal.getKind())) {
			// TODO: Should set WorkspaceEdit directly instead of Command
			CodeAction codeAction = new CodeAction(name);
			codeAction.setKind(proposal.getKind());
			codeAction.setCommand(command);
			codeAction.setDiagnostics(context.getDiagnostics());
			return Optional.of(Either.forRight(codeAction));
		} else {
			return Optional.of(Either.forLeft(command));
		}

Also, could we retitle this issue to something like Replace usage of java.apply.workspaceEdit in codeAction by workspaceEdit?

fbricon pushed a commit that referenced this issue Nov 13, 2019
If client advertises `CodeActionLiteralSupport`, using that for
`java.apply.workspaceEdit` would allow clients to use a generic
algorithm, instead of being forced to provide a special case for jdt.ls.

Fixes #376

Signed-off-by: Boris Staletic <boris.staletic@gmail.com>
@fbricon
Copy link
Contributor

fbricon commented Nov 13, 2019

The fix was reverted because of a regression. See #1256 (comment)

@nrabulinski
Copy link

Are there any plans to fix this behavior?

@theli-ua
Copy link

@fbricon given that you looked at previous attempts at fixing this could you take a look at linked PR, it should leave this compatible with vscode while allowing compliant clients to work as well. It basically does what @bstaletic suggested above 3 years ago

@rgrunber rgrunber added this to the End November 2023 milestone Nov 1, 2023
@rgrunber rgrunber self-assigned this Nov 8, 2023
rgrunber pushed a commit to theli-ua/eclipse.jdt.ls that referenced this issue Nov 9, 2023
- Fixes eclipse-jdtls#376
- This follows exactly same pattern as organizeImports was done.

Signed-off-by: Anton Romanov <theli.ua@gmail.com>
theli-ua pushed a commit to theli-ua/eclipse.jdt.ls that referenced this issue Nov 16, 2023
If client advertises `CodeActionLiteralSupport`, using that for
`java.apply.workspaceEdit` would allow clients to use a generic
algorithm, instead of being forced to provide a special case for jdt.ls.

Fixes eclipse-jdtls#376

Signed-off-by: Boris Staletic <boris.staletic@gmail.com>
Signed-off-by: Anton Romanov <theli.ua@gmail.com>
@rgrunber rgrunber modified the milestones: End April 2024, End May 2024 Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment