Skip to content

Commit 498ee9f

Browse files
CsCherrYYCsCherrYY
authored
Adjust the order of code actions (#2109)
* sort all code actions Signed-off-by: Shi Chen <[email protected]>
1 parent 2af9b88 commit 498ee9f

File tree

6 files changed

+196
-24
lines changed

6 files changed

+196
-24
lines changed

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java

+37-6
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.eclipse.jdt.ls.core.internal.text.correction.AssignToVariableAssistCommandProposal;
5252
import org.eclipse.jdt.ls.core.internal.text.correction.CUCorrectionCommandProposal;
5353
import org.eclipse.jdt.ls.core.internal.text.correction.NonProjectFixProcessor;
54+
import org.eclipse.jdt.ls.core.internal.text.correction.CodeActionComparator;
5455
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistProcessor;
5556
import org.eclipse.jdt.ls.core.internal.text.correction.RefactoringCorrectionCommandProposal;
5657
import org.eclipse.jdt.ls.core.internal.text.correction.SourceAssistProcessor;
@@ -214,6 +215,7 @@ public List<Either<Command, CodeAction>> getCodeActionCommands(CodeActionParams
214215
return Collections.emptyList();
215216
}
216217

218+
codeActions.sort(new CodeActionComparator());
217219
populateDataFields(codeActions);
218220
return codeActions;
219221
}
@@ -224,15 +226,21 @@ private void populateDataFields(List<Either<Command, CodeAction>> codeActions) {
224226
codeActions.forEach(action -> {
225227
if (action.isRight()) {
226228
Either<ChangeCorrectionProposal, CodeActionProposal> proposal = null;
227-
if (action.getRight().getData() instanceof ChangeCorrectionProposal) {
228-
proposal = Either.forLeft((ChangeCorrectionProposal) action.getRight().getData());
229-
} else if (action.getRight().getData() instanceof CodeActionProposal) {
230-
proposal = Either.forRight((CodeActionProposal) action.getRight().getData());
229+
Object originalData = action.getRight().getData();
230+
if (originalData instanceof CodeActionData) {
231+
Object originalProposal = ((CodeActionData) originalData).getProposal();
232+
if (originalProposal instanceof ChangeCorrectionProposal) {
233+
proposal = Either.forLeft((ChangeCorrectionProposal) originalProposal);
234+
} else if (originalProposal instanceof CodeActionProposal) {
235+
proposal = Either.forRight((CodeActionProposal) originalProposal);
236+
} else {
237+
action.getRight().setData(null);
238+
return;
239+
}
231240
} else {
232241
action.getRight().setData(null);
233242
return;
234243
}
235-
236244
Map<String, String> data = new HashMap<>();
237245
data.put(CodeActionResolveHandler.DATA_FIELD_REQUEST_ID, String.valueOf(response.getId()));
238246
data.put(CodeActionResolveHandler.DATA_FIELD_PROPOSAL_ID, String.valueOf(proposals.size()));
@@ -275,7 +283,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(ChangeCo
275283
CodeAction codeAction = new CodeAction(name);
276284
codeAction.setKind(proposal.getKind());
277285
if (command == null) { // lazy resolve the edit.
278-
codeAction.setData(proposal);
286+
codeAction.setData(new CodeActionData(proposal));
279287
} else {
280288
codeAction.setCommand(command);
281289
}
@@ -366,4 +374,27 @@ private static boolean containsKind(List<String> codeActionKinds, String baseKin
366374
return codeActionKinds.stream().anyMatch(kind -> kind.startsWith(baseKind));
367375
}
368376

377+
public static class CodeActionData {
378+
private final Object proposal;
379+
private final int priority;
380+
381+
public CodeActionData(Object proposal) {
382+
this.proposal = proposal;
383+
this.priority = CodeActionComparator.LOWEST_PRIORITY;
384+
}
385+
386+
public CodeActionData(Object proposal, int priority) {
387+
this.proposal = proposal;
388+
this.priority = priority;
389+
}
390+
391+
public Object getProposal() {
392+
return proposal;
393+
}
394+
395+
public int getPriority() {
396+
return priority;
397+
}
398+
}
399+
369400
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/CodeActionComparator.java

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2022 Microsoft Corporation and others.
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License 2.0
5+
* which accompanies this distribution, and is available at
6+
* https://www.eclipse.org/legal/epl-2.0/
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Microsoft Corporation - initial API and implementation
12+
*******************************************************************************/
13+
package org.eclipse.jdt.ls.core.internal.text.correction;
14+
15+
import java.util.Comparator;
16+
17+
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
18+
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler.CodeActionData;
19+
import org.eclipse.lsp4j.CodeAction;
20+
import org.eclipse.lsp4j.CodeActionKind;
21+
import org.eclipse.lsp4j.Command;
22+
import org.eclipse.lsp4j.jsonrpc.messages.Either;
23+
24+
public class CodeActionComparator implements Comparator<Either<Command, CodeAction>> {
25+
26+
public static int ORGANIZE_IMPORTS_PRIORITY = 0;
27+
public static int GENERATE_ACCESSORS_PRIORITY = 10;
28+
public static int GENERATE_CONSTRUCTORS_PRIORITY = 20;
29+
public static int GENERATE_HASHCODE_EQUALS_PRIORITY = 30;
30+
public static int GENERATE_TOSTRING_PRIORITY = 40;
31+
public static int GENERATE_OVERRIDE_IMPLEMENT_PRIORITY = 50;
32+
public static int GENERATE_DELEGATE_METHOD_PRIORITY = 60;
33+
public static int CHANGE_MODIFIER_TO_FINAL_PRIORITY = 70;
34+
public static int LOWEST_PRIORITY = 100;
35+
36+
public int compare(Either<Command, CodeAction> e1, Either<Command, CodeAction> e2) {
37+
if (e1.isRight() && e2.isRight()) {
38+
CodeAction action1 = e1.getRight();
39+
CodeAction action2 = e2.getRight();
40+
int kindDiff = getCodeActionKindOrdinal(action1.getKind()) - getCodeActionKindOrdinal(action2.getKind());
41+
if (kindDiff != 0) {
42+
return kindDiff;
43+
}
44+
Object data1 = action1.getData();
45+
Object data2 = action2.getData();
46+
if (data1 instanceof CodeActionData && data2 instanceof CodeActionData) {
47+
int priority1 = ((CodeActionData) data1).getPriority();
48+
int priority2 = ((CodeActionData) data2).getPriority();
49+
return priority1 - priority2;
50+
} else if (data1 instanceof CodeActionData) {
51+
return 10;
52+
} else if (data2 instanceof CodeActionData) {
53+
return -10;
54+
}
55+
}
56+
return 0;
57+
}
58+
59+
private int getCodeActionKindOrdinal(String kind) {
60+
if (kind.equals(CodeActionKind.QuickFix)) {
61+
return 0;
62+
} else if (kind.startsWith(CodeActionKind.Refactor)) {
63+
return 1000;
64+
} else if (kind.equals(JavaCodeActionKind.QUICK_ASSIST)) {
65+
return 2000;
66+
} else if (kind.startsWith(CodeActionKind.Source)) {
67+
return 3000;
68+
}
69+
return 4000;
70+
}
71+
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java

+19-9
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
import org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandler;
7777
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspVariableBinding;
7878
import org.eclipse.jdt.ls.core.internal.handlers.OrganizeImportsHandler;
79+
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler.CodeActionData;
7980
import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.AccessorCodeActionParams;
8081
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
8182
import org.eclipse.lsp4j.CodeAction;
@@ -144,12 +145,12 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
144145
// Generate QuickAssist
145146
if (isInImportDeclaration) {
146147
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
147-
JavaCodeActionKind.QUICK_ASSIST, organizeImportsProposal);
148+
JavaCodeActionKind.QUICK_ASSIST, organizeImportsProposal, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY);
148149
addSourceActionCommand($, params.getContext(), sourceOrganizeImports);
149150
}
150151
// Generate Source Action
151152
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
152-
CodeActionKind.SourceOrganizeImports, organizeImportsProposal);
153+
CodeActionKind.SourceOrganizeImports, organizeImportsProposal, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY);
153154
addSourceActionCommand($, params.getContext(), sourceOrganizeImports);
154155
}
155156

@@ -211,12 +212,12 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
211212
// Generate QuickAssist
212213
if (isInTypeDeclaration) {
213214
Optional<Either<Command, CodeAction>> generateToStringQuickAssist = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
214-
JavaCodeActionKind.QUICK_ASSIST, generateToStringProposal);
215+
JavaCodeActionKind.QUICK_ASSIST, generateToStringProposal, CodeActionComparator.GENERATE_TOSTRING_PRIORITY);
215216
addSourceActionCommand($, params.getContext(), generateToStringQuickAssist);
216217
}
217218
// Generate Source Action
218219
Optional<Either<Command, CodeAction>> generateToStringCommand = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
219-
JavaCodeActionKind.SOURCE_GENERATE_TO_STRING, generateToStringProposal);
220+
JavaCodeActionKind.SOURCE_GENERATE_TO_STRING, generateToStringProposal, CodeActionComparator.GENERATE_TOSTRING_PRIORITY);
220221
addSourceActionCommand($, params.getContext(), generateToStringCommand);
221222
}
222223
}
@@ -329,6 +330,7 @@ private Optional<Either<Command, CodeAction>> getOrganizeImportsAction(CodeActio
329330
CodeAction codeAction = new CodeAction(CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description);
330331
codeAction.setKind(kind);
331332
codeAction.setCommand(command);
333+
codeAction.setData(new CodeActionData(null, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY));
332334
codeAction.setDiagnostics(Collections.emptyList());
333335
return Optional.of(Either.forRight(codeAction));
334336

@@ -344,6 +346,7 @@ private Optional<Either<Command, CodeAction>> getOverrideMethodsAction(CodeActio
344346
CodeAction codeAction = new CodeAction(ActionMessages.OverrideMethodsAction_label);
345347
codeAction.setKind(kind);
346348
codeAction.setCommand(command);
349+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_OVERRIDE_IMPLEMENT_PRIORITY));
347350
codeAction.setDiagnostics(Collections.emptyList());
348351
return Optional.of(Either.forRight(codeAction));
349352
} else {
@@ -378,7 +381,7 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
378381
TextEdit edit = operation.createTextEdit(pm, accessors);
379382
return convertToWorkspaceEdit(context.getCompilationUnit(), edit);
380383
};
381-
return getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), actionMessage, kind, getAccessorsProposal);
384+
return getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), actionMessage, kind, getAccessorsProposal, CodeActionComparator.GENERATE_ACCESSORS_PRIORITY);
382385
} else {
383386
String actionMessage;
384387
switch (accessorKind) {
@@ -400,6 +403,7 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
400403
CodeAction codeAction = new CodeAction(actionMessage);
401404
codeAction.setKind(kind);
402405
codeAction.setCommand(command);
406+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_ACCESSORS_PRIORITY));
403407
codeAction.setDiagnostics(Collections.emptyList());
404408
return Optional.of(Either.forRight(codeAction));
405409
} else {
@@ -439,6 +443,7 @@ private Optional<Either<Command, CodeAction>> getHashCodeEqualsAction(CodeAction
439443
CodeAction codeAction = new CodeAction(ActionMessages.GenerateHashCodeEqualsAction_label);
440444
codeAction.setKind(kind);
441445
codeAction.setCommand(command);
446+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_HASHCODE_EQUALS_PRIORITY));
442447
codeAction.setDiagnostics(Collections.emptyList());
443448
return Optional.of(Either.forRight(codeAction));
444449
} else {
@@ -467,6 +472,7 @@ private Optional<Either<Command, CodeAction>> getGenerateToStringAction(CodeActi
467472
CodeAction codeAction = new CodeAction(ActionMessages.GenerateToStringAction_ellipsisLabel);
468473
codeAction.setKind(kind);
469474
codeAction.setCommand(command);
475+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_TOSTRING_PRIORITY));
470476
codeAction.setDiagnostics(Collections.emptyList());
471477
return Optional.of(Either.forRight(codeAction));
472478
} else {
@@ -493,14 +499,15 @@ private Optional<Either<Command, CodeAction>> getGenerateConstructorsAction(Code
493499
TextEdit edit = GenerateConstructorsHandler.generateConstructors(type, status.constructors, status.fields, params.getRange(), pm);
494500
return convertToWorkspaceEdit(type.getCompilationUnit(), edit);
495501
};
496-
return getCodeActionFromProposal(params.getContext(), type.getCompilationUnit(), ActionMessages.GenerateConstructorsAction_label, kind, generateConstructorsProposal);
502+
return getCodeActionFromProposal(params.getContext(), type.getCompilationUnit(), ActionMessages.GenerateConstructorsAction_label, kind, generateConstructorsProposal, CodeActionComparator.GENERATE_CONSTRUCTORS_PRIORITY);
497503
}
498504

499505
Command command = new Command(ActionMessages.GenerateConstructorsAction_ellipsisLabel, COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT, Collections.singletonList(params));
500506
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS)) {
501507
CodeAction codeAction = new CodeAction(ActionMessages.GenerateConstructorsAction_ellipsisLabel);
502508
codeAction.setKind(kind);
503509
codeAction.setCommand(command);
510+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_CONSTRUCTORS_PRIORITY));
504511
codeAction.setDiagnostics(Collections.emptyList());
505512
return Optional.of(Either.forRight(codeAction));
506513
} else {
@@ -525,6 +532,7 @@ private Optional<Either<Command, CodeAction>> getGenerateDelegateMethodsAction(C
525532
CodeAction codeAction = new CodeAction(ActionMessages.GenerateDelegateMethodsAction_label);
526533
codeAction.setKind(JavaCodeActionKind.SOURCE_GENERATE_DELEGATE_METHODS);
527534
codeAction.setCommand(command);
535+
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_DELEGATE_METHOD_PRIORITY));
528536
codeAction.setDiagnostics(Collections.EMPTY_LIST);
529537
return Optional.of(Either.forRight(codeAction));
530538
} else {
@@ -581,7 +589,7 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
581589
if (this.preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
582590
CodeAction codeAction = new CodeAction(actionMessage);
583591
codeAction.setKind(proposal.getKind());
584-
codeAction.setData(proposal);
592+
codeAction.setData(new CodeActionData(proposal, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
585593
codeAction.setDiagnostics(Collections.EMPTY_LIST);
586594
return Optional.of(Either.forRight(codeAction));
587595
} else {
@@ -601,6 +609,7 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
601609
CodeAction codeAction = new CodeAction(actionMessage);
602610
codeAction.setKind(proposal.getKind());
603611
codeAction.setCommand(command);
612+
codeAction.setData(new CodeActionData(null, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
604613
codeAction.setDiagnostics(Collections.EMPTY_LIST);
605614
return Optional.of(Either.forRight(codeAction));
606615
} else {
@@ -609,11 +618,11 @@ private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleActio
609618
}
610619
}
611620

612-
private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActionContext context, ICompilationUnit cu, String name, String kind, CodeActionProposal proposal) {
621+
private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActionContext context, ICompilationUnit cu, String name, String kind, CodeActionProposal proposal, int priority) {
613622
if (preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
614623
CodeAction codeAction = new CodeAction(name);
615624
codeAction.setKind(kind);
616-
codeAction.setData(proposal);
625+
codeAction.setData(new CodeActionData(proposal, priority));
617626
codeAction.setDiagnostics(Collections.EMPTY_LIST);
618627
return Optional.of(Either.forRight(codeAction));
619628
}
@@ -629,6 +638,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActi
629638
CodeAction codeAction = new CodeAction(name);
630639
codeAction.setKind(kind);
631640
codeAction.setCommand(command);
641+
codeAction.setData(new CodeActionData(null, priority));
632642
codeAction.setDiagnostics(context.getDiagnostics());
633643
return Optional.of(Either.forRight(codeAction));
634644
} else {

‎org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/ConvertMethodReferenceToLambaTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void testMethodReferenceToLambda() throws Exception {
7171
Range range = new Range(new Position(4, 34), new Position(4, 34));
7272
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu, range);
7373
assertEquals(2, codeActions.size());
74-
Either<Command, CodeAction> codeAction = codeActions.get(1);
74+
Either<Command, CodeAction> codeAction = codeActions.get(0);
7575
CodeAction action = codeAction.getRight();
7676
assertEquals(CodeActionKind.QuickFix, action.getKind());
7777
assertEquals("Convert to lambda expression", action.getTitle());
@@ -98,7 +98,7 @@ public void testLambdaToMethodReference() throws Exception {
9898
Range range = new Range(new Position(4, 39), new Position(4, 39));
9999
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu, range);
100100
assertEquals(2, codeActions.size());
101-
Either<Command, CodeAction> codeAction = codeActions.get(1);
101+
Either<Command, CodeAction> codeAction = codeActions.get(0);
102102
CodeAction action = codeAction.getRight();
103103
assertEquals(CodeActionKind.QuickFix, action.getKind());
104104
assertEquals("Convert to method reference", action.getTitle());

0 commit comments

Comments
 (0)