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>
  • Loading branch information
@bstaletic @theli-ua
bstaletic authored and theli-ua committed Nov 16, 2023
1 parent c9b550b commit c93f930
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 26 deletions.
11 changes: 8 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 @@ -264,6 +264,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 @@ -272,7 +273,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 @@ -281,7 +282,6 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(String u
}

if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(proposal.getKind())) {
// TODO: Should set WorkspaceEdit directly instead of Command
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(proposal.getKind());
if (command == null) { // lazy resolve the edit.
Expand All @@ -301,7 +301,12 @@ 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) {
codeAction.setEdit(edit);
} else {
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, -proposal.getRelevance()));
}
}
if (proposal.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 @@ -334,7 +334,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
3 changes: 1 addition & 2 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 @@ -632,8 +632,7 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(proposal.getKind())) {
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(proposal.getKind());
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
codeAction.setEdit(edit);
codeAction.setDiagnostics(Collections.EMPTY_LIST);
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down
18 changes: 13 additions & 5 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 @@ -310,11 +310,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 = (WorkspaceEdit) 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
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);
}
}
}
65 changes: 61 additions & 4 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 @@ -69,6 +69,7 @@
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import static org.mockito.Mockito.when;

/**
* @author Gorkem Ercan
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 = (WorkspaceEdit) 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(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(2).getRight().getKind(), CodeActionKind.SourceOrganizeImports);
WorkspaceEdit w = codeActions.get(0).getRight().getEdit();
Assert.assertNotNull(w);
}

@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 = (WorkspaceEdit) 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 @@ -709,6 +760,12 @@ public static Command getCommand(Either<Command, CodeAction> codeAction) {
return codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
}

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

private Diagnostic getDiagnostic(String code, Range range){
Diagnostic $ = new Diagnostic();
$.setCode(code);
Expand Down
6 changes: 3 additions & 3 deletions ...t.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateAccessorsActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -75,9 +76,8 @@ public void testGenerateAccessorsEnabled() throws JavaModelException {
Assert.assertNotNull(codeActions);
Either<Command, CodeAction> generateAccessorsAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS);
Assert.assertNotNull(generateAccessorsAction);
Command generateAccessorsCommand = CodeActionHandlerTest.getCommand(generateAccessorsAction);
Assert.assertNotNull(generateAccessorsCommand);
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, generateAccessorsCommand.getCommand());
WorkspaceEdit generateAccessorsEdit = CodeActionHandlerTest.getEdit(generateAccessorsAction);
Assert.assertNotNull(generateAccessorsEdit);
}

@Test
Expand Down
1 change: 1 addition & 0 deletions ...s.tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateConstructorsActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Assert;
import org.junit.Before;
Expand Down
6 changes: 3 additions & 3 deletions ...dt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateToStringActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -93,9 +94,8 @@ public void testGenerateToStringEnabled_emptyFields() throws JavaModelException
Assert.assertNotNull(codeActions);
Either<Command, CodeAction> toStringAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_TO_STRING);
Assert.assertNotNull(toStringAction);
Command toStringCommand = CodeActionHandlerTest.getCommand(toStringAction);
Assert.assertNotNull(toStringCommand);
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, toStringCommand.getCommand());
WorkspaceEdit toStringEdit = CodeActionHandlerTest.getEdit(toStringAction);
Assert.assertNotNull(toStringEdit);
}

@Test
Expand Down

0 comments on commit c93f930

Please sign in to comment.