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

Support refactoring: Convert anonymous class to nested #1178

Merged
merged 2 commits into from
Sep 17, 2019
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,7 @@
import org.eclipse.jdt.ls.core.internal.text.correction.AdvancedQuickAssistProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.CUCorrectionCommandProposal;
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;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.CodeActionContext;
Expand Down Expand Up @@ -150,6 +151,9 @@ private Optional<Either<Command, CodeAction>> getCodeActionFromProposal(ChangeCo
if (proposal instanceof CUCorrectionCommandProposal) {
CUCorrectionCommandProposal commandProposal = (CUCorrectionCommandProposal) proposal;
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else if (proposal instanceof RefactoringCorrectionCommandProposal) {
RefactoringCorrectionCommandProposal commandProposal = (RefactoringCorrectionCommandProposal) proposal;
command = new Command(name, commandProposal.getCommand(), commandProposal.getCommandArguments());
} else {
WorkspaceEdit edit = ChangeUtil.convertToWorkspaceEdit(proposal.getChange());
if (!ChangeUtil.hasChanges(edit)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.jdt.ls.core.internal.corrections.proposals.CUCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.LinkedCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.text.correction.AdvancedQuickAssistProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.RefactorProposalUtility;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Command;
Expand All @@ -40,6 +41,7 @@

public class GetRefactorEditHandler {
public static final String RENAME_COMMAND = "java.action.rename";
private static final String DEFAULT_POSITION_KEY = "name";

public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams params) {
final ICompilationUnit unit = JDTUtils.resolveCompilationUnit(params.context.getTextDocument().getUri());
Expand All @@ -52,6 +54,7 @@ public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams pa
context.setASTRoot(CodeActionHandler.getASTRoot(unit));
IProblemLocationCore[] locations = CodeActionHandler.getProblemLocationCores(unit, params.context.getContext().getDiagnostics());
boolean problemsAtLocation = locations.length != 0;
String positionKey = DEFAULT_POSITION_KEY;

try {
Map formatterOptions = params.options == null ? null : FormatterHandler.getOptions(params.options, unit);
Expand All @@ -69,6 +72,9 @@ public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams pa
proposal = (LinkedCorrectionProposal) RefactorProposalUtility.getExtractFieldProposal(params.context, context, problemsAtLocation, formatterOptions, initializeIn, false);
} else if (AdvancedQuickAssistProcessor.INVERT_VARIABLE_COMMAND.equals(params.command)) {
proposal = (LinkedCorrectionProposal) AdvancedQuickAssistProcessor.getInvertVariableProposal(params.context, context, context.getCoveringNode(), false);
} else if (QuickAssistProcessor.CONVERT_ANONYMOUS_CLASS_TO_NESTED_COMMAND.equals(params.command)) {
proposal = QuickAssistProcessor.getConvertAnonymousToNestedProposal(params.context, context, context.getCoveringNode(), false);
positionKey = "type_name";
}

if (proposal == null) {
Expand All @@ -80,8 +86,13 @@ public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams pa
LinkedProposalModelCore linkedProposalModel = proposal.getLinkedProposalModel();
Command additionalCommand = null;
if (linkedProposalModel != null) {
LinkedProposalPositionGroupCore linkedPositionGroup = linkedProposalModel.getPositionGroup("name", false);
PositionInformation highlightPosition = getFirstTrackedNodePosition(linkedPositionGroup);
LinkedProposalPositionGroupCore linkedPositionGroup = linkedProposalModel.getPositionGroup(positionKey, false);
PositionInformation highlightPosition;
if (QuickAssistProcessor.CONVERT_ANONYMOUS_CLASS_TO_NESTED_COMMAND.equals(params.command)) {
highlightPosition = getFirstTrackedNodePositionBySequenceRank(linkedPositionGroup);
} else {
highlightPosition = getFirstTrackedNodePosition(linkedPositionGroup);
}
if (highlightPosition != null) {
int offset = highlightPosition.getOffset();
int length = highlightPosition.getLength();
Expand Down Expand Up @@ -111,6 +122,26 @@ private static PositionInformation getFirstTrackedNodePosition(LinkedProposalPos
return positions[0];
}

private static PositionInformation getFirstTrackedNodePositionBySequenceRank(LinkedProposalPositionGroupCore positionGroup) {
Copy link
Contributor Author

@jdneo jdneo Sep 15, 2019

Choose a reason for hiding this comment

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

@fbricon @testforstephen

I think we should leverage the sequence rank information to get the first position in the position group.

If that is also the case in other proposals, we should avoid to get the first position by the array index since it's not reliable if any code refactor happens in the upstream side (jdt.core.manipulation)

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdneo ok so can you fix it in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fbricon How about addressing this in another PR?

Since I need some time to check if the sequence rank is also workable in the following proposals:

  • extract varaible
  • extract variable (all occurrence)
  • extract constant
  • extract method
  • extract field
  • invert variable

I can file another issue to track for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdneo fine, you can open a separate issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's here: #1180

if (positionGroup == null) {
return null;
}

PositionInformation[] positions = positionGroup.getPositions();
if (positions == null || positions.length == 0) {
return null;
}

PositionInformation targetPosition = positions[0];

for (int i = 1; i < positions.length; i++) {
if (positions[i].getSequenceRank() < targetPosition.getSequenceRank()) {
targetPosition = positions[i];
}
}
return targetPosition;
}

private static CUCorrectionProposal getExtractVariableProposal(CodeActionParams params, IInvocationContext context, boolean problemsAtLocation, String refactorType, Map formatterOptions) throws CoreException {
if (RefactorProposalUtility.EXTRACT_VARIABLE_ALL_OCCURRENCE_COMMAND.equals(refactorType)) {
return RefactorProposalUtility.getExtractVariableAllOccurrenceProposal(params, context, problemsAtLocation, formatterOptions, false);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2017, 2019 IBM Corporation and others.
* Copyright (c) 2000, 2019 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
Expand Down Expand Up @@ -2049,7 +2049,6 @@ public static CUCorrectionProposal getInvertVariableProposal(CodeActionParams pa
Arrays.asList(INVERT_VARIABLE_COMMAND, params));
}

List<CUCorrectionProposal> proposals = new ArrayList<>();
final AST ast = covering.getAST();
// find linked nodes
final MethodDeclaration method = ASTResolving.findParentMethodDeclaration(covering);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2017 IBM Corporation and others.
* Copyright (c) 2000, 2019 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
Expand Down Expand Up @@ -30,10 +30,13 @@
import java.util.Map;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaModelMarker;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.IMethod;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.dom.AST;
Expand Down Expand Up @@ -109,7 +112,9 @@
import org.eclipse.jdt.internal.corext.fix.IProposableFix;
import org.eclipse.jdt.internal.corext.fix.LambdaExpressionsFixCore;
import org.eclipse.jdt.internal.corext.fix.LinkedProposalModelCore;
import org.eclipse.jdt.internal.corext.refactoring.code.ConvertAnonymousToNestedRefactoring;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.internal.corext.util.Messages;
import org.eclipse.jdt.internal.ui.fix.AbstractCleanUpCore;
import org.eclipse.jdt.internal.ui.fix.LambdaExpressionsCleanUpCore;
import org.eclipse.jdt.internal.ui.fix.MultiFixMessages;
Expand All @@ -123,6 +128,7 @@
import org.eclipse.jdt.ls.core.internal.corrections.proposals.ChangeCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.FixCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.IProposalRelevance;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.RefactoringCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.TypeChangeCorrectionProposal;
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
import org.eclipse.jface.text.link.LinkedPositionGroup;
Expand All @@ -148,6 +154,8 @@ public class QuickAssistProcessor {
public static final String CONVERT_TO_MESSAGE_FORMAT_ID = "org.eclipse.jdt.ls.correction.convertToMessageFormat.assist"; //$NON-NLS-1$;
public static final String EXTRACT_METHOD_INPLACE_ID = "org.eclipse.jdt.ls.correction.extractMethodInplace.assist"; //$NON-NLS-1$;

public static final String CONVERT_ANONYMOUS_CLASS_TO_NESTED_COMMAND = "convertAnonymousClassToNestedCommand";

private PreferenceManager preferenceManager;

public QuickAssistProcessor(PreferenceManager preferenceManager) {
Expand Down Expand Up @@ -191,7 +199,7 @@ public List<CUCorrectionProposal> getAssists(CodeActionParams params, IInvocatio
getExtractFieldProposal(params, context, problemsAtLocation, resultingCollections);
// getInlineLocalProposal(context, coveringNode, resultingCollections);
// getConvertLocalToFieldProposal(context, coveringNode, resultingCollections);
// getConvertAnonymousToNestedProposal(context, coveringNode, resultingCollections);
getConvertAnonymousToNestedProposals(params, context, coveringNode, resultingCollections);
getConvertAnonymousClassCreationsToLambdaProposals(context, coveringNode, resultingCollections);
getConvertLambdaToAnonymousClassCreationsProposals(context, coveringNode, resultingCollections);
// getChangeLambdaBodyToBlockProposal(context, coveringNode, resultingCollections);
Expand Down Expand Up @@ -648,6 +656,78 @@ private boolean getExtractFieldProposal(CodeActionParams params, IInvocationCont
return true;
}

private boolean getConvertAnonymousToNestedProposals(CodeActionParams params, IInvocationContext context, final ASTNode node, Collection<CUCorrectionProposal> proposals) throws CoreException {
if (!(node instanceof Name)) {
return false;
}

if (proposals == null) {
return false;
}

RefactoringCorrectionProposal proposal = null;
if (this.preferenceManager.getClientPreferences().isAdvancedExtractRefactoringSupported()) {
proposal = getConvertAnonymousToNestedProposal(params, context, node, true /*returnAsCommand*/);
} else {
proposal = getConvertAnonymousToNestedProposal(params, context, node, false /*returnAsCommand*/);
}

if (proposal == null) {
return false;
}

proposals.add(proposal);
return true;
}

public static RefactoringCorrectionProposal getConvertAnonymousToNestedProposal(CodeActionParams params, IInvocationContext context, final ASTNode node, boolean returnAsCommand) throws CoreException {
String label = CorrectionMessages.QuickAssistProcessor_convert_anonym_to_nested;
ASTNode normalized = ASTNodes.getNormalizedNode(node);
if (normalized.getLocationInParent() != ClassInstanceCreation.TYPE_PROPERTY) {
return null;
}

final AnonymousClassDeclaration anonymTypeDecl = ((ClassInstanceCreation) normalized.getParent()).getAnonymousClassDeclaration();
if (anonymTypeDecl == null || anonymTypeDecl.resolveBinding() == null) {
return null;
}

final ConvertAnonymousToNestedRefactoring refactoring = new ConvertAnonymousToNestedRefactoring(anonymTypeDecl);
if (!refactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
return null;
}

if (returnAsCommand) {
return new RefactoringCorrectionCommandProposal(label, CodeActionKind.Refactor, context.getCompilationUnit(), IProposalRelevance.CONVERT_ANONYMOUS_TO_NESTED, RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID,
Arrays.asList(CONVERT_ANONYMOUS_CLASS_TO_NESTED_COMMAND, params));
}

String extTypeName = ASTNodes.getSimpleNameIdentifier((Name) node);
ITypeBinding anonymTypeBinding = anonymTypeDecl.resolveBinding();
String className;
if (anonymTypeBinding.getInterfaces().length == 0) {
className = Messages.format(CorrectionMessages.QuickAssistProcessor_name_extension_from_interface, extTypeName);
} else {
className = Messages.format(CorrectionMessages.QuickAssistProcessor_name_extension_from_class, extTypeName);
}
String[][] existingTypes = ((IType) anonymTypeBinding.getJavaElement()).resolveType(className);
int i = 1;
while (existingTypes != null) {
i++;
existingTypes = ((IType) anonymTypeBinding.getJavaElement()).resolveType(className + i);
}
refactoring.setClassName(i == 1 ? className : className + i);

LinkedProposalModelCore linkedProposalModel = new LinkedProposalModelCore();
refactoring.setLinkedProposalModel(linkedProposalModel);

final ICompilationUnit cu = context.getCompilationUnit();
RefactoringCorrectionProposal proposal = new RefactoringCorrectionProposal(label, CodeActionKind.Refactor, cu, refactoring, IProposalRelevance.CONVERT_ANONYMOUS_TO_NESTED);
proposal.setLinkedProposalModel(linkedProposalModel);
return proposal;

}

private static boolean getConvertAnonymousClassCreationsToLambdaProposals(IInvocationContext context, ASTNode covering, Collection<CUCorrectionProposal> resultingCollections) {
while (covering instanceof Name || covering instanceof Type || covering instanceof Dimension || covering.getParent() instanceof MethodDeclaration
|| covering.getLocationInParent() == AnonymousClassDeclaration.BODY_DECLARATIONS_PROPERTY) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*******************************************************************************
* Copyright (c) 2019 Microsoft Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Microsoft Corporation - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.text.correction;

import java.util.List;

import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.ls.core.internal.corrections.proposals.RefactoringCorrectionProposal;

public class RefactoringCorrectionCommandProposal extends RefactoringCorrectionProposal {

private String command;
private List<Object> commandArguments;

/**
* @param name
* @param kind
* @param cu
* @param relevance
*/
public RefactoringCorrectionCommandProposal(String name, String kind, ICompilationUnit cu, int relevance, String command, List<Object> commandArguments) {
super(name, kind, cu, null, relevance);
this.command = command;
this.commandArguments = commandArguments;
}

public String getCommand() {
return command;
}

public List<Object> getCommandArguments() {
return commandArguments;
}

}
Loading