Skip to content

Commit f6aa491

Browse files
CsCherrYYCsCherrYY
committed
sort quick assists
Signed-off-by: Shi Chen <[email protected]>
1 parent 839c8af commit f6aa491

File tree

4 files changed

+174
-21
lines changed

4 files changed

+174
-21
lines changed

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

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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.Arrays;
16+
import java.util.Comparator;
17+
import java.util.HashMap;
18+
import java.util.List;
19+
import java.util.Map;
20+
import java.util.stream.Collectors;
21+
22+
import org.eclipse.jdt.ls.core.internal.Messages;
23+
import org.eclipse.jdt.ls.core.internal.corrections.CorrectionMessages;
24+
import org.eclipse.lsp4j.CodeAction;
25+
import org.eclipse.lsp4j.Command;
26+
import org.eclipse.lsp4j.jsonrpc.messages.Either;
27+
28+
public class QuickAssistComparator implements Comparator<Either<Command, CodeAction>> {
29+
30+
public enum ContextType {
31+
FIELD_DECLARATION,
32+
TYPE_DECLARATION,
33+
IMPORT_DECLARATION
34+
}
35+
36+
private List<String> prioritiesList;
37+
38+
private static Map<ContextType, List<String>> prioritiesMap = new HashMap<>();
39+
40+
static {
41+
prioritiesMap.put(ContextType.FIELD_DECLARATION, Arrays.asList(
42+
ActionMessages.GenerateGetterSetterAction_templateLabel,
43+
ActionMessages.GenerateGetterAction_templateLabel,
44+
ActionMessages.GenerateSetterAction_templateLabel,
45+
ActionMessages.GenerateConstructorsAction_ellipsisLabel,
46+
ActionMessages.GenerateConstructorsAction_label));
47+
48+
prioritiesMap.put(ContextType.TYPE_DECLARATION, Arrays.asList(
49+
ActionMessages.GenerateConstructorsAction_ellipsisLabel,
50+
ActionMessages.GenerateConstructorsAction_label,
51+
ActionMessages.OverrideMethodsAction_label,
52+
ActionMessages.GenerateGetterSetterAction_templateLabel,
53+
ActionMessages.GenerateGetterAction_templateLabel,
54+
ActionMessages.GenerateSetterAction_templateLabel,
55+
ActionMessages.GenerateHashCodeEqualsAction_label,
56+
ActionMessages.GenerateToStringAction_ellipsisLabel
57+
));
58+
59+
prioritiesMap.put(ContextType.IMPORT_DECLARATION, Arrays.asList(
60+
CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description
61+
));
62+
}
63+
64+
public QuickAssistComparator(ContextType contextType, String fieldName) {
65+
this.prioritiesList = prioritiesMap.get(contextType).stream().map(message -> Messages.format(message, fieldName)).collect(Collectors.toList());
66+
}
67+
68+
public int compare(Either<Command, CodeAction> e1, Either<Command, CodeAction> e2) {
69+
String title1 = e1.isLeft() ? e1.getLeft().getTitle() : e1.isRight() ? e1.getRight().getTitle() : null;
70+
String title2 = e2.isLeft() ? e2.getLeft().getTitle() : e2.isRight() ? e2.getRight().getTitle() : null;
71+
if (title1 == null || title2 == null) {
72+
return 0;
73+
}
74+
int index1 = this.prioritiesList.indexOf(title1);
75+
int index2 = this.prioritiesList.indexOf(title2);
76+
if (index1 == -1 || index2 == -1) {
77+
return 0;
78+
}
79+
return index1 - index2;
80+
}
81+
}

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

+26-15
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
import org.eclipse.jdt.ls.core.internal.handlers.OrganizeImportsHandler;
7777
import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.AccessorCodeActionParams;
7878
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
79+
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistComparator.ContextType;
7980
import org.eclipse.lsp4j.CodeAction;
8081
import org.eclipse.lsp4j.CodeActionContext;
8182
import org.eclipse.lsp4j.CodeActionKind;
@@ -107,6 +108,7 @@ public SourceAssistProcessor(PreferenceManager preferenceManager) {
107108

108109
public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParams params, IInvocationContext context, IProblemLocationCore[] locations, IProgressMonitor monitor) {
109110
List<Either<Command, CodeAction>> $ = new ArrayList<>();
111+
List<Either<Command, CodeAction>> quickAssists = new ArrayList<>();
110112
ICompilationUnit cu = context.getCompilationUnit();
111113
IType type = getSelectionType(context);
112114
ASTNode node = context.getCoveredNode();
@@ -121,7 +123,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
121123
// Generate Constructor QuickAssist
122124
if (isInFieldDeclaration || isInTypeDeclaration) {
123125
Optional<Either<Command, CodeAction>> quickAssistGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
124-
addSourceActionCommand($, params.getContext(), quickAssistGenerateConstructors);
126+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistGenerateConstructors);
125127
}
126128
// Generate Constructor Source Action
127129
Optional<Either<Command, CodeAction>> sourceGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS, monitor);
@@ -132,7 +134,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
132134
// Generate QuickAssist
133135
if (isInImportDeclaration) {
134136
Optional<Either<Command, CodeAction>> quickAssistOrganizeImports = getOrganizeImportsAction(params, JavaCodeActionKind.QUICK_ASSIST);
135-
addSourceActionCommand($, params.getContext(), quickAssistOrganizeImports);
137+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistOrganizeImports);
136138
}
137139
// Generate Source Action
138140
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getOrganizeImportsAction(params, CodeActionKind.SourceOrganizeImports);
@@ -146,7 +148,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
146148
if (isInImportDeclaration) {
147149
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
148150
JavaCodeActionKind.QUICK_ASSIST, organizeImportsProposal);
149-
addSourceActionCommand($, params.getContext(), sourceOrganizeImports);
151+
addSourceActionCommand(quickAssists, params.getContext(), sourceOrganizeImports);
150152
}
151153
// Generate Source Action
152154
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
@@ -158,7 +160,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
158160
// Override/Implement Methods QuickAssist
159161
if (isInTypeDeclaration) {
160162
Optional<Either<Command, CodeAction>> quickAssistOverrideMethods = getOverrideMethodsAction(params, JavaCodeActionKind.QUICK_ASSIST);
161-
addSourceActionCommand($, params.getContext(), quickAssistOverrideMethods);
163+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistOverrideMethods);
162164
}
163165
// Override/Implement Methods Source Action
164166
Optional<Either<Command, CodeAction>> sourceOverrideMethods = getOverrideMethodsAction(params, JavaCodeActionKind.SOURCE_OVERRIDE_METHODS);
@@ -167,7 +169,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
167169

168170
String fieldName = getFieldName(fieldDeclaration, node);
169171
try {
170-
addGenerateAccessorsSourceActionCommand(params, context, $, type, fieldName, isInTypeDeclaration);
172+
addGenerateAccessorsSourceActionCommand(params, context, $, quickAssists, type, fieldName, isInTypeDeclaration);
171173
} catch (JavaModelException e) {
172174
JavaLanguageServerPlugin.logException("Failed to generate Getter and Setter source action", e);
173175
}
@@ -177,7 +179,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
177179
// Generate QuickAssist
178180
if (isInTypeDeclaration) {
179181
Optional<Either<Command, CodeAction>> quickAssistHashCodeEquals = getHashCodeEqualsAction(params, JavaCodeActionKind.QUICK_ASSIST);
180-
addSourceActionCommand($, params.getContext(), quickAssistHashCodeEquals);
182+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistHashCodeEquals);
181183
}
182184

183185
// Generate Source Action
@@ -198,7 +200,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
198200
// Generate QuickAssist
199201
if (isInTypeDeclaration) {
200202
Optional<Either<Command, CodeAction>> generateToStringQuickAssist = getGenerateToStringAction(params, JavaCodeActionKind.QUICK_ASSIST);
201-
addSourceActionCommand($, params.getContext(), generateToStringQuickAssist);
203+
addSourceActionCommand(quickAssists, params.getContext(), generateToStringQuickAssist);
202204
}
203205
// Generate Source Action
204206
Optional<Either<Command, CodeAction>> generateToStringCommand = getGenerateToStringAction(params, JavaCodeActionKind.SOURCE_GENERATE_TO_STRING);
@@ -213,7 +215,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
213215
if (isInTypeDeclaration) {
214216
Optional<Either<Command, CodeAction>> generateToStringQuickAssist = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
215217
JavaCodeActionKind.QUICK_ASSIST, generateToStringProposal);
216-
addSourceActionCommand($, params.getContext(), generateToStringQuickAssist);
218+
addSourceActionCommand(quickAssists, params.getContext(), generateToStringQuickAssist);
217219
}
218220
// Generate Source Action
219221
Optional<Either<Command, CodeAction>> generateToStringCommand = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
@@ -230,6 +232,15 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
230232
Optional<Either<Command, CodeAction>> generateFinalModifiers = addFinalModifierWherePossibleAction(context);
231233
addSourceActionCommand($, params.getContext(), generateFinalModifiers);
232234

235+
if (isInFieldDeclaration) {
236+
quickAssists.sort(new QuickAssistComparator(ContextType.FIELD_DECLARATION, fieldName));
237+
} else if (isInTypeDeclaration) {
238+
quickAssists.sort(new QuickAssistComparator(ContextType.TYPE_DECLARATION, fieldName));
239+
} else if (isInImportDeclaration) {
240+
quickAssists.sort(new QuickAssistComparator(ContextType.IMPORT_DECLARATION, fieldName));
241+
}
242+
243+
$.addAll(quickAssists);
233244
return $;
234245
}
235246

@@ -253,31 +264,31 @@ private String getFieldName(FieldDeclaration fieldDeclaration, ASTNode node) {
253264
return null;
254265
}
255266

256-
private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, IType type, String fieldName, boolean isInTypeDeclaration) throws JavaModelException {
267+
private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, List<Either<Command, CodeAction>> quickAssists, IType type, String fieldName, boolean isInTypeDeclaration) throws JavaModelException {
257268
AccessorField[] accessors = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.BOTH);
258269
AccessorField[] getters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.GETTER);
259270
AccessorField[] setters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.SETTER);
260271

261272
if (fieldName != null) {
262273
Optional<AccessorField> accessorField = Arrays.stream(accessors).filter(accessor -> accessor.fieldName.equals(fieldName)).findFirst();
263274
if (accessorField.isPresent() && accessorField.get().generateGetter && accessorField.get().generateSetter) {
264-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { accessorField.get() }, AccessorKind.BOTH));
275+
addSourceActionCommand(quickAssists, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { accessorField.get() }, AccessorKind.BOTH));
265276
}
266277
Optional<AccessorField> getterField = Arrays.stream(getters).filter(getter -> getter.fieldName.equals(fieldName)).findFirst();
267278
if (getterField.isPresent()) {
268-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { getterField.get() }, AccessorKind.GETTER));
279+
addSourceActionCommand(quickAssists, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { getterField.get() }, AccessorKind.GETTER));
269280
}
270281
Optional<AccessorField> setterField = Arrays.stream(setters).filter(setter -> setter.fieldName.equals(fieldName)).findFirst();
271282
if (setterField.isPresent()) {
272-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { setterField.get() }, AccessorKind.SETTER));
283+
addSourceActionCommand(quickAssists, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { setterField.get() }, AccessorKind.SETTER));
273284
}
274285
}
275286

276287
if (getters.length > 0 && setters.length > 0) {
277288
// Generate Getter and Setter QuickAssist
278289
if (isInTypeDeclaration) {
279290
Optional<Either<Command, CodeAction>> quickAssistGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, accessors, AccessorKind.BOTH);
280-
addSourceActionCommand($, params.getContext(), quickAssistGetterSetter);
291+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistGetterSetter);
281292
}
282293
// Generate Getter and Setter Source Action
283294
Optional<Either<Command, CodeAction>> sourceGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, accessors, AccessorKind.BOTH);
@@ -288,7 +299,7 @@ private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, II
288299
// Generate Getter QuickAssist
289300
if (isInTypeDeclaration) {
290301
Optional<Either<Command, CodeAction>> quickAssistGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, getters, AccessorKind.GETTER);
291-
addSourceActionCommand($, params.getContext(), quickAssistGetter);
302+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistGetter);
292303
}
293304
// Generate Getter Source Action
294305
Optional<Either<Command, CodeAction>> sourceGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, getters, AccessorKind.GETTER);
@@ -299,7 +310,7 @@ private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, II
299310
// Generate Setter QuickAssist
300311
if (isInTypeDeclaration) {
301312
Optional<Either<Command, CodeAction>> quickAssistSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, setters, AccessorKind.SETTER);
302-
addSourceActionCommand($, params.getContext(), quickAssistSetter);
313+
addSourceActionCommand(quickAssists, params.getContext(), quickAssistSetter);
303314
}
304315
// Generate Setter Source Action
305316
Optional<Either<Command, CodeAction>> sourceSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, setters, AccessorKind.SETTER);

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

+63
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,69 @@ public void testCodeAction_unimplementedMethodReference() throws Exception {
607607
Assert.assertEquals("Add missing method 'action' to class 'Foo'", action.getTitle());
608608
}
609609

610+
@Test
611+
public void testQuickAssistForImportDeclarationOrder() throws JavaModelException {
612+
//@formatter:off
613+
ICompilationUnit unit = getWorkingCopy(
614+
"src/java/Foo.java",
615+
"import java.util.List;\n"+
616+
"public class Foo {\n"+
617+
" void foo() {\n"+
618+
" String bar = \"astring\";"+
619+
" }\n"+
620+
"}\n");
621+
//@formatter:on
622+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "util");
623+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
624+
Assert.assertNotNull(codeActions);
625+
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
626+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Organize imports");
627+
}
628+
629+
@Test
630+
public void testQuickAssistForFieldDeclarationOrder() throws JavaModelException {
631+
//@formatter:off
632+
ICompilationUnit unit = getWorkingCopy("A.java", "package p;\r\n" +
633+
"\r\n" +
634+
"public class A {\r\n" +
635+
" public String name = \"name\";\r\n" +
636+
" public String pet = \"pet\";\r\n" +
637+
"}");
638+
//@formatter:on
639+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
640+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
641+
Assert.assertNotNull(codeActions);
642+
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
643+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Generate Getter and Setter for 'name'");
644+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(1)).getTitle(), "Generate Getter for 'name'");
645+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(2)).getTitle(), "Generate Setter for 'name'");
646+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(3)).getTitle(), "Generate Constructors...");
647+
}
648+
649+
@Test
650+
public void testQuickAssistForTypeDeclarationOrder() throws JavaModelException {
651+
//@formatter:off
652+
ICompilationUnit unit = getWorkingCopy("A.java", "package p;\r\n" +
653+
"\r\n" +
654+
"public class A {\r\n" +
655+
" public String name = \"name\";\r\n" +
656+
" public String pet = \"pet\";\r\n" +
657+
"}");
658+
//@formatter:on
659+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "A");
660+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
661+
Assert.assertNotNull(codeActions);
662+
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
663+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(0)).getTitle(), "Generate Constructors...");
664+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(1)).getTitle(), "Override/Implement Methods...");
665+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(2)).getTitle(), "Generate Getters and Setters...");
666+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(3)).getTitle(), "Generate Getters...");
667+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(4)).getTitle(), "Generate Setters...");
668+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(5)).getTitle(), "Generate hashCode() and equals()...");
669+
Assert.assertEquals(CodeActionHandlerTest.getCommand(quickAssistActions.get(6)).getTitle(), "Generate toString()...");
670+
671+
}
672+
610673
private List<Either<Command, CodeAction>> getCodeActions(CodeActionParams params) {
611674
return server.codeAction(params).join();
612675
}

0 commit comments

Comments
 (0)