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

Show "change modifiers to final" in quick assists #2134

Merged
merged 3 commits into from
Jun 29, 2022
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 @@ -39,6 +39,8 @@ private ActionMessages() {
public static String GenerateConstructorsAction_ellipsisLabel;
public static String GenerateDelegateMethodsAction_label;
public static String GenerateFinalModifiersAction_label;
public static String GenerateFinalModifiersAction_templateLabel;
public static String GenerateFinalModifiersAction_selectionLabel;
public static String MoveRefactoringAction_label;
public static String MoveRefactoringAction_templateLabel;
public static String InlineMethodRefactoringAction_label;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ GenerateConstructorsAction_label=Generate Constructors
GenerateConstructorsAction_ellipsisLabel=Generate Constructors...
GenerateDelegateMethodsAction_label=Generate Delegate Methods...
GenerateFinalModifiersAction_label=Change modifiers to final where possible
GenerateFinalModifiersAction_templateLabel=Add final modifier for ''{0}''
GenerateFinalModifiersAction_selectionLabel=Change modifiers to final
MoveRefactoringAction_label=Move...
MoveRefactoringAction_templateLabel=Move ''{0}''...
InlineMethodRefactoringAction_label=Inline Method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import org.eclipse.jdt.core.dom.FieldDeclaration;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.Statement;
import org.eclipse.jdt.core.dom.VariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;

public class CodeActionUtility {

Expand Down Expand Up @@ -100,4 +102,21 @@ public static List<String> getFieldNamesFromASTNode(ASTNode node) {
}
return Collections.emptyList();
}

public static List<String> getVariableNamesFromASTNode(ASTNode node) {
if (node instanceof VariableDeclaration) {
SimpleName name = ((VariableDeclaration) node).getName();
if (name != null) {
return Arrays.asList(name.getIdentifier());
}
} else if (node instanceof VariableDeclarationStatement) {
List<String> names = new ArrayList<>();
List<VariableDeclarationFragment> fragments = ((VariableDeclarationStatement) node).fragments();
for (VariableDeclarationFragment fragment : fragments) {
names.addAll(CodeActionUtility.getFieldNamesFromASTNode(fragment));
}
return names;
}
return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
Expand All @@ -42,6 +43,8 @@
import org.eclipse.jdt.core.dom.ImportDeclaration;
import org.eclipse.jdt.core.dom.Statement;
import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
import org.eclipse.jdt.core.manipulation.OrganizeImportsOperation;
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
Expand Down Expand Up @@ -226,6 +229,9 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
Optional<Either<Command, CodeAction>> generateFinalModifiers = addFinalModifierWherePossibleAction(context);
addSourceActionCommand($, params.getContext(), generateFinalModifiers);

Optional<Either<Command, CodeAction>> generateFinalModifiersQuickAssist = addFinalModifierWherePossibleQuickAssist(context);
addSourceActionCommand($, params.getContext(), generateFinalModifiersQuickAssist);

return $;
}

Expand Down Expand Up @@ -528,14 +534,52 @@ private Optional<Either<Command, CodeAction>> getGenerateDelegateMethodsAction(C

private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleAction(IInvocationContext context) {
IProposableFix fix = (IProposableFix) VariableDeclarationFixCore.createCleanUp(context.getASTRoot(), true, true, true);
return getFinalModifierWherePossibleAction(context, fix, ActionMessages.GenerateFinalModifiersAction_label, JavaCodeActionKind.SOURCE_GENERATE_FINAL_MODIFIERS);
}

private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleQuickAssist(IInvocationContext context) {
ASTNode coveringNode = context.getCoveringNode();
List<ASTNode> coveredNodes = QuickAssistProcessor.getFullyCoveredNodes(context, coveringNode);
List<ASTNode> possibleASTNodes = getPossibleASTNodesForFinalModifier(coveredNodes);
if (possibleASTNodes.size() == 0) {
possibleASTNodes = getPossibleASTNodesForFinalModifier(Arrays.asList(coveringNode));
}
Set<String> names = new HashSet<>();
for (ASTNode node : possibleASTNodes) {
names.addAll(CodeActionUtility.getVariableNamesFromASTNode(node));
}
String actionMessage = ActionMessages.GenerateFinalModifiersAction_selectionLabel;
if (names.size() == 1) {
actionMessage = Messages.format(ActionMessages.GenerateFinalModifiersAction_templateLabel, names.iterator().next());
}
IProposableFix fix = (IProposableFix) VariableDeclarationFixCore.createChangeModifierToFinalFix(context.getASTRoot(), possibleASTNodes.toArray(new ASTNode[0]));
return getFinalModifierWherePossibleAction(context, fix, actionMessage, JavaCodeActionKind.QUICK_ASSIST);
}

private List<ASTNode> getPossibleASTNodesForFinalModifier(List<ASTNode> targetNodes) {
List<ASTNode> results = new ArrayList<>();
for (ASTNode targetNode : targetNodes) {
ASTNode variableDeclaration = CodeActionUtility.inferASTNode(targetNode, VariableDeclaration.class);
ASTNode fieldDeclaration = CodeActionUtility.inferASTNode(targetNode, FieldDeclaration.class);
ASTNode variableDeclarationStatement = CodeActionUtility.inferASTNode(targetNode, VariableDeclarationStatement.class);
if (variableDeclaration != null) {
results.add(variableDeclaration);
} else if (fieldDeclaration != null) {
results.addAll(((FieldDeclaration) fieldDeclaration).fragments());
} else if (variableDeclarationStatement != null) {
results.add(variableDeclarationStatement);
}
}
return results;
}

private Optional<Either<Command, CodeAction>> getFinalModifierWherePossibleAction(IInvocationContext context, IProposableFix fix, String actionMessage, String kind) {
if (fix == null) {
return Optional.empty();
}

FixCorrectionProposal proposal = new FixCorrectionProposal(fix, null, IProposalRelevance.MAKE_VARIABLE_DECLARATION_FINAL, context, JavaCodeActionKind.SOURCE_GENERATE_FINAL_MODIFIERS);
FixCorrectionProposal proposal = new FixCorrectionProposal(fix, null, IProposalRelevance.MAKE_VARIABLE_DECLARATION_FINAL, context, kind);
if (this.preferenceManager.getClientPreferences().isResolveCodeActionSupported()) {
CodeAction codeAction = new CodeAction(ActionMessages.GenerateFinalModifiersAction_label);
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(proposal.getKind());
codeAction.setData(proposal);
codeAction.setDiagnostics(Collections.EMPTY_LIST);
Expand All @@ -552,9 +596,9 @@ private Optional<Either<Command, CodeAction>> addFinalModifierWherePossibleActio
if (!ChangeUtil.hasChanges(edit)) {
return Optional.empty();
}
Command command = new Command(ActionMessages.GenerateFinalModifiersAction_label, CodeActionHandler.COMMAND_ID_APPLY_EDIT, Collections.singletonList(edit));
Command command = new Command(actionMessage, CodeActionHandler.COMMAND_ID_APPLY_EDIT, Collections.singletonList(edit));
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(proposal.getKind())) {
CodeAction codeAction = new CodeAction(ActionMessages.GenerateFinalModifiersAction_label);
CodeAction codeAction = new CodeAction(actionMessage);
codeAction.setKind(proposal.getKind());
codeAction.setCommand(command);
codeAction.setDiagnostics(Collections.EMPTY_LIST);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/*******************************************************************************
* 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.handlers;

import static org.junit.Assert.assertNotNull;

import java.util.List;

import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.IPackageFragment;
import org.eclipse.jdt.core.IPackageFragmentRoot;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.ls.core.internal.CodeActionUtil;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public class GenerateFinalModifiersQuickAssistTest extends AbstractCompilationUnitBasedTest {

private IJavaProject fJavaProject;
private IPackageFragmentRoot fRoot;
private IPackageFragment fPackageP;

@Override
@Before
public void setup() throws Exception {
fJavaProject = newEmptyProject();
fRoot = fJavaProject.findPackageFragmentRoot(fJavaProject.getPath().append("src"));
assertNotNull(fRoot);
fPackageP = fRoot.createPackageFragment("p", true, null);
server = new JDTLanguageServer(projectsManager, this.preferenceManager);
}

@Test
public void testInsertFinalModifierForFieldDeclarationQuickAssist() throws JavaModelException {
//@formatter:off
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
"\r\n" +
"public class A {\r\n" +
" private String name = \"a\";\r\n" +
" private String test;\r\n" +
"}"
, true, null);
//@formatter:on
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "String name");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'name'"));
params = CodeActionUtil.constructCodeActionParams(unit, "a\";");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'name'"));
params = CodeActionUtil.constructCodeActionParams(unit, "test;");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'test'"));
}

@Test
public void testInsertFinalModifierForFieldDeclarationsQuickAssist() throws JavaModelException {
//@formatter:off
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
"\r\n" +
"public class A {\r\n" +
" private String name;\r\n" +
" private String name1 = \"b\";\r\n" +
"}"
, true, null);
//@formatter:on
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "private String name;\r\n private String name1 = \"b\";");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Change modifiers to final"));
}

@Test
public void testInsertFinalModifierForParameterQuickAssist() throws JavaModelException {
//@formatter:off
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
"\r\n" +
"public class A {\r\n" +
" public String getName(String a, String b) {}\r\n" +
"}"
, true, null);
//@formatter:on
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "b");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'b'"));
params = CodeActionUtil.constructCodeActionParams(unit, "String a, String b");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Change modifiers to final"));
}

@Test
public void testInsertFinalModifierForLocalVariableSelectionQuickAssist() throws JavaModelException {
//@formatter:off
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
"\r\n" +
"public class A {\r\n" +
" public String getName(String a, String b) {\r\n" +
" String c = a;\r\n" +
" String d = b;\r\n" +
" }\r\n" +
"}"
, true, null);
//@formatter:on
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "c");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'c'"));
params = CodeActionUtil.constructCodeActionParams(unit, "String c = a;\r\n String d = b;");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Change modifiers to final"));
params = CodeActionUtil.constructCodeActionParams(unit, "c = a;\r\n String d = b;");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Change modifiers to final"));
}

@Test
public void testInsertFinalModifierForLocalVariableQuickAssist() throws JavaModelException {
//@formatter:off
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
"\r\n" +
"public class A {\r\n" +
" public String getName() {\r\n" +
" String c;\r\n" +
" String d, e;\r\n" +
" }\r\n" +
"}"
, true, null);
//@formatter:on
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "c;");
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'c'"));
params = CodeActionUtil.constructCodeActionParams(unit, "String d, e;");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Change modifiers to final"));
params = CodeActionUtil.constructCodeActionParams(unit, "d");
codeActions = server.codeAction(params).join();
Assert.assertNotNull(codeActions);
Assert.assertTrue(CodeActionHandlerTest.commandExists(codeActions, CodeActionHandler.COMMAND_ID_APPLY_EDIT, "Add final modifier for 'd'"));
}
}