Skip to content

Commit 15af562

Browse files
authored
show accessors quick fixes for field declarations (#2092)
* show accessors quick fixes for field declarations Signed-off-by: Shi Chen <chenshi@microsoft.com>
1 parent c7b80fc commit 15af562

7 files changed

Lines changed: 187 additions & 101 deletions

File tree

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/proposals/LocalCorrectionsSubProcessor.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,10 +460,6 @@ public static void addUnusedMemberProposal(IInvocationContext context, IProblemL
460460
}
461461
}
462462
}
463-
464-
if (problemId == IProblem.UnusedPrivateField) {
465-
GetterSetterCorrectionSubProcessor.addGetterSetterProposal(context, problem, proposals, IProposalRelevance.GETTER_SETTER_UNUSED_PRIVATE_FIELD);
466-
}
467463
}
468464

469465
public static void getUnreachableCodeProposals(IInvocationContext context, IProblemLocationCore problem, Collection<ChangeCorrectionProposal> proposals) {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2018-2019 Microsoft Corporation and others.
2+
* Copyright (c) 2018-2022 Microsoft Corporation and others.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License 2.0
55
* which accompanies this distribution, and is available at
@@ -25,10 +25,13 @@ private ActionMessages() {
2525
public static String OverrideMethodsAction_label;
2626
public static String GenerateGetterSetterAction_label;
2727
public static String GenerateGetterSetterAction_ellipsisLabel;
28+
public static String GenerateGetterSetterAction_templateLabel;
2829
public static String GenerateGetterAction_label;
2930
public static String GenerateGetterAction_ellipsisLabel;
31+
public static String GenerateGetterAction_templateLabel;
3032
public static String GenerateSetterAction_label;
3133
public static String GenerateSetterAction_ellipsisLabel;
34+
public static String GenerateSetterAction_templateLabel;
3235
public static String GenerateHashCodeEqualsAction_label;
3336
public static String GenerateToStringAction_label;
3437
public static String GenerateToStringAction_ellipsisLabel;

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/ActionMessages.properties

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
###############################################################################
2-
# Copyright (c) 2018-2019 Microsoft Corporation and others.
2+
# Copyright (c) 2018-2022 Microsoft Corporation and others.
33
# All rights reserved. This program and the accompanying materials
44
# are made available under the terms of the Eclipse Public License 2.0
55
# which accompanies this distribution, and is available at
@@ -14,10 +14,13 @@
1414
OverrideMethodsAction_label=Override/Implement Methods...
1515
GenerateGetterSetterAction_label=Generate Getters and Setters
1616
GenerateGetterSetterAction_ellipsisLabel=Generate Getters and Setters...
17+
GenerateGetterSetterAction_templateLabel=Generate Getter and Setter for ''{0}''
1718
GenerateGetterAction_label=Generate Getters
1819
GenerateGetterAction_ellipsisLabel=Generate Getters...
20+
GenerateGetterAction_templateLabel=Generate Getter for ''{0}''
1921
GenerateSetterAction_label=Generate Setters
2022
GenerateSetterAction_ellipsisLabel=Generate Setters...
23+
GenerateSetterAction_templateLabel=Generate Setter for ''{0}''
2124
GenerateHashCodeEqualsAction_label=Generate hashCode() and equals()...
2225
GenerateToStringAction_label=Generate toString()
2326
GenerateToStringAction_ellipsisLabel=Generate toString()...

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

Lines changed: 129 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2017-2021 Microsoft Corporation and others.
2+
* Copyright (c) 2017-2022 Microsoft Corporation and others.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License 2.0
55
* which accompanies this distribution, and is available at
@@ -36,10 +36,13 @@
3636
import org.eclipse.jdt.core.dom.AnonymousClassDeclaration;
3737
import org.eclipse.jdt.core.dom.BodyDeclaration;
3838
import org.eclipse.jdt.core.dom.CompilationUnit;
39+
import org.eclipse.jdt.core.dom.FieldDeclaration;
3940
import org.eclipse.jdt.core.dom.ITypeBinding;
4041
import org.eclipse.jdt.core.dom.ImportDeclaration;
42+
import org.eclipse.jdt.core.dom.SimpleName;
4143
import org.eclipse.jdt.core.dom.Statement;
4244
import org.eclipse.jdt.core.dom.TypeDeclaration;
45+
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
4346
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
4447
import org.eclipse.jdt.core.manipulation.OrganizeImportsOperation;
4548
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
@@ -51,6 +54,7 @@
5154
import org.eclipse.jdt.ls.core.internal.JDTUtils;
5255
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
5356
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
57+
import org.eclipse.jdt.ls.core.internal.Messages;
5458
import org.eclipse.jdt.ls.core.internal.TextEditConverter;
5559
import org.eclipse.jdt.ls.core.internal.codemanipulation.GenerateGetterSetterOperation;
5660
import org.eclipse.jdt.ls.core.internal.codemanipulation.GenerateGetterSetterOperation.AccessorField;
@@ -105,18 +109,19 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
105109
List<Either<Command, CodeAction>> $ = new ArrayList<>();
106110
ICompilationUnit cu = context.getCompilationUnit();
107111
IType type = getSelectionType(context);
108-
boolean isInTypeDeclaration = isInTypeDeclaration(context);
109-
boolean isInImportDeclaration = isInImportDeclaration(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;
110120

111-
try {
112-
// Generate Constructor QuickAssist
113-
IJavaElement element = JDTUtils.findElementAtSelection(cu, params.getRange().getEnd().getLine(), params.getRange().getEnd().getCharacter(), this.preferenceManager, new NullProgressMonitor());
114-
if (element instanceof IField || isInTypeDeclaration) {
115-
Optional<Either<Command, CodeAction>> quickAssistGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
116-
addSourceActionCommand($, params.getContext(), quickAssistGenerateConstructors);
117-
}
118-
} catch (JavaModelException e) {
119-
JavaLanguageServerPlugin.logException(e);
121+
// Generate Constructor QuickAssist
122+
if (isInFieldDeclaration || isInTypeDeclaration) {
123+
Optional<Either<Command, CodeAction>> quickAssistGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
124+
addSourceActionCommand($, params.getContext(), quickAssistGenerateConstructors);
120125
}
121126
// Generate Constructor Source Action
122127
Optional<Either<Command, CodeAction>> sourceGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS, monitor);
@@ -160,43 +165,9 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
160165
addSourceActionCommand($, params.getContext(), sourceOverrideMethods);
161166
}
162167

168+
String fieldName = getFieldName(fieldDeclaration, node);
163169
try {
164-
AccessorField[] accessors = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.BOTH);
165-
AccessorField[] getters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.GETTER);
166-
AccessorField[] setters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.SETTER);
167-
168-
if (getters.length > 0 && setters.length > 0) {
169-
// Generate Getter and Setter QuickAssist
170-
if (isInTypeDeclaration) {
171-
Optional<Either<Command, CodeAction>> quickAssistGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, accessors, AccessorKind.BOTH);
172-
addSourceActionCommand($, params.getContext(), quickAssistGetterSetter);
173-
}
174-
// Generate Getter and Setter Source Action
175-
Optional<Either<Command, CodeAction>> sourceGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, accessors, AccessorKind.BOTH);
176-
addSourceActionCommand($, params.getContext(), sourceGetterSetter);
177-
}
178-
179-
if (getters.length > 0) {
180-
// Generate Getter QuickAssist
181-
if (isInTypeDeclaration) {
182-
Optional<Either<Command, CodeAction>> quickAssistGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, getters, AccessorKind.GETTER);
183-
addSourceActionCommand($, params.getContext(), quickAssistGetter);
184-
}
185-
// Generate Getter Source Action
186-
Optional<Either<Command, CodeAction>> sourceGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, getters, AccessorKind.GETTER);
187-
addSourceActionCommand($, params.getContext(), sourceGetter);
188-
}
189-
190-
if (setters.length > 0) {
191-
// Generate Setter QuickAssist
192-
if (isInTypeDeclaration) {
193-
Optional<Either<Command, CodeAction>> quickAssistSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, setters, AccessorKind.SETTER);
194-
addSourceActionCommand($, params.getContext(), quickAssistSetter);
195-
}
196-
// Generate Setter Source Action
197-
Optional<Either<Command, CodeAction>> sourceSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, setters, AccessorKind.SETTER);
198-
addSourceActionCommand($, params.getContext(), sourceSetter);
199-
}
170+
addGenerateAccessorsSourceActionCommand(params, context, $, type, fieldName, isInTypeDeclaration);
200171
} catch (JavaModelException e) {
201172
JavaLanguageServerPlugin.logException("Failed to generate Getter and Setter source action", e);
202173
}
@@ -262,6 +233,80 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
262233
return $;
263234
}
264235

236+
private 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+
private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, IType type, String fieldName, boolean isInTypeDeclaration) throws JavaModelException {
257+
AccessorField[] accessors = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.BOTH);
258+
AccessorField[] getters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.GETTER);
259+
AccessorField[] setters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.SETTER);
260+
261+
if (fieldName != null) {
262+
Optional<AccessorField> accessorField = Arrays.stream(accessors).filter(accessor -> accessor.fieldName.equals(fieldName)).findFirst();
263+
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));
265+
}
266+
Optional<AccessorField> getterField = Arrays.stream(getters).filter(getter -> getter.fieldName.equals(fieldName)).findFirst();
267+
if (getterField.isPresent()) {
268+
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { getterField.get() }, AccessorKind.GETTER));
269+
}
270+
Optional<AccessorField> setterField = Arrays.stream(setters).filter(setter -> setter.fieldName.equals(fieldName)).findFirst();
271+
if (setterField.isPresent()) {
272+
addSourceActionCommand($, params.getContext(), getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, new AccessorField[] { setterField.get() }, AccessorKind.SETTER));
273+
}
274+
}
275+
276+
if (getters.length > 0 && setters.length > 0) {
277+
// Generate Getter and Setter QuickAssist
278+
if (isInTypeDeclaration) {
279+
Optional<Either<Command, CodeAction>> quickAssistGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, accessors, AccessorKind.BOTH);
280+
addSourceActionCommand($, params.getContext(), quickAssistGetterSetter);
281+
}
282+
// Generate Getter and Setter Source Action
283+
Optional<Either<Command, CodeAction>> sourceGetterSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, accessors, AccessorKind.BOTH);
284+
addSourceActionCommand($, params.getContext(), sourceGetterSetter);
285+
}
286+
287+
if (getters.length > 0) {
288+
// Generate Getter QuickAssist
289+
if (isInTypeDeclaration) {
290+
Optional<Either<Command, CodeAction>> quickAssistGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, getters, AccessorKind.GETTER);
291+
addSourceActionCommand($, params.getContext(), quickAssistGetter);
292+
}
293+
// Generate Getter Source Action
294+
Optional<Either<Command, CodeAction>> sourceGetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, getters, AccessorKind.GETTER);
295+
addSourceActionCommand($, params.getContext(), sourceGetter);
296+
}
297+
298+
if (setters.length > 0) {
299+
// Generate Setter QuickAssist
300+
if (isInTypeDeclaration) {
301+
Optional<Either<Command, CodeAction>> quickAssistSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, isInTypeDeclaration, setters, AccessorKind.SETTER);
302+
addSourceActionCommand($, params.getContext(), quickAssistSetter);
303+
}
304+
// Generate Setter Source Action
305+
Optional<Either<Command, CodeAction>> sourceSetter = getGetterSetterAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS, isInTypeDeclaration, setters, AccessorKind.SETTER);
306+
addSourceActionCommand($, params.getContext(), sourceSetter);
307+
}
308+
}
309+
265310
private void addSourceActionCommand(List<Either<Command, CodeAction>> result, CodeActionContext context, Optional<Either<Command, CodeAction>> target) {
266311
if (!target.isPresent()) {
267312
return;
@@ -325,28 +370,25 @@ private Optional<Either<Command, CodeAction>> getOverrideMethodsAction(CodeActio
325370
}
326371

327372
private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionParams params, IInvocationContext context, IType type, String kind, boolean isInTypeDeclaration, AccessorField[] accessors, AccessorKind accessorKind) {
328-
String ellipsisActionMessage;
329-
String actionMessage;
330-
switch (accessorKind) {
331-
case BOTH:
332-
actionMessage = ActionMessages.GenerateGetterSetterAction_label;
333-
ellipsisActionMessage = ActionMessages.GenerateGetterSetterAction_ellipsisLabel;
334-
break;
335-
case GETTER:
336-
actionMessage = ActionMessages.GenerateGetterAction_label;
337-
ellipsisActionMessage = ActionMessages.GenerateGetterAction_ellipsisLabel;
338-
break;
339-
case SETTER:
340-
actionMessage = ActionMessages.GenerateSetterAction_label;
341-
ellipsisActionMessage = ActionMessages.GenerateSetterAction_ellipsisLabel;
342-
break;
343-
default:
344-
return Optional.empty();
345-
}
373+
boolean isQuickAssist = kind.equals(JavaCodeActionKind.QUICK_ASSIST);
346374
try {
347375
if (accessors == null || accessors.length == 0) {
348376
return Optional.empty();
349377
} else if (accessors.length == 1 || !preferenceManager.getClientPreferences().isAdvancedGenerateAccessorsSupported()) {
378+
String actionMessage;
379+
switch (accessorKind) {
380+
case BOTH:
381+
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateGetterSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterSetterAction_label;
382+
break;
383+
case GETTER:
384+
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateGetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateGetterAction_label;
385+
break;
386+
case SETTER:
387+
actionMessage = isQuickAssist ? Messages.format(ActionMessages.GenerateSetterAction_templateLabel, accessors[0].fieldName) : ActionMessages.GenerateSetterAction_label;
388+
break;
389+
default:
390+
return Optional.empty();
391+
}
350392
CodeActionProposal getAccessorsProposal = (pm) -> {
351393
// If cursor position is not specified, then insert to the last by default.
352394
IJavaElement insertBefore = isInTypeDeclaration ? CodeGenerationUtils.findInsertElement(type, null) : CodeGenerationUtils.findInsertElement(type, params.getRange());
@@ -356,10 +398,24 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
356398
};
357399
return getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), actionMessage, kind, getAccessorsProposal);
358400
} else {
401+
String actionMessage;
402+
switch (accessorKind) {
403+
case BOTH:
404+
actionMessage = ActionMessages.GenerateGetterSetterAction_ellipsisLabel;
405+
break;
406+
case GETTER:
407+
actionMessage = ActionMessages.GenerateGetterAction_ellipsisLabel;
408+
break;
409+
case SETTER:
410+
actionMessage = ActionMessages.GenerateSetterAction_ellipsisLabel;
411+
break;
412+
default:
413+
return Optional.empty();
414+
}
359415
AccessorCodeActionParams accessorParams = new AccessorCodeActionParams(params.getTextDocument(), params.getRange(), params.getContext(), accessorKind);
360-
Command command = new Command(ellipsisActionMessage, COMMAND_ID_ACTION_GENERATEACCESSORSPROMPT, Collections.singletonList(accessorParams));
416+
Command command = new Command(actionMessage, COMMAND_ID_ACTION_GENERATEACCESSORSPROMPT, Collections.singletonList(accessorParams));
361417
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(JavaCodeActionKind.SOURCE_GENERATE_ACCESSORS)) {
362-
CodeAction codeAction = new CodeAction(ellipsisActionMessage);
418+
CodeAction codeAction = new CodeAction(actionMessage);
363419
codeAction.setKind(kind);
364420
codeAction.setCommand(command);
365421
codeAction.setDiagnostics(Collections.emptyList());
@@ -667,19 +723,13 @@ private static ASTNode getImportDeclarationNode(ASTNode node) {
667723
return node instanceof ImportDeclaration ? node : null;
668724
}
669725

670-
private static boolean isInTypeDeclaration(IInvocationContext context) {
671-
ASTNode node = context.getCoveredNode();
726+
private static FieldDeclaration getFieldDeclarationNode(ASTNode node) {
672727
if (node == null) {
673-
node = context.getCoveringNode();
728+
return null;
674729
}
675-
return getTypeDeclarationNode(node) != null;
676-
}
677-
678-
private static boolean isInImportDeclaration(IInvocationContext context) {
679-
ASTNode node = context.getCoveredNode();
680-
if (node == null) {
681-
node = context.getCoveringNode();
730+
while (node != null && !(node instanceof FieldDeclaration) && !(node instanceof Statement)) {
731+
node = node.getParent();
682732
}
683-
return getImportDeclarationNode(node) != null;
733+
return node instanceof FieldDeclaration ? (FieldDeclaration) node : null;
684734
}
685735
}

0 commit comments

Comments
 (0)