From 7e2db60abdfeac144d7baa0e4a9035bea4395a89 Mon Sep 17 00:00:00 2001 From: anna Date: Tue, 21 Feb 2012 20:25:43 +0100 Subject: [PATCH] create constructor parameter from field: suggest to choose fields to create parameters from (IDEA-76417); insert parameters in "fields" order (IDEA-81634) --- .../ChangeMethodSignatureFromUsageFix.java | 51 +++-- ...reateConstructorParameterFromFieldFix.java | 185 +++++++++++++----- .../impl/AssignFieldFromParameterAction.java | 2 +- .../impl/CreateFieldFromParameterAction.java | 10 +- .../afterChainedCalls.java | 6 +- 5 files changed, 178 insertions(+), 76 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeMethodSignatureFromUsageFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeMethodSignatureFromUsageFix.java index 7401ad126927..ddce0ac7c902 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeMethodSignatureFromUsageFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeMethodSignatureFromUsageFix.java @@ -178,11 +178,27 @@ public class ChangeMethodSignatureFromUsageFix implements IntentionAction, HighP final PsiMethod method = SuperMethodWarningUtil.checkSuperMethod(myTargetMethod, RefactoringBundle.message("to.refactor")); if (method == null) return; - if (!CodeInsightUtilBase.prepareFileForWrite(method.getContainingFile())) return; + myNewParametersInfo = getNewParametersInfo(myExpressions, myTargetMethod, mySubstitutor); + final List parameterInfos = + performChange(project, editor, file, method, myMinUsagesNumberToShowDialog, myNewParametersInfo, myChangeAllUsages, false); + if (parameterInfos != null) { + myNewParametersInfo = parameterInfos.toArray(new ParameterInfoImpl[parameterInfos.size()]); + } + } + + public static List performChange(final Project project, + final Editor editor, + final PsiFile file, + final PsiMethod method, + final int minUsagesNumber, + final ParameterInfoImpl[] newParametersInfo, + final boolean changeAllUsages, + final boolean allowDelegation) { + if (!CodeInsightUtilBase.prepareFileForWrite(method.getContainingFile())) return null; final FindUsagesManager findUsagesManager = ((FindManagerImpl)FindManager.getInstance(project)).getFindUsagesManager(); final FindUsagesHandler handler = findUsagesManager.getFindUsagesHandler(method, false); - if (handler == null) return; //on failure or cancel (e.g. cancel of super methods dialog) + if (handler == null) return null;//on failure or cancel (e.g. cancel of super methods dialog) final JavaMethodFindUsagesOptions options = new JavaMethodFindUsagesOptions(project); options.isImplementingMethods = true; @@ -196,7 +212,7 @@ public class ChangeMethodSignatureFromUsageFix implements IntentionAction, HighP Processor processor = new Processor() { @Override public boolean process(final UsageInfo t) { - return ++usagesFound[0] < myMinUsagesNumberToShowDialog; + return ++usagesFound[0] < minUsagesNumber; } }; @@ -204,21 +220,20 @@ public class ChangeMethodSignatureFromUsageFix implements IntentionAction, HighP } }; String progressTitle = QuickFixBundle.message("searching.for.usages.progress.title"); - if (!ProgressManager.getInstance().runProcessWithProgressSynchronously(runnable, progressTitle, true, project)) return; + if (!ProgressManager.getInstance().runProcessWithProgressSynchronously(runnable, progressTitle, true, project)) return null; - myNewParametersInfo = getNewParametersInfo(myExpressions, myTargetMethod, mySubstitutor); - if (ApplicationManager.getApplication().isUnitTestMode() || usagesFound[0] < myMinUsagesNumberToShowDialog) { + if (ApplicationManager.getApplication().isUnitTestMode() || usagesFound[0] < minUsagesNumber) { ChangeSignatureProcessor processor = new ChangeSignatureProcessor( project, method, false, null, method.getName(), method.getReturnType(), - myNewParametersInfo){ + newParametersInfo){ @Override @NotNull protected UsageInfo[] findUsages() { - return myChangeAllUsages ? super.findUsages() : UsageInfo.EMPTY_ARRAY; + return changeAllUsages ? super.findUsages() : UsageInfo.EMPTY_ARRAY; } }; processor.run(); @@ -230,25 +245,21 @@ public class ChangeMethodSignatureFromUsageFix implements IntentionAction, HighP }); } else { - List parameterInfos = myNewParametersInfo != null - ? new ArrayList(Arrays.asList(myNewParametersInfo)) + List parameterInfos = newParametersInfo != null + ? new ArrayList(Arrays.asList(newParametersInfo)) : new ArrayList(); final PsiReferenceExpression refExpr = TargetElementUtil.findReferenceExpression(editor); - JavaChangeSignatureDialog dialog = new JavaChangeSignatureDialog(project, method, allowDelegate(), refExpr); + JavaChangeSignatureDialog dialog = new JavaChangeSignatureDialog(project, method, allowDelegation, refExpr); dialog.setParameterInfos(parameterInfos); dialog.show(); - List parameters = dialog.getParameters(); - myNewParametersInfo = parameters.toArray(new ParameterInfoImpl[parameters.size()]); + return dialog.getParameters(); } + return null; } - protected boolean allowDelegate() { - return false; - } - - public String getNewParameterNameByOldIndex(int oldIndex) { - if (myNewParametersInfo == null) return null; - for (ParameterInfoImpl info : myNewParametersInfo) { + public static String getNewParameterNameByOldIndex(int oldIndex, final ParameterInfoImpl[] parametersInfo) { + if (parametersInfo == null) return null; + for (ParameterInfoImpl info : parametersInfo) { if (info.oldParameterIndex == oldIndex) { return info.getName(); } diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java index 01b775f16b30..1f4e18cb3a7e 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateConstructorParameterFromFieldFix.java @@ -19,28 +19,33 @@ import com.intellij.codeInsight.CodeInsightUtilBase; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.QuickFixBundle; import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; +import com.intellij.codeInsight.generation.PsiElementClassMember; +import com.intellij.codeInsight.generation.PsiFieldMember; import com.intellij.codeInsight.generation.PsiMethodMember; -import com.intellij.codeInsight.hint.HintManager; import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInsight.intention.impl.AssignFieldFromParameterAction; +import com.intellij.codeInsight.intention.impl.CreateFieldFromParameterAction; import com.intellij.ide.util.MemberChooser; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.Key; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.codeStyle.SuggestedNameInfo; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.impl.source.jsp.jspJava.JspClass; import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; +import com.intellij.refactoring.changeSignature.ParameterInfoImpl; import com.intellij.refactoring.util.RefactoringUtil; -import com.intellij.util.ArrayUtil; +import com.intellij.util.Function; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ConcurrentWeakHashMap; import org.jetbrains.annotations.NotNull; @@ -143,25 +148,40 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { } else if (!constrs.isEmpty()) { final Collection> fieldsToFix = getFieldsToFix(); - final PsiMethod constructor = constrs.get(0); - final List fields = new ArrayList(); - for (SmartPsiElementPointer elementPointer : fieldsToFix) { - final PsiField field = elementPointer.getElement(); - if (field != null && isAvailable(field) && filterConstructorsIfFieldAlreadyAssigned(new PsiMethod[]{constructor}, field).contains(constructor)) { - fields.add(field); + try { + final PsiMethod constructor = constrs.get(0); + final List fields = new ArrayList(); + for (SmartPsiElementPointer elementPointer : fieldsToFix) { + final PsiField field = elementPointer.getElement(); + if (field != null && isAvailable(field) && filterConstructorsIfFieldAlreadyAssigned(new PsiMethod[]{constructor}, field).contains(constructor)) { + fields.add(field); + } + } + if (constrs.size() == constructors.length && fields.size() > 1 && !ApplicationManager.getApplication().isUnitTestMode()) { + PsiFieldMember[] members = new PsiFieldMember[fields.size()]; + int i = 0; + for (PsiField field : fields) { + members[i++] = new PsiFieldMember(field); + } + MemberChooser chooser = new MemberChooser(members, false, true, project); + chooser.setTitle("Choose Fields to Generate Constructor Parameters for"); + chooser.show(); + if (chooser.getExitCode() != DialogWrapper.OK_EXIT_CODE) return; + final List selectedElements = chooser.getSelectedElements(); + if (selectedElements == null) return; + fields.clear(); + for (PsiElementClassMember member : selectedElements) { + fields.add((PsiField)member.getElement()); + } } - } - Collections.sort(fields, new Comparator() { - @Override - public int compare(PsiField o1, PsiField o2) { - return o1.getTextOffset() - o2.getTextOffset(); - } - }); - addParameterToConstructor(project, file, editor, constructor, constrs.size() == constructors.length - ? fields.toArray(new PsiField[fields.size()]) - : new PsiField[]{getField()}); - fieldsToFix.clear(); + addParameterToConstructor(project, file, editor, constructor, constrs.size() == constructors.length + ? fields.toArray(new PsiField[fields.size()]) + : new PsiField[]{getField()}); + } + finally { + fieldsToFix.clear(); + } } } @@ -211,45 +231,81 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { final Editor editor, final PsiMethod constructor, final PsiField[] fields) throws IncorrectOperationException { - final PsiParameter[] parameters = constructor.getParameterList().getParameters(); - PsiExpression[] expressions = new PsiExpression[parameters.length+fields.length]; - PsiElementFactory factory = JavaPsiFacade.getInstance(file.getProject()).getElementFactory(); + final PsiParameterList parameterList = constructor.getParameterList(); + final PsiParameter[] parameters = parameterList.getParameters(); + ParameterInfoImpl[] newParamInfos = new ParameterInfoImpl[parameters.length + fields.length]; + final List params = new ArrayList(Arrays.asList(parameters)); + Collections.addAll(params, fields); + Collections.sort(params, new FieldParameterComparator(parameterList)); + int i = 0; - for (; i < parameters.length; i++) { - PsiParameter parameter = parameters[i]; - String value = PsiTypesUtil.getDefaultValueOfType(parameter.getType()); - expressions[i] = factory.createExpressionFromText(value, parameter); - } - for (PsiField field : fields) { - expressions[i++] = factory.createExpressionFromText(field.getName(), constructor); - } - if (constructor.isVarArgs()) { - ArrayUtil.rotateLeft(expressions, parameters.length - 1, expressions.length - 1); + for (PsiVariable param : params) { + final PsiType paramType = param.getType(); + if (param instanceof PsiParameter) { + newParamInfos[i++] = new ParameterInfoImpl(parameterList.getParameterIndex((PsiParameter)param), param.getName(), paramType, param.getName()); + } else { + final String uniqueParameterName = getUniqueParameterName(parameters, param); + newParamInfos[i++] = new ParameterInfoImpl(-1, uniqueParameterName, paramType, uniqueParameterName); + } } final SmartPointerManager manager = SmartPointerManager.getInstance(project); final SmartPsiElementPointer constructorPointer = manager.createSmartPsiElementPointer(constructor); - final ChangeMethodSignatureFromUsageFix addParamFix = new ChangeMethodSignatureFromUsageFix(constructor, expressions, PsiSubstitutor.EMPTY, constructor, true, 1){ - @Override - protected boolean allowDelegate() { - return true; - } - }; - if (addParamFix.isAvailable(project, editor, file)) { - addParamFix.invoke(project, editor, file); - } else if (addParamFix.isMethodSignatureExists() && !ApplicationManager.getApplication().isUnitTestMode()) { - HintManager.getInstance().showErrorHint(editor, "Constructor with corresponding signature already exist"); - } + final PsiMethod fromText = JavaPsiFacade.getElementFactory(project).createMethodFromText(createDummyMethod(constructor, newParamInfos), + constructor); + final PsiClass containingClass = constructor.getContainingClass(); + if (containingClass == null) return false; + final int minUsagesNumber = containingClass.findMethodsBySignature(fromText, false).length > 0 ? 0 : 1; + final List parameterInfos = + ChangeMethodSignatureFromUsageFix.performChange(project, editor, file, constructor, minUsagesNumber, newParamInfos, true, true); + + final ParameterInfoImpl[] resultParams = parameterInfos != null ? parameterInfos.toArray(new ParameterInfoImpl[parameterInfos.size()]) : + newParamInfos; return ApplicationManager.getApplication().runWriteAction(new Computable() { @Override public Boolean compute() { - return doCreate(project, editor, parameters, constructorPointer, addParamFix, fields); + return doCreate(project, editor, parameters, constructorPointer, resultParams, fields); } }); } + private static String createDummyMethod(PsiMethod constructor, ParameterInfoImpl[] newParamInfos) { + return constructor.getName() + "(" + StringUtil.join(newParamInfos, new Function() { + @Override + public String fun(ParameterInfoImpl info) { + return info.getTypeText() + " " + info.getName(); + } + }, ", ") + "){}"; + } + + private static String getUniqueParameterName(PsiParameter[] parameters, PsiVariable variable) { + final JavaCodeStyleManager styleManager = JavaCodeStyleManager.getInstance(variable.getProject()); + final SuggestedNameInfo nameInfo = styleManager + .suggestVariableName(VariableKind.PARAMETER, + styleManager.variableNameToPropertyName(variable.getName(), VariableKind.FIELD), + null, variable.getType()); + String newName = nameInfo.names[0]; + int n = 1; + while (true) { + if (isUnique(parameters, newName)) { + break; + } + newName = nameInfo.names[0] + n++; + } + return newName; + } + + private static boolean isUnique(PsiParameter[] params, String newName) { + for (PsiParameter parameter : params) { + if (Comparing.strEqual(parameter.getName(), newName)) { + return false; + } + } + return true; + } + private static boolean doCreate(Project project, Editor editor, PsiParameter[] parameters, SmartPsiElementPointer constructorPointer, - ChangeMethodSignatureFromUsageFix addParamFix, PsiField[] fields) { + ParameterInfoImpl[] parameterInfos, PsiField[] fields) { PsiMethod constructor = (PsiMethod)constructorPointer.getElement(); assert constructor != null; PsiParameter[] newParameters = constructor.getParameterList().getParameters(); @@ -265,7 +321,7 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { field.getType()).names[0]; PsiParameter parameter = findParamByName(defaultParamName, newParameters); if (parameter == null) { - parameter = fields.length == 1 ? findParamByName(addParamFix.getNewParameterNameByOldIndex(-1), newParameters) : null; + parameter = fields.length == 1 ? findParamByName(ChangeMethodSignatureFromUsageFix.getNewParameterNameByOldIndex(-1, parameterInfos), newParameters) : null; if (parameter == null) { continue; } @@ -306,4 +362,39 @@ public class CreateConstructorParameterFromFieldFix implements IntentionAction { public boolean startInWriteAction() { return false; } + + private static class FieldParameterComparator implements Comparator { + private final PsiParameterList myParameterList; + + public FieldParameterComparator(PsiParameterList parameterList) { + myParameterList = parameterList; + } + + @Override + public int compare(PsiVariable o1, PsiVariable o2) { + + if (o1 instanceof PsiParameter && ((PsiParameter)o1).isVarArgs()) return 1; + if (o2 instanceof PsiParameter && ((PsiParameter)o2).isVarArgs()) return -1; + + if (o1 instanceof PsiField && o2 instanceof PsiField) { + return o1.getTextOffset() - o2.getTextOffset(); + } + if (o1 instanceof PsiParameter && o2 instanceof PsiParameter) { + return myParameterList.getParameterIndex((PsiParameter)o1) - myParameterList.getParameterIndex((PsiParameter)o2); + } + + if (o1 instanceof PsiField && o2 instanceof PsiParameter) { + final PsiField field = CreateFieldFromParameterAction.getParameterAssignedToField((PsiParameter)o2); + if (field == null) return 1; + return o1.getTextOffset() - field.getTextOffset(); + } + if (o1 instanceof PsiParameter && o2 instanceof PsiField) { + final PsiField field = CreateFieldFromParameterAction.getParameterAssignedToField((PsiParameter)o1); + if (field == null) return -1; + return field.getTextOffset() - o2.getTextOffset(); + } + + return 0; + } + } } diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java index d38aa38b6d86..7ae40fc1ad26 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/AssignFieldFromParameterAction.java @@ -58,7 +58,7 @@ public class AssignFieldFromParameterAction extends BaseIntentionAction { || !type.isValid() || targetClass == null || targetClass.isInterface() - || CreateFieldFromParameterAction.isParameterAssignedToField(myParameter)) { + || CreateFieldFromParameterAction.getParameterAssignedToField(myParameter) != null) { return false; } PsiField field = findFieldToAssign(myParameter); diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/CreateFieldFromParameterAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/CreateFieldFromParameterAction.java index 01741ad97d18..566e799dc8ae 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/CreateFieldFromParameterAction.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/CreateFieldFromParameterAction.java @@ -102,13 +102,14 @@ public class CreateFieldFromParameterAction implements IntentionAction { && myParameter.getManager().isInProject(myParameter) && types != null && types[0].isValid() - && !isParameterAssignedToField(myParameter) + && getParameterAssignedToField(myParameter) == null && targetClass != null && !targetClass.isInterface() ; } - static boolean isParameterAssignedToField(final PsiParameter parameter) { + @Nullable + public static PsiField getParameterAssignedToField(final PsiParameter parameter) { for (PsiReference reference : ReferencesSearch.search(parameter, new LocalSearchScope(parameter.getDeclarationScope()), false)) { if (!(reference instanceof PsiReferenceExpression)) continue; final PsiReferenceExpression expression = (PsiReferenceExpression)reference; @@ -118,10 +119,9 @@ public class CreateFieldFromParameterAction implements IntentionAction { final PsiExpression lExpression = assignmentExpression.getLExpression(); if (!(lExpression instanceof PsiReferenceExpression)) continue; final PsiElement element = ((PsiReferenceExpression)lExpression).resolve(); - if (!(element instanceof PsiField)) continue; - return true; + if (element instanceof PsiField) return (PsiField)element; } - return false; + return null; } @Nullable diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainedCalls.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainedCalls.java index 830383e3a2ee..582c4ce5907e 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainedCalls.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/createConstructorParameterFromField/afterChainedCalls.java @@ -4,11 +4,11 @@ class A { private int j; A(int field) { - this(0, field); + this(field, 0); } - A(int j, int field) { - this.j = j; + A(int field, int j) { this.field = field; + this.j = j; } } \ No newline at end of file