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 25, 2024
1 parent e74bed9 commit 9515f73
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 89 deletions.
12 changes: 9 additions & 3 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 @@ -324,7 +324,13 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
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());
}

}
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);
}
}
}
110 changes: 92 additions & 18 deletions ...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.LanguageServerWorkingCopyOwner;
import org.eclipse.jdt.ls.core.internal.ProjectUtils;
import org.eclipse.jdt.ls.core.internal.ResourceUtils;
import org.eclipse.jdt.ls.core.internal.WorkspaceHelper;
import org.eclipse.jdt.ls.core.internal.codemanipulation.AbstractSourceTestCase;
import org.eclipse.jdt.ls.core.internal.correction.AbstractQuickFixTest;
Expand Down Expand Up @@ -120,8 +121,34 @@ public void testCodeAction_removeUnusedImport() throws Exception{
List<Either<Command, CodeAction>> organizeImportActions = findActions(codeActions, CodeActionKind.SourceOrganizeImports);
Assert.assertNotNull(organizeImportActions);
Assert.assertEquals(1, organizeImportActions.size());
Command c = codeActions.get(0).getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
WorkspaceEdit e = codeActions.get(0).getRight().getEdit();
Assert.assertNotNull(e);
}

@Test
public void testCodeActionLiteral_removeUnusedImport() throws Exception{
when(preferenceManager.getClientPreferences().isSupportedCodeActionKind(CodeActionKind.QuickFix)).thenReturn(true);
ICompilationUnit unit = getWorkingCopy(
"src/java/Foo.java",
"import java.sql.*; \n" +
"public class Foo {\n"+
" void foo() {\n"+
" }\n"+
"}\n");

CodeActionParams params = new CodeActionParams();
params.setTextDocument(new TextDocumentIdentifier(JDTUtils.toURI(unit)));
final Range range = CodeActionUtil.getRange(unit, "java.sql");
params.setRange(range);
params.setContext(new CodeActionContext(Arrays.asList(getDiagnostic(Integer.toString(IProblem.UnusedImport), range))));
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
Assert.assertNotNull(codeActions);
Assert.assertTrue(codeActions.size() >= 3);
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(1).getRight().getKind(), JavaCodeActionKind.QUICK_ASSIST);
Assert.assertEquals(codeActions.get(2).getRight().getKind(), CodeActionKind.SourceOrganizeImports);
WorkspaceEdit w = codeActions.get(0).getRight().getEdit();
Assert.assertNotNull(w);
}

@Test
Expand Down Expand Up @@ -192,13 +219,13 @@ public void testCodeAction_organizeImportsQuickFix() throws Exception {

Assert.assertNotNull(codeActions);
List<Either<Command, CodeAction>> quickAssistActions = findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description));
Assert.assertTrue(CodeActionHandlerTest.titleExists(quickAssistActions, CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description));
// Test if the quick assist exists only for type declaration
params = CodeActionUtil.constructCodeActionParams(unit, "String bar");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertFalse(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description));
Assert.assertFalse(CodeActionHandlerTest.titleExists(quickAssistActions, CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description));
}

@Test
Expand Down Expand Up @@ -403,8 +430,32 @@ public void testCodeAction_removeUnterminatedString() throws Exception{
Assert.assertNotNull(codeActions);
Assert.assertFalse(codeActions.isEmpty());
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
Command c = codeActions.get(0).getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
WorkspaceEdit e = codeActions.get(0).getRight().getEdit();
Assert.assertNotNull(e);
}

@Test
public void testCodeActionLiteral_removeUnterminatedString() throws Exception{
when(preferenceManager.getClientPreferences().isSupportedCodeActionKind(CodeActionKind.QuickFix)).thenReturn(true);
ICompilationUnit unit = getWorkingCopy(
"src/java/Foo.java",
"public class Foo {\n"+
" void foo() {\n"+
"String s = \"some str\n" +
" }\n"+
"}\n");

CodeActionParams params = new CodeActionParams();
params.setTextDocument(new TextDocumentIdentifier(JDTUtils.toURI(unit)));
final Range range = CodeActionUtil.getRange(unit, "some str");
params.setRange(range);
params.setContext(new CodeActionContext(Arrays.asList(getDiagnostic(Integer.toString(IProblem.UnterminatedString), range))));
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
Assert.assertNotNull(codeActions);
Assert.assertFalse(codeActions.isEmpty());
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
WorkspaceEdit w = codeActions.get(0).getRight().getEdit();
Assert.assertNotNull(w);
}

@Test
Expand Down Expand Up @@ -655,7 +706,7 @@ public void testQuickAssistForImportDeclarationOrder() throws JavaModelException
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Organize imports");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Organize imports");
}

@Test
Expand All @@ -672,10 +723,10 @@ public void testQuickAssistForFieldDeclarationOrder() throws JavaModelException
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Generate Getter and Setter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(1)).getTitle(), "Generate Getter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(2)).getTitle(), "Generate Setter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(3)).getTitle(), "Generate Constructors...");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Generate Getter and Setter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(1)), "Generate Getter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(2)), "Generate Setter for 'name'");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(3)), "Generate Constructors...");
}

@Test
Expand All @@ -692,13 +743,13 @@ public void testQuickAssistForTypeDeclarationOrder() throws JavaModelException {
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Generate Getters and Setters");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(1)).getTitle(), "Generate Getters");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(2)).getTitle(), "Generate Setters");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(3)).getTitle(), "Generate Constructors...");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(4)).getTitle(), "Generate hashCode() and equals()...");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(5)).getTitle(), "Generate toString()...");
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(6)).getTitle(), "Override/Implement Methods...");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Generate Getters and Setters");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(1)), "Generate Getters");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(2)), "Generate Setters");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(3)), "Generate Constructors...");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(4)), "Generate hashCode() and equals()...");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(5)), "Generate toString()...");
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(6)), "Override/Implement Methods...");
}

private List<Either<Command, CodeAction>> getCodeActions(CodeActionParams params) {
Expand All @@ -709,6 +760,17 @@ public static Command getCommand(Either<Command, CodeAction> codeAction) {
return codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
}

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

public static WorkspaceEdit getEdit(Either<Command, CodeAction> codeAction) {
assertTrue(codeAction.isRight());
assertNotNull(codeAction.getRight().getEdit());
return codeAction.getRight().getEdit();
}

private Diagnostic getDiagnostic(String code, Range range){
Diagnostic $ = new Diagnostic();
$.setCode(code);
Expand Down Expand Up @@ -769,4 +831,16 @@ public static boolean commandExists(List<Either<Command, CodeAction>> codeAction
}
return false;
}

public static boolean titleExists(List<Either<Command, CodeAction>> codeActions, String title) {
if (codeActions.isEmpty()) {
return false;
}
for (Either<Command, CodeAction> codeAction : codeActions) {
if (title.equals(getTitle(codeAction))) {
return true;
}
}
return false;
}
}
Loading

0 comments on commit 9515f73

Please sign in to comment.