Take advantage of CodeActionLiteral client capability

[theli-ua]

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
This is essentially a rebase of 1278
Superseeds #2688

Since the original reason for revert is now addressed by #1657

[eclipse-ls-bot]

Can one of the admins verify this patch?

[krobelus]

I've tested kak-lsp on this and it does work. Thanks so much!

There is an interesting issue which I'll try to fix soon in kak-lsp:
jdtls sends a code action containing both an edit and a command.
LSP says

/**

• If a code action provides an edit and a command, first the edit is
• executed and then the command.
  */

Which would be wrong here because the edit and command do the same thing, but they are not idempotent.

However jdtls does not include java.apply.workspaceEdit in the methods it supports in workspace/executeCommand (executeCommandProvider).
I guess kak-lsp should recognize this and not send java.apply.workspaceEdit, thus avoiding the error.
Maybe I'll add a warning to the logs. I don't think LSP specifies what should happen in this case.

Here is the code action sent by jdtls:

{
  "jsonrpc": "2.0",
  "id": 11,
  "result": [
    {
      "title": "Insert missing quote",
      "kind": "quickfix",
      "diagnostics": [
        {
          "range": {
            "start": {
              "line": 5,
              "character": 15
            },
            "end": {
              "line": 5,
              "character": 24
            }
          },
          "severity": 1,
          "code": "1610612995",
          "source": "Java",
          "message": "String literal is not properly closed by a double-quote"
        }
      ],
      "edit": {
        "changes": {},
        "documentChanges": [
          {
            "textDocument": {
              "version": null,
              "uri": "file:///home/johannes/git/kak-lsp/t/java/test.java"
            },
            "edits": [
              {
                "range": {
                  "start": {
                    "line": 5,
                    "character": 24
                  },
                  "end": {
                    "line": 5,
                    "character": 24
                  }
                },
                "newText": "\""
              }
            ]
          }
        ]
      },
      "command": {
        "title": "Insert missing quote",
        "command": "java.apply.workspaceEdit",
        "arguments": [
          {
            "changes": {},
            "documentChanges": [
              {
                "textDocument": {
                  "version": null,
                  "uri": "file:///home/johannes/git/kak-lsp/t/java/test.java"
                },
                "edits": [
                  {
                    "range": {
                      "start": {
                        "line": 5,
                        "character": 24
                      },
                      "end": {
                        "line": 5,
                        "character": 24
                      }
                    },
                    "newText": "\""
                  }
                ]
              }
            ]
          }
        ]
      }
    },
  ]
}

[krobelus]

There's still a bunch of other places where jdtls requires executeCommand of java.apply.workspaceEdit (for example the "add JavaDoc" code action.
When I add a hack to kak-lsp to support this command (via workspace/applyEdit) that means that the code action from the earlier comment will add two quotes instead of just one.

Can we convert those other places to also use codeAction.edit instead of only the java.apply.workspaceEdit command?

[krobelus]

I was able to make "Generate toString()" work with this patch but not yet the "Add Javadoc" one.

diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
index 5bfd515a..66852dd0 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
@@ -482,6 +482,12 @@ public class ClientPreferences {
 			&& capabilities.getWorkspace().getWorkspaceEdit().getChangeAnnotationSupport() != null;
     }
 
+	public boolean isWorkspaceEditSupport() {
+		return v3supported
+			&& capabilities.getWorkspace() != null
+			&& capabilities.getWorkspace().getWorkspaceEdit() != null;
+    }
+
 	public boolean skipProjectConfiguration() {
 		return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("skipProjectConfiguration", "false").toString());
 	}
diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
index c4b10686..ddbb4ba9 100644
--- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
+++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
@@ -704,8 +704,12 @@ public class SourceAssistProcessor {
 			if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(kind)) {
 				CodeAction codeAction = new CodeAction(name);
 				codeAction.setKind(kind);
-				codeAction.setCommand(command);
-				codeAction.setData(new CodeActionData(null, priority));
+				if (preferenceManager.getClientPreferences().isWorkspaceEditSupport()) {
+					codeAction.setEdit(edit);
+				} else {
+					codeAction.setCommand(command);
+					codeAction.setData(new CodeActionData(null, priority));
+				}
 				codeAction.setDiagnostics(context.getDiagnostics());
 				return Optional.of(Either.forRight(codeAction));
 			} else {

[krobelus]

if edit != null this line means that we still include the nonstandard command.
By kak-lsp default shows an error in this case (see my other comment).
Can we delete this line so it's not sent in this case?

[jnturton]

Hi, is this PR still active? It would be great to get this working...

[rgrunber]

Unless I'm missing something obvious, I think I agree with this change, and it looks completely safe to me. There might be a few places where we need to fix it up.

First of all, this change wouldn't affect vscode-java client at all. In SourceAssistProcessor and CodeActionHandler, this change attempts to change behaviour when isResolveCodeActionSupported() is false (it's true for vscode-java). So we're fine here.

The change sets the workspace edit of a code action where isSupportedCodeActionKind(..) is true. That setting implies code action literal support. Where the support is not present, we keep the current behaviour. To me that seems fine and if clients are setting code action literal support, they should expect to deal with the edits directly (no commands).