Skip to content

Commit

Permalink
Take advantage of CodeActionLiteral client capability.
Browse files Browse the repository at this point in the history 
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>
Signed-off-by: Anton Romanov <theli.ua@gmail.com>
Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
  • Loading branch information
@bstaletic @rgrunber
bstaletic authored and rgrunber committed Sep 26, 2024
1 parent 46da1a8 commit b06c7c6
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 123 deletions.
13 changes: 9 additions & 4 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
String name = proposal.getName();

Command command = null;
WorkspaceEdit edit = null;
if (proposal instanceof CUCorrectionCommandProposal commandProposal) {
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else if (proposal instanceof RefactoringCorrectionCommandProposal commandProposal) {
Expand All @@ -296,7 +297,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else {
if (!this.preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
WorkspaceEdit edit = ChangeUtil.convertToWorkspaceEdit(proposal.getChange());
edit = ChangeUtil.convertToWorkspaceEdit(proposal.getChange());
if (!ChangeUtil.hasChanges(edit)) {
return Optional.empty();
}
Expand All @@ -305,7 +306,6 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
}

if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(pk.getKind())) {
// TODO: Should set WorkspaceEdit directly instead of Command
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(pk.getKind());
if (command == null) { // lazy resolve the edit.
Expand All @@ -323,8 +323,13 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
// The relevance is in descending order while CodeActionComparator sorts in ascending order
codeAction.setData(new CodeActionData(proposal, -proposal.getRelevance()));
} else {
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, -proposal.getRelevance()));
if (edit != null) {
// no code action resolve support, but have code action literal support
codeAction.setEdit(edit);
} else {
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, -proposal.getRelevance()));
}
}
if (pk.getKind() != JavaCodeActionKind.QUICK_ASSIST) {
codeAction.setDiagnostics(context.getDiagnostics());
Expand Down
2 changes: 1 addition & 1 deletion ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ public boolean isHierarchicalDocumentSymbolSupported() {
}

/**
* {@code true} if the client has explicitly set the
* {@code true} if the client has listed {@code kind} in
* {@code textDocument.codeAction.codeActionLiteralSupport.codeActionKind.valueSet}
* value. Otherwise, {@code false}.
*/
Expand Down
10 changes: 4 additions & 6 deletions ...t.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(kind);
codeAction.setData(new CodeActionData(proposal, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
WorkspaceEdit edit;
Expand All @@ -632,9 +632,8 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(kind)) {
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
codeAction.setEdit(edit);
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
return Optional.of(Either.forLeft(command));
Expand Down Expand Up @@ -713,8 +712,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActi
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(kind)) {
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, priority));
codeAction.setEdit(edit);
codeAction.setDiagnostics(context.getDiagnostics());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down
21 changes: 15 additions & 6 deletions ...se.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/AbstractQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,19 @@ protected String evaluateCodeActionCommand(Either<Command, CodeAction> codeActio
throws BadLocationException, JavaModelException {

Command c = codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
Assert.assertNotNull(c.getArguments());
Assert.assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
WorkspaceEdit we = (WorkspaceEdit) c.getArguments().get(0);
return evaluateWorkspaceEdit(we);
if (c != null) {
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
Assert.assertNotNull(c.getArguments());
Assert.assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
WorkspaceEdit we = (WorkspaceEdit) c.getArguments().get(0);
return evaluateWorkspaceEdit(we);
} else {
WorkspaceEdit we = codeAction.getRight().getEdit();
if (we.getDocumentChanges() != null) {
return evaluateChanges(we.getDocumentChanges());
}
return evaluateChanges(we.getChanges());
}
}

public static String evaluateWorkspaceEdit(WorkspaceEdit edit) throws JavaModelException, BadLocationException {
Expand Down Expand Up @@ -367,7 +375,8 @@ public Command getCommand(Either<Command, CodeAction> codeAction) {
}

public String getTitle(Either<Command, CodeAction> codeAction) {
return ResourceUtils.dos2Unix(getCommand(codeAction).getTitle());
Command command = getCommand(codeAction);
return ResourceUtils.dos2Unix(command != null ? command.getTitle() : codeAction.getRight().getTitle());
}

}
13 changes: 5 additions & 8 deletions ...s/src/org/eclipse/jdt/ls/core/internal/correction/ConvertMethodReferenceToLambdaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -75,10 +76,8 @@ public void testMethodReferenceToLambda() throws Exception {
CodeAction action = codeAction.getRight();
assertEquals(JavaCodeActionKind.QUICK_ASSIST, action.getKind());
assertEquals("Convert to lambda expression", action.getTitle());
Command c = action.getCommand();
assertEquals("java.apply.workspaceEdit", c.getCommand());
assertNotNull(c.getArguments());
assertEquals("Convert to lambda expression", c.getTitle());
WorkspaceEdit edit = action.getEdit();
assertNotNull(edit);
}

@Test
Expand All @@ -102,10 +101,8 @@ public void testLambdaToMethodReference() throws Exception {
CodeAction action = codeAction.getRight();
assertEquals(JavaCodeActionKind.QUICK_ASSIST, action.getKind());
assertEquals("Clean up lambda expression", action.getTitle());
Command c = action.getCommand();
assertEquals("java.apply.workspaceEdit", c.getCommand());
assertNotNull(c.getArguments());
assertEquals("Clean up lambda expression", c.getTitle());
WorkspaceEdit edit = action.getEdit();
assertNotNull(edit);
}

}
7 changes: 2 additions & 5 deletions ...tests/src/org/eclipse/jdt/ls/core/internal/correction/IntroduceParameterRefactorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.eclipse.jdt.core.IPackageFragmentRoot;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler;
import org.eclipse.jdt.ls.core.internal.preferences.ClientPreferences;
import org.eclipse.jdt.ls.core.internal.text.correction.ActionMessages;
import org.eclipse.jdt.ls.core.internal.text.correction.RefactorProposalUtility;
Expand Down Expand Up @@ -104,10 +103,8 @@ private void testIntroduceParameterAction(ICompilationUnit cu, Range range) thro
CodeAction action = codeAction.getRight();
assertEquals(JavaCodeActionKind.REFACTOR_INTRODUCE_PARAMETER, action.getKind());
assertEquals("Introduce Parameter...", action.getTitle());
Command c = action.getCommand();
assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
assertEquals(1, c.getArguments().size());
WorkspaceEdit workspaceEdit = (WorkspaceEdit) c.getArguments().get(0);
WorkspaceEdit workspaceEdit = action.getEdit();
assertNotNull(workspaceEdit);
Map<String, List<TextEdit>> changes = workspaceEdit.getChanges();
List<TextEdit> textEdits = changes.values().iterator().next();
assertEquals(1, textEdits.size());
Expand Down
8 changes: 1 addition & 7 deletions ...jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/MissingEnumQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.eclipse.jdt.core.IPackageFragmentRoot;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler;
import org.eclipse.jdt.ls.core.internal.text.correction.ActionMessages;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionKind;
Expand All @@ -35,7 +34,6 @@
import org.eclipse.lsp4j.TextEdit;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

Expand Down Expand Up @@ -136,11 +134,7 @@ public void testMissingEnumConstantDespiteDefault() throws Exception {
}

private TextEdit getTextEdit(Either<Command, CodeAction> codeAction) {
Command c = codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
Assert.assertNotNull(c.getArguments());
Assert.assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
WorkspaceEdit we = (WorkspaceEdit) c.getArguments().get(0);
WorkspaceEdit we = codeAction.getRight().getEdit();
Iterator<Entry<String, List<TextEdit>>> editEntries = we.getChanges().entrySet().iterator();
Entry<String, List<TextEdit>> entry = editEntries.next();
TextEdit edit = entry.getValue().get(0);
Expand Down
16 changes: 11 additions & 5 deletions ...lipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/ReorgQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,16 @@ private Either<Command, CodeAction> findAction(List<Either<Command, CodeAction>>

private WorkspaceEdit getWorkspaceEdit(Either<Command, CodeAction> codeAction) {
Command c = codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
assertNotNull(c.getArguments());
assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
return (WorkspaceEdit) c.getArguments().get(0);
if (c != null) {
assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
assertNotNull(c.getArguments());
assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
return (WorkspaceEdit) c.getArguments().get(0);
} else {
WorkspaceEdit e = (WorkspaceEdit) codeAction.getRight().getEdit();
assertNotNull(e);
return e;
}
}

private void assertRenameFileOperation(Either<Command, CodeAction> codeAction, String newUri) {
Expand Down Expand Up @@ -455,4 +461,4 @@ public void testWrongTypeNameInAnnot() throws Exception {
Expected e1 = new Expected("Rename type to 'E'", buf.toString());
assertCodeActions(cu, e1);
}
}
}
2 changes: 1 addition & 1 deletion ...ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/UnresolvedTypesQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ public void testTypeCreation() throws Exception {

assertCodeActionExists(cu, e1);
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu);
WorkspaceEdit edit = (WorkspaceEdit) codeActions.get(0).getRight().getCommand().getArguments().get(0);
WorkspaceEdit edit = codeActions.get(0).getRight().getEdit();
assertNotNull(edit.getDocumentChanges());
ResourceOperation resourceOperation = edit.getDocumentChanges().get(0).getRight();
assertNotNull(resourceOperation);
Expand Down
Loading

0 comments on commit b06c7c6

Please sign in to comment.