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>
  • Loading branch information
@fbricon
Boris Staletic authored and fbricon committed Nov 27, 2019
1 parent 8a0b8ec commit 436d901
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 33 deletions.
10 changes: 7 additions & 3 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java
View file
Expand Up @@ -150,25 +150,29 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(ChangeCo
String name = proposal.getName();

Command command;
WorkspaceEdit edit = null;
if (proposal instanceof CUCorrectionCommandProposal) {
CUCorrectionCommandProposal commandProposal = (CUCorrectionCommandProposal) proposal;
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else if (proposal instanceof RefactoringCorrectionCommandProposal) {
RefactoringCorrectionCommandProposal commandProposal = (RefactoringCorrectionCommandProposal) proposal;
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else {
WorkspaceEdit edit = ChangeUtil.convertToWorkspaceEdit(proposal.getChange());
edit = ChangeUtil.convertToWorkspaceEdit(proposal.getChange());
if (!ChangeUtil.hasChanges(edit)) {
return Optional.empty();
}
command = new Command(name, COMMAND_ID_APPLY_EDIT, Collections.singletonList(edit));
}

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);
if (edit != null) {
codeAction.setEdit(edit);
} else {
codeAction.setCommand(command);
}
codeAction.setDiagnostics(context.getDiagnostics());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down
6 changes: 3 additions & 3 deletions ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
View file
Expand Up @@ -260,9 +260,9 @@ public boolean isSemanticHighlightingSupported() {
}

/**
* {@code true} if the client has explicitly set the
* {@code textDocument.documentSymbol.hierarchicalDocumentSymbolSupport} to
* {@code true} when initializing the LS. Otherwise, {@code false}.
* {@code true} if the client has listed {@code kind} in
* {@code textDocument.codeAction.codeActionLiteralSupport.codeActionKind.valueSet}
* when initializing the LS. Otherwise, {@code false}.
*/
public boolean isSupportedCodeActionKind(String kind) {
//@formatter:off
Expand Down
2 changes: 1 addition & 1 deletion ...t.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
View file
Expand Up @@ -352,7 +352,7 @@ private Optional<Either<Command, CodeAction>> convertToWorkspaceEditAction(CodeA
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(kind)) {
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setEdit(workspaceEdit);
codeAction.setDiagnostics(context.getDiagnostics());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down
24 changes: 16 additions & 8 deletions ...se.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/AbstractQuickFixTest.java
View file
Expand Up @@ -294,14 +294,22 @@ 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);
if (we.getDocumentChanges() != null) {
return evaluateChanges(we.getDocumentChanges());
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);
if (we.getDocumentChanges() != null) {
return evaluateChanges(we.getDocumentChanges());
}
return evaluateChanges(we.getChanges());
} else {
WorkspaceEdit we = (WorkspaceEdit) codeAction.getRight().getEdit();
if (we.getDocumentChanges() != null) {
return evaluateChanges(we.getDocumentChanges());
}
return evaluateChanges(we.getChanges());
}
return evaluateChanges(we.getChanges());
}

private String evaluateChanges(List<Either<TextDocumentEdit, ResourceOperation>> documentChanges) throws BadLocationException, JavaModelException {
Expand Down Expand Up @@ -338,7 +346,7 @@ public Command getCommand(Either<Command, CodeAction> codeAction) {
}

public String getTitle(Either<Command, CodeAction> codeAction) {
return getCommand(codeAction).getTitle();
return codeAction.isLeft() ? codeAction.getLeft().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
Expand Up @@ -193,10 +193,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 @@ -433,4 +439,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
Expand Up @@ -52,6 +52,7 @@
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import static org.mockito.Mockito.when;

/**
* @author Gorkem Ercan
Expand Down Expand Up @@ -94,8 +95,34 @@ public void testCodeAction_removeUnusedImport() throws Exception{
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);
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 @@ -265,8 +292,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 @@ -390,6 +441,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
Expand Up @@ -28,6 +28,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 @@ -69,9 +70,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
6 changes: 3 additions & 3 deletions ...s.tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateConstructorsActionTest.java
View file
Expand Up @@ -28,6 +28,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 @@ -88,9 +89,8 @@ public void testGenerateConstructorsEnabled_emptyFields() throws JavaModelExcept
Assert.assertNotNull(codeActions);
Either<Command, CodeAction> constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
Assert.assertNotNull(constructorAction);
Command constructorCommand = CodeActionHandlerTest.getCommand(constructorAction);
Assert.assertNotNull(constructorCommand);
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, constructorCommand.getCommand());
WorkspaceEdit constructorEdit = CodeActionHandlerTest.getEdit(constructorAction);
Assert.assertNotNull(constructorEdit);
}

@Test
Expand Down
6 changes: 3 additions & 3 deletions ...dt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateToStringActionTest.java
View file
Expand Up @@ -28,6 +28,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 @@ -89,9 +90,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 436d901

Please sign in to comment.