Skip to content

Adjust the order of code actions #2109

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 29, 2022
Merged

Adjust the order of code actions #2109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58dbf56
sort code actions
CsCherrYY Jun 1, 2022
fbceff2
sort all code actions
CsCherrYY Jun 28, 2022
21f6c19
fix tests
CsCherrYY Jun 28, 2022
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
43 changes: 37 additions & 6 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.eclipse.jdt.ls.core.internal.text.correction.AssignToVariableAssistCommandProposal;
import org.eclipse.jdt.ls.core.internal.text.correction.CUCorrectionCommandProposal;
import org.eclipse.jdt.ls.core.internal.text.correction.NonProjectFixProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.CodeActionComparator;
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.RefactoringCorrectionCommandProposal;
import org.eclipse.jdt.ls.core.internal.text.correction.SourceAssistProcessor;
Expand Down Expand Up @@ -216,6 +217,7 @@ public List<Either<Command, CodeAction>> getCodeActionCommands(CodeActionParams
return Collections.emptyList();
}

codeActions.sort(new CodeActionComparator());
populateDataFields(codeActions);
return codeActions;
}
Expand All @@ -226,15 +228,21 @@ private void populateDataFields(List<Either<Command, CodeAction>> codeActions) {
codeActions.forEach(action -> {
if (action.isRight()) {
Either<ChangeCorrectionProposal, CodeActionProposal> proposal = null;
if (action.getRight().getData() instanceof ChangeCorrectionProposal) {
proposal = Either.forLeft((ChangeCorrectionProposal) action.getRight().getData());
} else if (action.getRight().getData() instanceof CodeActionProposal) {
proposal = Either.forRight((CodeActionProposal) action.getRight().getData());
Object originalData = action.getRight().getData();
if (originalData instanceof CodeActionData) {
Object originalProposal = ((CodeActionData) originalData).getProposal();
if (originalProposal instanceof ChangeCorrectionProposal) {
proposal = Either.forLeft((ChangeCorrectionProposal) originalProposal);
} else if (originalProposal instanceof CodeActionProposal) {
proposal = Either.forRight((CodeActionProposal) originalProposal);
} else {
action.getRight().setData(null);
return;
}
} else {
action.getRight().setData(null);
return;
}

Map<String, String> data = new HashMap<>();
data.put(CodeActionResolveHandler.DATA_FIELD_REQUEST_ID, String.valueOf(response.getId()));
data.put(CodeActionResolveHandler.DATA_FIELD_PROPOSAL_ID, String.valueOf(proposals.size()));
Expand Down Expand Up @@ -277,7 +285,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(ChangeCo
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(proposal.getKind());
if (command == null) { // lazy resolve the edit.
codeAction.setData(proposal);
codeAction.setData(new CodeActionData(proposal));
} else {
codeAction.setCommand(command);
}
Expand Down Expand Up @@ -360,4 +368,27 @@ private static boolean containsKind(List<String> codeActionKinds, String baseKin
return codeActionKinds.stream().anyMatch(kind -> kind.startsWith(baseKind));
}

public static class CodeActionData {
private final Object proposal;
private final int priority;

public CodeActionData(Object proposal) {
this.proposal = proposal;
this.priority = CodeActionComparator.LOWEST_PRIORITY;
}

public CodeActionData(Object proposal, int priority) {
this.proposal = proposal;
this.priority = priority;
}

public Object getProposal() {
return proposal;
}

public int getPriority() {
return priority;
}
}

}
71 changes: 71 additions & 0 deletions ...dt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/CodeActionComparator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*******************************************************************************
* Copyright (c) 2022 Microsoft Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Microsoft Corporation - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.text.correction;

import java.util.Comparator;

import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler.CodeActionData;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionKind;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.jsonrpc.messages.Either;

public class CodeActionComparator implements Comparator<Either<Command, CodeAction>> {

public static int ORGANIZE_IMPORTS_PRIORITY = 0;
public static int GENERATE_ACCESSORS_PRIORITY = 10;
public static int GENERATE_CONSTRUCTORS_PRIORITY = 20;
public static int GENERATE_HASHCODE_EQUALS_PRIORITY = 30;
public static int GENERATE_TOSTRING_PRIORITY = 40;
public static int GENERATE_OVERRIDE_IMPLEMENT_PRIORITY = 50;
public static int GENERATE_DELEGATE_METHOD_PRIORITY = 60;
public static int CHANGE_MODIFIER_TO_FINAL_PRIORITY = 70;
public static int LOWEST_PRIORITY = 100;

public int compare(Either<Command, CodeAction> e1, Either<Command, CodeAction> e2) {
if (e1.isRight() && e2.isRight()) {
CodeAction action1 = e1.getRight();
CodeAction action2 = e2.getRight();
int kindDiff = getCodeActionKindOrdinal(action1.getKind()) - getCodeActionKindOrdinal(action2.getKind());
if (kindDiff != 0) {
return kindDiff;
}
Object data1 = action1.getData();
Object data2 = action2.getData();
if (data1 instanceof CodeActionData && data2 instanceof CodeActionData) {
int priority1 = ((CodeActionData) data1).getPriority();
int priority2 = ((CodeActionData) data2).getPriority();
return priority1 - priority2;
} else if (data1 instanceof CodeActionData) {
return 10;
} else if (data2 instanceof CodeActionData) {
return -10;
}
}
return 0;
}

private int getCodeActionKindOrdinal(String kind) {
if (kind.equals(CodeActionKind.QuickFix)) {
return 0;
} else if (kind.startsWith(CodeActionKind.Refactor)) {
return 1000;
} else if (kind.equals(JavaCodeActionKind.QUICK_ASSIST)) {
return 2000;
} else if (kind.startsWith(CodeActionKind.Source)) {
return 3000;
}
return 4000;
}
}
28 changes: 19 additions & 9 deletions ...t.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandler;
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspVariableBinding;
import org.eclipse.jdt.ls.core.internal.handlers.OrganizeImportsHandler;
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler.CodeActionData;
import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.AccessorCodeActionParams;
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
import org.eclipse.lsp4j.CodeAction;
Expand Down Expand Up @@ -145,12 +146,12 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
// Generate QuickAssist
if (isInImportDeclaration) {
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
JavaCodeActionKind.QUICK_ASSIST, organizeImportsProposal);
JavaCodeActionKind.QUICK_ASSIST, organizeImportsProposal, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY);
addSourceActionCommand($, params.getContext(), sourceOrganizeImports);
}
// Generate Source Action
Optional<Either<Command, CodeAction>> sourceOrganizeImports = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
CodeActionKind.SourceOrganizeImports, organizeImportsProposal);
CodeActionKind.SourceOrganizeImports, organizeImportsProposal, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY);
addSourceActionCommand($, params.getContext(), sourceOrganizeImports);
}

Expand Down Expand Up @@ -212,12 +213,12 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
// Generate QuickAssist
if (isInTypeDeclaration) {
Optional<Either<Command, CodeAction>> generateToStringQuickAssist = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
JavaCodeActionKind.QUICK_ASSIST, generateToStringProposal);
JavaCodeActionKind.QUICK_ASSIST, generateToStringProposal, CodeActionComparator.GENERATE_TOSTRING_PRIORITY);
addSourceActionCommand($, params.getContext(), generateToStringQuickAssist);
}
// Generate Source Action
Optional<Either<Command, CodeAction>> generateToStringCommand = getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), ActionMessages.GenerateToStringAction_label,
JavaCodeActionKind.SOURCE_GENERATE_TO_STRING, generateToStringProposal);
JavaCodeActionKind.SOURCE_GENERATE_TO_STRING, generateToStringProposal, CodeActionComparator.GENERATE_TOSTRING_PRIORITY);
addSourceActionCommand($, params.getContext(), generateToStringCommand);
}
}
Expand Down Expand Up @@ -347,6 +348,7 @@ private Optional<Either<Command, CodeAction>> getOrganizeImportsAction(CodeActio
CodeAction codeAction = new CodeAction(CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.ORGANIZE_IMPORTS_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));

Expand All @@ -362,6 +364,7 @@ private Optional<Either<Command, CodeAction>> getOverrideMethodsAction(CodeActio
CodeAction codeAction = new CodeAction(ActionMessages.OverrideMethodsAction_label);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_OVERRIDE_IMPLEMENT_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down Expand Up @@ -396,7 +399,7 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
TextEdit edit = operation.createTextEdit(pm, accessors);
return convertToWorkspaceEdit(context.getCompilationUnit(), edit);
};
return getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), actionMessage, kind, getAccessorsProposal);
return getCodeActionFromProposal(params.getContext(), context.getCompilationUnit(), actionMessage, kind, getAccessorsProposal, CodeActionComparator.GENERATE_ACCESSORS_PRIORITY);
} else {
String actionMessage;
switch (accessorKind) {
Expand All @@ -418,6 +421,7 @@ private Optional<Either<Command, CodeAction>> getGetterSetterAction(CodeActionPa
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_ACCESSORS_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down Expand Up @@ -457,6 +461,7 @@ private Optional<Either<Command, CodeAction>> getHashCodeEqualsAction(CodeAction
CodeAction codeAction = new CodeAction(ActionMessages.GenerateHashCodeEqualsAction_label);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_HASHCODE_EQUALS_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down Expand Up @@ -485,6 +490,7 @@ private Optional<Either<Command, CodeAction>> getGenerateToStringAction(CodeActi
CodeAction codeAction = new CodeAction(ActionMessages.GenerateToStringAction_ellipsisLabel);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_TOSTRING_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
Expand All @@ -511,14 +517,15 @@ private Optional<Either<Command, CodeAction>> getGenerateConstructorsAction(Code
TextEdit edit = GenerateConstructorsHandler.generateConstructors(type, status.constructors, status.fields, params.getRange(), pm);
return convertToWorkspaceEdit(type.getCompilationUnit(), edit);
};
return getCodeActionFromProposal(params.getContext(), type.getCompilationUnit(), ActionMessages.GenerateConstructorsAction_label, kind, generateConstructorsProposal);
return getCodeActionFromProposal(params.getContext(), type.getCompilationUnit(), ActionMessages.GenerateConstructorsAction_label, kind, generateConstructorsProposal, CodeActionComparator.GENERATE_CONSTRUCTORS_PRIORITY);
}

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(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_CONSTRUCTORS_PRIORITY));
codeAction.setDiagnostics(Collections.emptyList());
return Optional.of(Either.forRight(codeAction));
} else {
Expand All @@ -543,6 +550,7 @@ private Optional<Either<Command, CodeAction>> getGenerateDelegateMethodsAction(C
CodeAction codeAction = new CodeAction(ActionMessages.GenerateDelegateMethodsAction_label);
codeAction.setKind(JavaCodeActionKind.SOURCE_GENERATE_DELEGATE_METHODS);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.GENERATE_DELEGATE_METHOD_PRIORITY));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
return Optional.of(Either.forRight(codeAction));
} else {
Expand All @@ -561,7 +569,7 @@ private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleActio
if (this.preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
CodeAction codeAction = new CodeAction(ActionMessages.GenerateFinalModifiersAction_label);
codeAction.setKind(proposal.getKind());
codeAction.setData(proposal);
codeAction.setData(new CodeActionData(proposal, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
return Optional.of(Either.forRight(codeAction));
} else {
Expand All @@ -581,6 +589,7 @@ private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleActio
CodeAction codeAction = new CodeAction(ActionMessages.GenerateFinalModifiersAction_label);
codeAction.setKind(proposal.getKind());
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, CodeActionComparator.CHANGE_MODIFIER_TO_FINAL_PRIORITY));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
return Optional.of(Either.forRight(codeAction));
} else {
Expand All @@ -589,11 +598,11 @@ private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleActio
}
}

private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActionContext context, ICompilationUnit cu, String name, String kind, CodeActionProposal proposal) {
private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActionContext context, ICompilationUnit cu, String name, String kind, CodeActionProposal proposal, int priority) {
if (preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(kind);
codeAction.setData(proposal);
codeAction.setData(new CodeActionData(proposal, priority));
codeAction.setDiagnostics(Collections.EMPTY_LIST);
return Optional.of(Either.forRight(codeAction));
}
Expand All @@ -609,6 +618,7 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(CodeActi
CodeAction codeAction = new CodeAction(name);
codeAction.setKind(kind);
codeAction.setCommand(command);
codeAction.setData(new CodeActionData(null, priority));
codeAction.setDiagnostics(context.getDiagnostics());
return Optional.of(Either.forRight(codeAction));
} else {
Expand Down
4 changes: 2 additions & 2 deletions ...ts/src/org/eclipse/jdt/ls/core/internal/correction/ConvertMethodReferenceToLambaTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void testMethodReferenceToLambda() throws Exception {
Range range = new Range(new Position(4, 34), new Position(4, 34));
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu, range);
assertEquals(2, codeActions.size());
Either<Command, CodeAction> codeAction = codeActions.get(1);
Either<Command, CodeAction> codeAction = codeActions.get(0);
CodeAction action = codeAction.getRight();
assertEquals(CodeActionKind.QuickFix, action.getKind());
assertEquals("Convert to lambda expression", action.getTitle());
Expand All @@ -98,7 +98,7 @@ public void testLambdaToMethodReference() throws Exception {
Range range = new Range(new Position(4, 39), new Position(4, 39));
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu, range);
assertEquals(2, codeActions.size());
Either<Command, CodeAction> codeAction = codeActions.get(1);
Either<Command, CodeAction> codeAction = codeActions.get(0);
CodeAction action = codeAction.getRight();
assertEquals(CodeActionKind.QuickFix, action.getKind());
assertEquals("Convert to method reference", action.getTitle());
Expand Down
Loading