Skip to content

Commit 661c33f

Browse files
fbriconfbricon
committed
fix: allow constructor codeaction when type has no fields
Signed-off-by: Fred Bricon <[email protected]>
1 parent b125547 commit 661c33f

File tree

3 files changed

+136
-75
lines changed

3 files changed

+136
-75
lines changed

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,16 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
122122
IType type = getSelectionType(context);
123123
ArrayList<ASTNode> coveredNodes = QuickAssistProcessor.getFullyCoveredNodes(context, context.getCoveringNode());
124124
ASTNode coveringNode = context.getCoveringNode();
125-
boolean isInFieldDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, FieldDeclaration.class) != null;
125+
FieldDeclaration fieldDeclaration = (FieldDeclaration) CodeActionUtility.findASTNode(coveredNodes, coveringNode, FieldDeclaration.class);
126+
boolean isInFieldDeclaration = fieldDeclaration != null;
127+
boolean isInStaticFieldDeclaration = isInFieldDeclaration && Modifier.isStatic(fieldDeclaration.getModifiers());
128+
126129
ASTNode typeDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, TypeDeclaration.class);
127130
boolean isInTypeDeclaration = typeDeclaration != null;
128131
boolean isInImportDeclaration = CodeActionUtility.findASTNode(coveredNodes, coveringNode, ImportDeclaration.class) != null;
129132

130133
// Generate Constructor QuickAssist
131-
if (isInFieldDeclaration || isInTypeDeclaration) {
134+
if (isInFieldDeclaration && !isInStaticFieldDeclaration || isInTypeDeclaration) {
132135
Optional<Either<Command, CodeAction>> quickAssistGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
133136
addSourceActionCommand($, params.getContext(), quickAssistGenerateConstructors);
134137
}
@@ -503,12 +506,6 @@ private Optional<Either<Command, CodeAction>> getGenerateConstructorsAction(Code
503506
if (type == null || type.isAnnotation() || type.isInterface() || type.isAnonymous() || type.getCompilationUnit() == null) {
504507
return Optional.empty();
505508
}
506-
507-
boolean hasNonStaticField = hasFields(type, false);
508-
if (!hasNonStaticField) {
509-
return Optional.empty();
510-
}
511-
512509
} catch (JavaModelException e) {
513510
return Optional.empty();
514511
}

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,11 @@ public void testCodeActionLiteral_removeUnusedImport() throws Exception{
157157
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
158158
Assert.assertNotNull(codeActions);
159159
Assert.assertTrue(codeActions.size() >= 4);
160-
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
161-
Assert.assertEquals(codeActions.get(1).getRight().getKind(), CodeActionKind.QuickFix);
162-
Assert.assertEquals(codeActions.get(2).getRight().getKind(), JavaCodeActionKind.QUICK_ASSIST);
163-
Assert.assertEquals(codeActions.get(3).getRight().getKind(), CodeActionKind.SourceOrganizeImports);
160+
Assert.assertEquals(CodeActionKind.QuickFix, codeActions.get(0).getRight().getKind());
161+
Assert.assertEquals(CodeActionKind.QuickFix, codeActions.get(1).getRight().getKind());
162+
Assert.assertEquals(JavaCodeActionKind.QUICK_ASSIST, codeActions.get(2).getRight().getKind());
163+
Assert.assertEquals(JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS, codeActions.get(3).getRight().getKind());
164+
Assert.assertEquals(CodeActionKind.SourceOrganizeImports, codeActions.get(4).getRight().getKind());
164165
WorkspaceEdit w = codeActions.get(0).getRight().getEdit();
165166
Assert.assertNotNull(w);
166167
}
@@ -467,7 +468,7 @@ public void testCodeAction_removeUnterminatedString() throws Exception{
467468
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
468469
Assert.assertNotNull(codeActions);
469470
Assert.assertFalse(codeActions.isEmpty());
470-
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
471+
Assert.assertEquals(CodeActionKind.QuickFix, codeActions.get(0).getRight().getKind());
471472
WorkspaceEdit e = codeActions.get(0).getRight().getEdit();
472473
Assert.assertNotNull(e);
473474
}
@@ -491,7 +492,7 @@ public void testCodeActionLiteral_removeUnterminatedString() throws Exception{
491492
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
492493
Assert.assertNotNull(codeActions);
493494
Assert.assertFalse(codeActions.isEmpty());
494-
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
495+
Assert.assertEquals(CodeActionKind.QuickFix, codeActions.get(0).getRight().getKind());
495496
WorkspaceEdit w = codeActions.get(0).getRight().getEdit();
496497
Assert.assertNotNull(w);
497498
}
@@ -537,7 +538,7 @@ public void testCodeAction_superfluousSemicolon() throws Exception{
537538
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
538539
Assert.assertNotNull(codeActions);
539540
Assert.assertEquals(1, codeActions.size());
540-
Assert.assertEquals(codeActions.get(0), CodeActionKind.QuickFix);
541+
Assert.assertEquals(CodeActionKind.QuickFix, codeActions.get(0).getRight().getKind());
541542
WorkspaceEdit we = codeActions.get(0).getRight().getEdit();
542543
List<org.eclipse.lsp4j.TextEdit> edits = we.getChanges().get(JDTUtils.toURI(unit));
543544
Assert.assertEquals(1, edits.size());
@@ -766,7 +767,7 @@ public void testQuickAssistForImportDeclarationOrder() throws JavaModelException
766767
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
767768
Assert.assertNotNull(codeActions);
768769
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
769-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Organize imports");
770+
Assert.assertEquals("Organize imports", CodeActionHandlerTest.getTitle(quickAssistActions.get(0)));
770771
}
771772

772773
@Test
@@ -783,10 +784,10 @@ public void testQuickAssistForFieldDeclarationOrder() throws JavaModelException
783784
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
784785
Assert.assertNotNull(codeActions);
785786
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
786-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Generate Getter and Setter for 'name'");
787-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(1)), "Generate Getter for 'name'");
788-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(2)), "Generate Setter for 'name'");
789-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(3)), "Generate Constructors...");
787+
Assert.assertEquals("Generate Getter and Setter for 'name'", CodeActionHandlerTest.getTitle(quickAssistActions.get(0)));
788+
Assert.assertEquals("Generate Getter for 'name'", CodeActionHandlerTest.getTitle(quickAssistActions.get(1)));
789+
Assert.assertEquals("Generate Setter for 'name'", CodeActionHandlerTest.getTitle(quickAssistActions.get(2)));
790+
Assert.assertEquals("Generate Constructors...", CodeActionHandlerTest.getTitle(quickAssistActions.get(3)));
790791
}
791792

792793
@Test
@@ -803,13 +804,13 @@ public void testQuickAssistForTypeDeclarationOrder() throws JavaModelException {
803804
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
804805
Assert.assertNotNull(codeActions);
805806
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
806-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(0)), "Generate Getters and Setters");
807-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(1)), "Generate Getters");
808-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(2)), "Generate Setters");
809-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(3)), "Generate Constructors...");
810-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(4)), "Generate hashCode() and equals()...");
811-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(5)), "Generate toString()...");
812-
Assert.assertEquals(CodeActionHandlerTest.getTitle(quickAssistActions.get(6)), "Override/Implement Methods...");
807+
Assert.assertEquals("Generate Getters and Setters", CodeActionHandlerTest.getTitle(quickAssistActions.get(0)));
808+
Assert.assertEquals("Generate Getters", CodeActionHandlerTest.getTitle(quickAssistActions.get(1)));
809+
Assert.assertEquals("Generate Setters", CodeActionHandlerTest.getTitle(quickAssistActions.get(2)));
810+
Assert.assertEquals("Generate Constructors...", CodeActionHandlerTest.getTitle(quickAssistActions.get(3)));
811+
Assert.assertEquals("Generate hashCode() and equals()...", CodeActionHandlerTest.getTitle(quickAssistActions.get(4)));
812+
Assert.assertEquals("Generate toString()...", CodeActionHandlerTest.getTitle(quickAssistActions.get(5)));
813+
Assert.assertEquals("Override/Implement Methods...", CodeActionHandlerTest.getTitle(quickAssistActions.get(6)));
813814
}
814815

815816
private List<Either<Command, CodeAction>> getCodeActions(CodeActionParams params) {

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

Lines changed: 111 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,14 @@
1717

1818
import java.util.List;
1919

20+
import org.eclipse.core.runtime.NullProgressMonitor;
2021
import org.eclipse.jdt.core.ICompilationUnit;
2122
import org.eclipse.jdt.core.IJavaProject;
2223
import org.eclipse.jdt.core.IPackageFragment;
2324
import org.eclipse.jdt.core.IPackageFragmentRoot;
2425
import org.eclipse.jdt.core.JavaModelException;
26+
import org.eclipse.jdt.internal.corext.codemanipulation.CodeGenerationSettings;
27+
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
2528
import org.eclipse.jdt.ls.core.internal.CodeActionUtil;
2629
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
2730
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
@@ -30,8 +33,8 @@
3033
import org.eclipse.lsp4j.CodeAction;
3134
import org.eclipse.lsp4j.CodeActionParams;
3235
import org.eclipse.lsp4j.Command;
33-
import org.eclipse.lsp4j.WorkspaceEdit;
3436
import org.eclipse.lsp4j.jsonrpc.messages.Either;
37+
import org.eclipse.text.edits.TextEdit;
3538
import org.junit.Assert;
3639
import org.junit.Before;
3740
import org.junit.Test;
@@ -61,12 +64,13 @@ public void setup() throws Exception {
6164
@Test
6265
public void testGenerateConstructorsEnabled() throws JavaModelException {
6366
//@formatter:off
64-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
65-
"\r\n" +
66-
"public class A {\r\n" +
67-
" String name;\r\n" +
68-
"}"
69-
, true, null);
67+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
68+
package p;
69+
70+
public class A {
71+
String name;
72+
}
73+
""", true, null);
7074
//@formatter:on
7175
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
7276
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
@@ -81,12 +85,13 @@ public void testGenerateConstructorsEnabled() throws JavaModelException {
8185
@Test
8286
public void testGenerateConstructorsQuickAssist() throws JavaModelException {
8387
//@formatter:off
84-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
85-
"\r\n" +
86-
"public class A {\r\n" +
87-
" String name;\r\n" +
88-
"}"
89-
, true, null);
88+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
89+
package p;
90+
91+
public class A {
92+
String name;
93+
}
94+
""", true, null);
9095
//@formatter:on
9196
// test for field declaration
9297
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
@@ -106,52 +111,60 @@ public void testGenerateConstructorsQuickAssist() throws JavaModelException {
106111
@Test
107112
public void testGenerateConstructorsQuickAssistWithAllStaticFields() throws JavaModelException {
108113
//@formatter:off
109-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
110-
"\r\n" +
111-
"public class A {\r\n" +
112-
" static String name;\r\n" +
113-
"}"
114-
, true, null);
114+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
115+
package p;
116+
117+
public class A {
118+
static String name;
119+
}
120+
""", true, null);
115121
//@formatter:on
116122
// test for field declaration
117123
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "static String name");
118124
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
119125
Assert.assertNotNull(codeActions);
120-
Either<Command, CodeAction> constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
121-
Assert.assertNull(constructorAction);
126+
Either<Command, CodeAction> quickAssistActions = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.QUICK_ASSIST);
127+
Assert.assertNotNull(quickAssistActions);
128+
Assert.assertFalse("Generate constructors quick assist should not be available for a static field",
129+
CodeActionHandlerTest.commandExists(List.of(quickAssistActions), SourceAssistProcessor.COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT));
122130
// test for type declaration
123131
params = CodeActionUtil.constructCodeActionParams(unit, "class A");
124132
codeActions = server.codeAction(params).join();
125133
Assert.assertNotNull(codeActions);
126-
constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
127-
Assert.assertNull(constructorAction);
134+
Either<Command, CodeAction> constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
135+
// constructor should not refer to static fields
136+
Assert.assertNotNull(constructorAction);
137+
128138
}
129139

130140
@Test
131-
public void testGenerateConstructorsDisabled_emptyFields() throws JavaModelException {
141+
public void testGenerateConstructors_emptyFields() throws JavaModelException {
132142
//@formatter:off
133-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
134-
"\r\n" +
135-
"public class A {\r\n" +
136-
"}"
137-
, true, null);
143+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
144+
package p;
145+
146+
public class A {
147+
}
148+
""", true, null);
138149
//@formatter:on
139150
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "class A");
140151
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
141152
Assert.assertNotNull(codeActions);
142153
Either<Command, CodeAction> constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
143-
Assert.assertNull(constructorAction);
154+
Assert.assertNotNull(constructorAction);
155+
144156
}
145157

146158
@Test
147159
public void testGenerateConstructorsDisabled_interface() throws JavaModelException {
148160
//@formatter:off
149-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
150-
"\r\n" +
151-
"public interface A {\r\n" +
152-
" public final String name = \"test\";\r\n" +
153-
"}"
154-
, true, null);
161+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
162+
package p;
163+
164+
public interface A {
165+
final String name = "test";
166+
}
167+
""", true, null);
155168
//@formatter:on
156169
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
157170
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
@@ -162,22 +175,72 @@ public void testGenerateConstructorsDisabled_interface() throws JavaModelExcepti
162175
@Test
163176
public void testGenerateConstructorsDisabled_anonymous() throws JavaModelException {
164177
//@formatter:off
165-
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
166-
"\r\n" +
167-
"public class A {\r\n" +
168-
" public Runnable getRunnable() {\r\n" +
169-
" return new Runnable() {\r\n" +
170-
" @Override\r\n" +
171-
" public void run() {\r\n" +
172-
" }\r\n" +
173-
" };\r\n" +
174-
" }\r\n" +
175-
"}"
176-
, true, null);
178+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", """
179+
package p;
180+
181+
public class A {
182+
public Runnable getRunnable() {
183+
return new Runnable() {
184+
@Override
185+
public void run() {
186+
}
187+
};
188+
}
189+
}
190+
""", true, null);
177191
//@formatter:on
178192
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "run()");
179193
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
180194
Assert.assertNotNull(codeActions);
181195
Assert.assertFalse("The operation is not applicable to anonymous", CodeActionHandlerTest.containsKind(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS));
182196
}
197+
198+
@Test
199+
public void testGenerateConstructorsWithSuperDelegation() throws Exception {
200+
fPackageP.createCompilationUnit("A.java", """
201+
package p;
202+
203+
public class A {
204+
public A() {
205+
}
206+
public A(String a) {
207+
}
208+
}
209+
""", true, null);
210+
ICompilationUnit unitB = fPackageP.createCompilationUnit("B.java", """
211+
package p;
212+
213+
public class B extends A {
214+
}
215+
""", true, null);
216+
// Verify the code action is available
217+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unitB, "class B");
218+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
219+
Assert.assertNotNull(codeActions);
220+
Either<Command, CodeAction> constructorAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
221+
Assert.assertNotNull("Generate constructors action should be available", constructorAction);
222+
223+
// Verify quick assist is also available
224+
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
225+
Assert.assertTrue("Quick assist should be available", CodeActionHandlerTest.commandExists(quickAssistActions, SourceAssistProcessor.COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT));
226+
227+
// Generate constructors using the handler and verify they delegate to super
228+
GenerateConstructorsHandler.CheckConstructorsResponse response = GenerateConstructorsHandler.checkConstructorsStatus(params);
229+
Assert.assertNotNull(response.constructors);
230+
Assert.assertEquals("Should have 2 constructors from superclass", 2, response.constructors.length);
231+
Assert.assertNotNull(response.fields);
232+
Assert.assertEquals("Should have no fields", 0, response.fields.length);
233+
234+
CodeGenerationSettings settings = new CodeGenerationSettings();
235+
settings.createComments = false;
236+
TextEdit edit = GenerateConstructorsHandler.generateConstructors(unitB.findPrimaryType(), response.constructors, response.fields, settings, null, new NullProgressMonitor());
237+
Assert.assertNotNull(edit);
238+
JavaModelUtil.applyEdit(unitB, edit, true, null);
239+
240+
// Verify the generated constructors delegate to super
241+
String actual = unitB.getSource();
242+
Assert.assertTrue("Should contain B() constructor\n" + actual, actual.contains("public B() {"));
243+
Assert.assertTrue("Should contain B(String a) constructor\n" + actual, actual.contains("public B(String a) {"));
244+
Assert.assertTrue("Should contain super(a) call\n" + actual, actual.contains("super(a);"));
245+
}
183246
}

0 commit comments

Comments
 (0)