Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Generate Constructor to Show Fixes for type declaration #1937

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.jdt.core.ISourceRange;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.internal.core.SourceField;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.corrections.DiagnosticsHelper;
import org.eclipse.lsp4j.Range;
Expand All @@ -40,7 +41,7 @@ public static IJavaElement findInsertElement(IType type, Range cursor) {
} else if (Objects.equals(insertionLocation, INSERT_BEFORE_CURSOR)) {
return findElementAtPosition(type, cursor);
}

return findElementAfterPosition(type, cursor);
}

Expand All @@ -51,10 +52,30 @@ public static IJavaElement findInsertElement(IType type, int currentOffset) {
} else if (Objects.equals(insertionLocation, INSERT_BEFORE_CURSOR)) {
return findElementAtPosition(type, currentOffset);
}

return findElementAfterPosition(type, currentOffset);
}

public static IJavaElement findInsertElementAfterLastField(IType type) {
int lastOffset = 0;
try {
IJavaElement[] members = type.getChildren();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the results of IParent.getChildren() are in no particular order, we have to call findElementAfterPosition after we find the last field.

for (IJavaElement member : members) {
if (member instanceof SourceField) {
ISourceRange sourceRange = ((IMember) member).getSourceRange();
int offset = sourceRange.getOffset() + sourceRange.getLength();
if (offset > lastOffset) {
lastOffset = offset;
}
}
}
return findElementAfterPosition(type, lastOffset);
} catch (JavaModelException e) {
// do nothing.
}
return null;
}

private static IJavaElement findElementAtPosition(IType type, int currentOffset) {
try {
IJavaElement[] members = type.getChildren();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.NodeFinder;
import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
import org.eclipse.jdt.internal.corext.codemanipulation.AddCustomConstructorOperation;
Expand All @@ -42,6 +44,7 @@
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.dom.Bindings;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.corrections.DiagnosticsHelper;
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspMethodBinding;
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspVariableBinding;
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
Expand Down Expand Up @@ -158,13 +161,19 @@ public static TextEdit generateConstructors(IType type, LspMethodBinding[] const

ITypeBinding typeBinding = ASTNodes.getTypeBinding(astRoot, type);
if (typeBinding != null) {

ASTNode declarationNode = null;
if (cursor != null) {
ASTNode node = NodeFinder.perform(astRoot, DiagnosticsHelper.getStartOffset(type.getCompilationUnit(), cursor), DiagnosticsHelper.getLength(type.getCompilationUnit(), cursor));
declarationNode = SourceAssistProcessor.getDeclarationNode(node);
}
// If cursor position is not specified, then insert to the last by default.
IJavaElement insertPosition = (declarationNode instanceof TypeDeclaration) ? CodeGenerationUtils.findInsertElementAfterLastField(type) : CodeGenerationUtils.findInsertElement(type, cursor);

Map<String, IVariableBinding> fieldBindings = new HashMap<>();
for (IVariableBinding binding : typeBinding.getDeclaredFields()) {
fieldBindings.put(binding.getKey(), binding);
}

// If cursor position is not specified, then insert to the last by default.
IJavaElement insertPosition = CodeGenerationUtils.findInsertElement(type, cursor);
IVariableBinding[] selectedFields = Arrays.stream(fields).map(field -> fieldBindings.get(field.bindingKey)).filter(binding -> binding != null).toArray(IVariableBinding[]::new);
IMethodBinding[] superConstructors = getVisibleConstructors(astRoot, typeBinding);
TextEdit textEdit = new MultiTextEdit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,19 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
IType type = getSelectionType(context);
boolean isInTypeDeclaration = isInTypeDeclaration(context);

// Generate Constructor quickassist
Optional<Either<Command, CodeAction>> generateConstructors = null;
try {
// Generate Constructor QuickAssist
IJavaElement element = JDTUtils.findElementAtSelection(cu, params.getRange().getEnd().getLine(), params.getRange().getEnd().getCharacter(), this.preferenceManager, new NullProgressMonitor());
if (element instanceof IField) {
generateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
addSourceActionCommand($, params.getContext(), generateConstructors);
if (element instanceof IField || isInTypeDeclaration) {
Optional<Either<Command, CodeAction>> quickAssistGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.QUICK_ASSIST, monitor);
addSourceActionCommand($, params.getContext(), quickAssistGenerateConstructors);
}
} catch (JavaModelException e) {
JavaLanguageServerPlugin.logException(e);
}
// Generate Constructor Source Action
Optional<Either<Command, CodeAction>> sourceGenerateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS, monitor);
addSourceActionCommand($, params.getContext(), sourceGenerateConstructors);

// Organize Imports
if (preferenceManager.getClientPreferences().isAdvancedOrganizeImportsSupported()) {
Expand Down Expand Up @@ -195,23 +197,6 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
}
}

// Generate Constructors
if (generateConstructors == null) {
generateConstructors = getGenerateConstructorsAction(params, context, type, JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS, monitor);
} else if (generateConstructors.isPresent()) {
Command command = new Command(ActionMessages.GenerateConstructorsAction_ellipsisLabel, COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT, Collections.singletonList(params));
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS)) {
CodeAction codeAction = new CodeAction(ActionMessages.GenerateConstructorsAction_ellipsisLabel);
codeAction.setKind(JavaCodeActionKind.SOURCE_GENERATE_CONSTRUCTORS);
codeAction.setCommand(command);
codeAction.setDiagnostics(Collections.emptyList());
generateConstructors = Optional.of(Either.forRight(codeAction));
} else {
generateConstructors = Optional.of(Either.forLeft(command));
}
}
addSourceActionCommand($, params.getContext(), generateConstructors);

// Generate Delegate Methods
Optional<Either<Command, CodeAction>> generateDelegateMethods = getGenerateDelegateMethodsAction(params, context, type);
addSourceActionCommand($, params.getContext(), generateDelegateMethods);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,12 @@ public void testCodeAction_removeUnusedImport() throws Exception{
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
Assert.assertNotNull(codeActions);
Assert.assertTrue(codeActions.size() >= 3);
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(1).getRight().getKind(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(2).getRight().getKind(), CodeActionKind.SourceOrganizeImports);
List<Either<Command, CodeAction>> quickAssistActions = findActions(codeActions, CodeActionKind.QuickFix);
Assert.assertNotNull(quickAssistActions);
Assert.assertTrue(quickAssistActions.size() >= 2);
List<Either<Command, CodeAction>> organizeImportActions = findActions(codeActions, CodeActionKind.SourceOrganizeImports);
Assert.assertNotNull(organizeImportActions);
Assert.assertEquals(1, organizeImportActions.size());
Command c = codeActions.get(0).getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
}
Expand Down Expand Up @@ -475,9 +478,12 @@ public void testCodeAction_ignoringOtherDiagnosticWithoutCode() throws Exception
List<Either<Command, CodeAction>> codeActions = getCodeActions(params);
Assert.assertNotNull(codeActions);
Assert.assertTrue(codeActions.size() >= 3);
Assert.assertEquals(codeActions.get(0).getRight().getKind(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(1).getRight().getKind(), CodeActionKind.QuickFix);
Assert.assertEquals(codeActions.get(2).getRight().getKind(), CodeActionKind.SourceOrganizeImports);
List<Either<Command, CodeAction>> quickAssistActions = findActions(codeActions, CodeActionKind.QuickFix);
Assert.assertNotNull(quickAssistActions);
Assert.assertTrue(quickAssistActions.size() >= 2);
List<Either<Command, CodeAction>> organizeImportActions = findActions(codeActions, CodeActionKind.SourceOrganizeImports);
Assert.assertNotNull(organizeImportActions);
Assert.assertEquals(1, organizeImportActions.size());
Command c = codeActions.get(0).getRight().getCommand();
Assert.assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
}
Expand Down Expand Up @@ -615,7 +621,7 @@ public static List<Either<Command, CodeAction>> findActions(List<Either<Command,
}

public static boolean commandExists(List<Either<Command, CodeAction>> codeActions, String command) {
if (codeActions.isEmpty()) {
if (codeActions == null || codeActions.isEmpty()) {
return false;
}
for (Either<Command, CodeAction> codeAction : codeActions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public void testGenerateConstructorsQuickAssist() throws JavaModelException {
"}"
, true, null);
//@formatter:on
// test for field declaration
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Expand All @@ -94,6 +95,12 @@ public void testGenerateConstructorsQuickAssist() throws JavaModelException {
Command constructorCommand = CodeActionHandlerTest.getCommand(constructorAction);
Assert.assertNotNull(constructorCommand);
Assert.assertEquals(SourceAssistProcessor.COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT, constructorCommand.getCommand());
// test for type declaration
params = CodeActionUtil.constructCodeActionParams(unit, "A");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
List<Either<Command, CodeAction>> quickAssistActions = CodeActionHandlerTest.findActions(codeActions, JavaCodeActionKind.QUICK_ASSIST);
Assert.assertTrue(CodeActionHandlerTest.commandExists(quickAssistActions, SourceAssistProcessor.COMMAND_ID_ACTION_GENERATECONSTRUCTORSPROMPT));
}

@Test
Expand Down