Skip to content

Commit 46e83ee

Browse files
CsCherrYYCsCherrYY
authored
Support multiple selection for generate accessors (#2136)
Signed-off-by: Shi Chen [email protected]
1 parent b7b3190 commit 46e83ee

File tree

5 files changed

+155
-107
lines changed

5 files changed

+155
-107
lines changed

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ private static IJavaElement findElementAfterPosition(IType type, int currentOffs
110110
IJavaElement[] members = type.getChildren();
111111
for (IJavaElement member : members) {
112112
ISourceRange sourceRange = ((IMember) member).getSourceRange();
113-
if (currentOffset < sourceRange.getOffset()) {
113+
if (currentOffset <= sourceRange.getOffset()) {
114114
return member;
115115
}
116116
}
@@ -126,11 +126,11 @@ private static IJavaElement findElementAfterPosition(IType type, Range range) {
126126
return null;
127127
}
128128

129-
int startOffset = DiagnosticsHelper.getStartOffset(type.getCompilationUnit(), range);
130-
if (startOffset < 0) {
129+
int endOffset = DiagnosticsHelper.getEndOffset(type.getCompilationUnit(), range);
130+
if (endOffset < 0) {
131131
return null;
132132
}
133133

134-
return findElementAfterPosition(type, startOffset);
134+
return findElementAfterPosition(type, endOffset);
135135
}
136136
}

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

+2-28
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import org.eclipse.jdt.core.dom.AST;
3333
import org.eclipse.jdt.core.dom.ASTNode;
3434
import org.eclipse.jdt.core.dom.CompilationUnit;
35-
import org.eclipse.jdt.core.dom.FieldDeclaration;
3635
import org.eclipse.jdt.core.dom.IMethodBinding;
3736
import org.eclipse.jdt.core.dom.ITypeBinding;
3837
import org.eclipse.jdt.core.dom.IVariableBinding;
@@ -51,6 +50,7 @@
5150
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspMethodBinding;
5251
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspVariableBinding;
5352
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
53+
import org.eclipse.jdt.ls.core.internal.text.correction.CodeActionUtility;
5454
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistProcessor;
5555
import org.eclipse.jdt.ls.core.internal.text.correction.SourceAssistProcessor;
5656
import org.eclipse.lsp4j.CodeActionParams;
@@ -227,33 +227,7 @@ private static List<String> getFieldNames(ICompilationUnit unit, CompilationUnit
227227
}
228228
InnovationContext context = CodeActionHandler.getContext(unit, astRoot, range);
229229
ArrayList<ASTNode> coveredNodes = QuickAssistProcessor.getFullyCoveredNodes(context, context.getCoveringNode());
230-
List<String> names = new ArrayList<>();
231-
// add covered names
232-
coveredNodes.forEach(coveredNode -> {
233-
if (coveredNode instanceof FieldDeclaration) {
234-
String name = SourceAssistProcessor.getFieldName((FieldDeclaration) coveredNode, coveredNode);
235-
if (name != null) {
236-
names.add(name);
237-
}
238-
} else if (coveredNode instanceof VariableDeclarationFragment) {
239-
String name = SourceAssistProcessor.getFieldName((VariableDeclarationFragment) coveredNode);
240-
if (name != null) {
241-
names.add(name);
242-
}
243-
}
244-
});
245-
// add covering name
246-
ASTNode coveringNode = context.getCoveringNode();
247-
if (coveringNode != null) {
248-
FieldDeclaration fieldDeclaration = SourceAssistProcessor.getFieldDeclarationNode(coveringNode);
249-
if (fieldDeclaration != null) {
250-
String name = SourceAssistProcessor.getFieldName(fieldDeclaration, coveringNode);
251-
if (name != null) {
252-
names.add(name);
253-
}
254-
}
255-
}
256-
return names;
230+
return CodeActionUtility.getFieldNames(coveredNodes, context.getCoveringNode());
257231
}
258232

259233
public static class CheckConstructorsResponse {

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

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
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+
14+
package org.eclipse.jdt.ls.core.internal.text.correction;
15+
16+
import java.util.ArrayList;
17+
import java.util.Arrays;
18+
import java.util.Collections;
19+
import java.util.List;
20+
21+
import org.eclipse.jdt.core.dom.ASTNode;
22+
import org.eclipse.jdt.core.dom.BodyDeclaration;
23+
import org.eclipse.jdt.core.dom.FieldDeclaration;
24+
import org.eclipse.jdt.core.dom.SimpleName;
25+
import org.eclipse.jdt.core.dom.Statement;
26+
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
27+
28+
public class CodeActionUtility {
29+
30+
/**
31+
* find a ASTNode with the given type from the coveredNodes and the coveringNode.
32+
* If there are multiple results, return the first one.
33+
*/
34+
public static ASTNode findASTNode(List<ASTNode> coveredNodes, ASTNode coveringNode, Class<?> nodeType) {
35+
if (!ASTNode.class.isAssignableFrom(nodeType)) {
36+
return null;
37+
}
38+
if (coveredNodes.size() > 0) {
39+
for (ASTNode node : coveredNodes) {
40+
if (nodeType.isInstance(node)) {
41+
return node;
42+
}
43+
ASTNode inferredNode = inferASTNode(node, nodeType);
44+
if (inferredNode != null) {
45+
return inferredNode;
46+
}
47+
}
48+
} else if (coveringNode != null) {
49+
return inferASTNode(coveringNode, nodeType);
50+
}
51+
return null;
52+
}
53+
54+
/**
55+
* infer a ASTNode with the given type from a ASTNode.
56+
*/
57+
public static ASTNode inferASTNode(ASTNode node, Class<?> nodeType) {
58+
if (node == null || !ASTNode.class.isAssignableFrom(nodeType)) {
59+
return null;
60+
}
61+
while (node != null && !nodeType.isInstance(node) && !(node instanceof Statement) && !(node instanceof BodyDeclaration)) {
62+
node = node.getParent();
63+
}
64+
return nodeType.isInstance(node) ? node : null;
65+
}
66+
67+
public static List<String> getFieldNames(List<ASTNode> coveredNodes, ASTNode coveringNode) {
68+
if (coveredNodes.size() > 0) {
69+
// find field names in covered nodes
70+
List<String> names = new ArrayList<>();
71+
for (ASTNode node : coveredNodes) {
72+
names.addAll(getFieldNamesFromASTNode(node));
73+
}
74+
return names;
75+
} else if (coveringNode != null) {
76+
// infer field name in covering node
77+
return getFieldNamesFromASTNode(coveringNode);
78+
}
79+
return Collections.emptyList();
80+
}
81+
82+
public static List<String> getFieldNamesFromASTNode(ASTNode node) {
83+
if (node instanceof SimpleName) {
84+
ASTNode parent = node.getParent();
85+
if (parent instanceof VariableDeclarationFragment) {
86+
return CodeActionUtility.getFieldNamesFromASTNode(parent);
87+
}
88+
} else if (node instanceof VariableDeclarationFragment) {
89+
SimpleName name = ((VariableDeclarationFragment) node).getName();
90+
if (name != null) {
91+
return Arrays.asList(name.getIdentifier());
92+
}
93+
} else if (node instanceof FieldDeclaration) {
94+
List<String> names = new ArrayList<>();
95+
List<VariableDeclarationFragment> fragments = ((FieldDeclaration) node).fragments();
96+
for (VariableDeclarationFragment fragment : fragments) {
97+
names.addAll(CodeActionUtility.getFieldNamesFromASTNode(fragment));
98+
}
99+
return names;
100+
}
101+
return Collections.emptyList();
102+
}
103+
}

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

+23-74
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121
import java.util.Optional;
2222
import java.util.Set;
23+
import java.util.stream.Collectors;
2324
import java.util.stream.Stream;
2425

2526
import org.eclipse.core.runtime.CoreException;
@@ -39,10 +40,8 @@
3940
import org.eclipse.jdt.core.dom.FieldDeclaration;
4041
import org.eclipse.jdt.core.dom.ITypeBinding;
4142
import org.eclipse.jdt.core.dom.ImportDeclaration;
42-
import org.eclipse.jdt.core.dom.SimpleName;
4343
import org.eclipse.jdt.core.dom.Statement;
4444
import org.eclipse.jdt.core.dom.TypeDeclaration;
45-
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
4645
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
4746
import org.eclipse.jdt.core.manipulation.OrganizeImportsOperation;
4847
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
@@ -109,14 +108,11 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
109108
List<Either<Command, CodeAction>> $ = new ArrayList<>();
110109
ICompilationUnit cu = context.getCompilationUnit();
111110
IType type = getSelectionType(context);
112-
ASTNode node = context.getCoveredNode();
113-
if (node == null) {
114-
node = context.getCoveringNode();
115-
}
116-
FieldDeclaration fieldDeclaration = getFieldDeclarationNode(node);
117-
boolean isInFieldDeclaration = fieldDeclaration != null;
118-
boolean isInTypeDeclaration = getTypeDeclarationNode(node) != null;
119-
boolean isInImportDeclaration = getImportDeclarationNode(node) != null;
111+
ArrayList<ASTNode> coveredNodes = QuickAssistProcessor.getFullyCoveredNodes(context, context.getCoveringNode());
112+
ASTNode coveringNode = context.getCoveringNode();
113+
boolean isInFieldDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, FieldDeclaration.class) != null;
114+
boolean isInTypeDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, TypeDeclaration.class) != null;
115+
boolean isInImportDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, ImportDeclaration.class) != null;
120116

121117
// Generate Constructor QuickAssist
122118
if (isInFieldDeclaration || isInTypeDeclaration) {
@@ -165,9 +161,9 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
165161
addSourceActionCommand($, params.getContext(), sourceOverrideMethods);
166162
}
167163

168-
String fieldName = getFieldName(fieldDeclaration, node);
164+
List<String> fieldNames = CodeActionUtility.getFieldNames(coveredNodes, coveringNode);
169165
try {
170-
addGenerateAccessorsSourceActionCommand(params, context, $, type, fieldName, isInTypeDeclaration);
166+
addGenerateAccessorsSourceActionCommand(params, context, $, type, fieldNames, isInTypeDeclaration);
171167
} catch (JavaModelException e) {
172168
JavaLanguageServerPlugin.logException("Failed to generate Getter and Setter source action", e);
173169
}
@@ -233,50 +229,23 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
233229
return $;
234230
}
235231

236-
public static String getFieldName(FieldDeclaration fieldDeclaration, ASTNode node) {
237-
if (fieldDeclaration == null || node == null) {
238-
return null;
239-
}
240-
if (node instanceof SimpleName) {
241-
// Focus on a field name
242-
ASTNode parent = node.getParent();
243-
if (parent instanceof VariableDeclarationFragment) {
244-
// Ensure the field name is under a variable declaration
245-
return ((SimpleName) node).getIdentifier();
246-
}
247-
}
248-
List<VariableDeclarationFragment> fragments = fieldDeclaration.fragments();
249-
if (fragments != null && fragments.size() == 1) {
250-
// FieldDeclaration has only one field
251-
return fragments.get(0).getName().getIdentifier();
252-
}
253-
return null;
254-
}
255-
256-
public static String getFieldName(VariableDeclarationFragment fragment) {
257-
if (fragment == null) {
258-
return null;
259-
}
260-
return fragment.getName().getIdentifier();
261-
}
262-
263-
private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, IType type, String fieldName, boolean isInTypeDeclaration) throws JavaModelException {
232+
private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, IType type, List<String> fieldNames, boolean isInTypeDeclaration) throws JavaModelException {
264233
AccessorField[] accessors = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.BOTH);
265234
AccessorField[] getters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.GETTER);
266235
AccessorField[] setters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.SETTER);
267236

268-
if (fieldName != null) {
269-
Optional<AccessorField> accessorField = Arrays.stream(accessors).filter(accessor -> accessor.fieldName.equals(fieldName)).findFirst();
270-
if (accessorField.isPresent() && accessorField.get().generateGetter && accessorField.get().generateSetter) {
271-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { accessorField.get() }, AccessorKind.BOTH));
237+
if (fieldNames.size() > 0) {
238+
List<AccessorField> accessorFields = Arrays.stream(accessors).filter(accessor -> fieldNames.contains(accessor.fieldName) && accessor.generateGetter && accessor.generateSetter).collect(Collectors.toList());
239+
if (accessorFields.size() > 0) {
240+
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, accessorFields.toArray(new AccessorField[0]), AccessorKind.BOTH));
272241
}
273-
Optional<AccessorField> getterField = Arrays.stream(getters).filter(getter -> getter.fieldName.equals(fieldName)).findFirst();
274-
if (getterField.isPresent()) {
275-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { getterField.get() }, AccessorKind.GETTER));
242+
List<AccessorField> getterFields = Arrays.stream(getters).filter(getter -> fieldNames.contains(getter.fieldName)).collect(Collectors.toList());
243+
if (getterFields.size() > 0) {
244+
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, getterFields.toArray(new AccessorField[0]), AccessorKind.GETTER));
276245
}
277-
Optional<AccessorField> setterField = Arrays.stream(setters).filter(setter -> setter.fieldName.equals(fieldName)).findFirst();
278-
if (setterField.isPresent()) {
279-
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { setterField.get() }, AccessorKind.SETTER));
246+
List<AccessorField> setterFields = Arrays.stream(setters).filter(setter -> fieldNames.contains(setter.fieldName)).collect(Collectors.toList());
247+
if (setterFields.size() > 0) {
248+
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, setterFields.toArray(new AccessorField[0]), AccessorKind.SETTER));
280249
}
281250
}
282251

@@ -381,17 +350,17 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
381350
try {
382351
if (accessors == null || accessors.length == 0) {
383352
return Optional.empty();
384-
} else if (accessors.length == 1 || !preferenceManager.getClientPreferences().isAdvancedGenerateAccessorsSupported()) {
353+
} else if (isQuickAssist || accessors.length == 1 || !preferenceManager.getClientPreferences().isAdvancedGenerateAccessorsSupported()) {
385354
String actionMessage;
386355
switch (accessorKind) {
387356
case BOTH:
388-
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateGetterSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterSetterAction_label;
357+
actionMessage = isQuickAssist && accessors.length == 1 ? Messages.format(ActionMessages.GenerateGetterSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterSetterAction_label;
389358
break;
390359
case GETTER:
391-
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateGetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterAction_label;
360+
actionMessage = isQuickAssist && accessors.length == 1 ? Messages.format(ActionMessages.GenerateGetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterAction_label;
392361
break;
393362
case SETTER:
394-
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateSetterAction_label;
363+
actionMessage = isQuickAssist && accessors.length == 1 ? Messages.format(ActionMessages.GenerateSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateSetterAction_label;
395364
break;
396365
default:
397366
return Optional.empty();
@@ -719,24 +688,4 @@ public static ASTNode getTypeDeclarationNode(ASTNode node) {
719688
}
720689
return node instanceof TypeDeclaration ? node : null;
721690
}
722-
723-
private static ASTNode getImportDeclarationNode(ASTNode node) {
724-
if (node == null) {
725-
return null;
726-
}
727-
while (node != null && !(node instanceof ImportDeclaration) && !(node instanceof Statement)) {
728-
node = node.getParent();
729-
}
730-
return node instanceof ImportDeclaration ? node : null;
731-
}
732-
733-
public static FieldDeclaration getFieldDeclarationNode(ASTNode node) {
734-
if (node == null) {
735-
return null;
736-
}
737-
while (node != null && !(node instanceof FieldDeclaration) && !(node instanceof Statement)) {
738-
node = node.getParent();
739-
}
740-
return node instanceof FieldDeclaration ? (FieldDeclaration) node : null;
741-
}
742691
}

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

+23-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@ public void testGenerateAccessorsQuickAssistForTypeDeclaration() throws JavaMode
173173
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
174174
Assert.assertNotNull(codeActions);
175175
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
176-
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, SourceAssistProcessor.COMMAND_ID_ACTION_GENERATEACCESSORSPROMPT));
176+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Getters and Setters"));
177+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Getters"));
178+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Setters"));
177179
}
178180

179181
@Test
@@ -196,6 +198,26 @@ public void testGenerateAccessorsQuickAssistForFieldDeclaration() throws JavaMod
196198
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Setter for 'name'"));
197199
}
198200

201+
@Test
202+
public void testGenerateAccessorsQuickAssistForMultipleFieldDeclaration() throws JavaModelException {
203+
//@formatter:off
204+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
205+
"\r\n" +
206+
"public class A {\r\n" +
207+
" public String name = \"name\";\r\n" +
208+
" public String pet = \"pet\";\r\n" +
209+
"}"
210+
, true, null);
211+
//@formatter:on
212+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name = \"name\";\r\n public String pet = \"pet\";");
213+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
214+
Assert.assertNotNull(codeActions);
215+
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
216+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Getters and Setters"));
217+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Getters"));
218+
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Generate Setters"));
219+
}
220+
199221
@Test
200222
public void testGenerateAccessorsQuickAssistForFinalField() throws JavaModelException {
201223
//@formatter:off

0 commit comments

Comments
 (0)