From 1347a2332e1e2dbddd3fa95544ab0fe3a1ed5d8f Mon Sep 17 00:00:00 2001 From: anna Date: Tue, 3 Nov 2009 15:55:08 +0300 Subject: [PATCH] introduce parameter object: add setters/getters, fix visibility for existing class --- .../impl/quickfix/CreateFromUsageBaseFix.java | 4 +- .../generation/GenerateMembersUtil.java | 3 +- .../changeSignature/ChangeSignatureProcessor.java | 4 +- .../ConvertToInstanceMethodProcessor.java | 4 +- .../extractclass/ExtractClassProcessor.java | 18 +- .../InlineSuperClassRefactoringProcessor.java | 5 +- .../IntroduceParameterObjectDialog.java | 50 +++- .../IntroduceParameterObjectForm.form | 34 ++- .../IntroduceParameterObjectProcessor.java | 295 ++++++++++----------- .../usageInfo/AppendAccessorsUsageInfo.java | 109 ++++++++ .../usageInfo/MergeMethodArguments.java | 3 +- .../move/moveMembers/MoveJavaMemberHandler.java | 3 +- .../move/moveMembers/MoveMembersProcessor.java | 15 +- .../util/FixableUsagesRefactoringProcessor.java | 12 +- .../refactoring/util/RefactoringConflictsUtil.java | 8 +- .../existingBeanIfNoGeneration/after/Param.java | 11 + .../existingBeanIfNoGeneration/after/Test.java | 10 + .../existingBeanIfNoGeneration/before/Param.java | 11 + .../existingBeanIfNoGeneration/before/Test.java | 11 + .../existingBeanVisibility/after/p/Param.java | 12 + .../existingBeanVisibility/after/p2/Test.java | 14 + .../existingBeanVisibility/before/p/Param.java | 12 + .../existingBeanVisibility/before/p2/Test.java | 11 + .../after/Param.java | 15 ++ .../after/Test.java | 11 + .../before/Param.java | 7 + .../before/Test.java | 11 + .../paramNameConflict/after/Param.java | 11 + .../paramNameConflict/after/Test.java | 11 + .../paramNameConflict/before/Param.java | 11 + .../paramNameConflict/before/Test.java | 11 + .../refactoring/IntroduceParameterObjectTest.java | 50 +++- .../src/com/intellij/util/VisibilityUtil.java | 22 ++ .../refactoring/move/MoveGroovyMemberHandler.java | 5 +- 34 files changed, 580 insertions(+), 244 deletions(-) create mode 100644 java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/AppendAccessorsUsageInfo.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p2/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p2/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Test.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Param.java create mode 100644 java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Test.java diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateFromUsageBaseFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateFromUsageBaseFix.java index 6301f19fc5..e02d3ccda4 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateFromUsageBaseFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/CreateFromUsageBaseFix.java @@ -35,8 +35,8 @@ import com.intellij.openapi.ui.popup.PopupChooserBuilder; import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.refactoring.util.RefactoringConflictsUtil; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.VisibilityUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -156,7 +156,7 @@ public abstract class CreateFromUsageBaseFix extends BaseIntentionAction { list.deleteChildRange(list.getFirstChild(), list.getLastChild()); return; } - RefactoringConflictsUtil.setVisibility(list, getVisibility(parentClass, targetClass)); + VisibilityUtil.setVisibility(list, getVisibility(parentClass, targetClass)); } protected String getVisibility(PsiClass parentClass, PsiClass targetClass) { diff --git a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java index 5e80c2f3ef..7d1a0dcf59 100644 --- a/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/generation/GenerateMembersUtil.java @@ -29,7 +29,6 @@ import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.refactoring.util.RefactoringConflictsUtil; import com.intellij.util.VisibilityUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.HashMap; @@ -212,7 +211,7 @@ public class GenerateMembersUtil { newMethod = factory.createMethod(method.getName(), substituteType(substitutor, returnType)); } - RefactoringConflictsUtil.setVisibility(newMethod.getModifierList(), VisibilityUtil.getVisibilityModifier(method.getModifierList())); + VisibilityUtil.setVisibility(newMethod.getModifierList(), VisibilityUtil.getVisibilityModifier(method.getModifierList())); PsiElement navigationElement = method.getNavigationElement(); PsiDocComment docComment = ((PsiDocCommentOwner)navigationElement).getDocComment(); diff --git a/java/java-impl/src/com/intellij/refactoring/changeSignature/ChangeSignatureProcessor.java b/java/java-impl/src/com/intellij/refactoring/changeSignature/ChangeSignatureProcessor.java index 1beae85a19..55fd1868d5 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeSignature/ChangeSignatureProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/changeSignature/ChangeSignatureProcessor.java @@ -391,7 +391,7 @@ public class ChangeSignatureProcessor extends BaseRefactoringProcessor { private void addInaccessibilityDescriptions(Set usages, MultiMap conflictDescriptions) throws IncorrectOperationException { PsiMethod method = myChangeInfo.getMethod(); PsiModifierList modifierList = (PsiModifierList)method.getModifierList().copy(); - RefactoringConflictsUtil.setVisibility(modifierList, myNewVisibility); + VisibilityUtil.setVisibility(modifierList, myNewVisibility); for (Iterator iterator = usages.iterator(); iterator.hasNext();) { UsageInfo usageInfo = iterator.next(); @@ -1035,7 +1035,7 @@ public class ChangeSignatureProcessor extends BaseRefactoringProcessor { final String highestVisibility = isOriginal ? myNewVisibility : VisibilityUtil.getHighestVisibility(myNewVisibility, VisibilityUtil.getVisibilityModifier(modifierList)); - RefactoringConflictsUtil.setVisibility(modifierList, highestVisibility); + VisibilityUtil.setVisibility(modifierList, highestVisibility); } if (myChangeInfo.isNameChanged) { diff --git a/java/java-impl/src/com/intellij/refactoring/convertToInstanceMethod/ConvertToInstanceMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/convertToInstanceMethod/ConvertToInstanceMethodProcessor.java index a5fd0a2bba..c192501dce 100644 --- a/java/java-impl/src/com/intellij/refactoring/convertToInstanceMethod/ConvertToInstanceMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/convertToInstanceMethod/ConvertToInstanceMethodProcessor.java @@ -179,10 +179,10 @@ public class ConvertToInstanceMethodProcessor extends BaseRefactoringProcessor { final PsiModifierList copy = (PsiModifierList)myMethod.getModifierList().copy(); if (myNewVisibility != null) { if (myNewVisibility.equals(VisibilityUtil.ESCALATE_VISIBILITY)) { - RefactoringConflictsUtil.setVisibility(copy, PsiModifier.PUBLIC); + VisibilityUtil.setVisibility(copy, PsiModifier.PUBLIC); } else { - RefactoringConflictsUtil.setVisibility(copy, myNewVisibility); + VisibilityUtil.setVisibility(copy, myNewVisibility); } } diff --git a/java/java-impl/src/com/intellij/refactoring/extractclass/ExtractClassProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractclass/ExtractClassProcessor.java index 58a07e6e45..2eacd3a97b 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractclass/ExtractClassProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractclass/ExtractClassProcessor.java @@ -42,7 +42,6 @@ import com.intellij.refactoring.psi.MethodInheritanceUtils; import com.intellij.refactoring.psi.TypeParametersVisitor; import com.intellij.refactoring.util.FixableUsageInfo; import com.intellij.refactoring.util.FixableUsagesRefactoringProcessor; -import com.intellij.refactoring.util.RefactoringConflictsUtil; import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.usageView.UsageInfo; import com.intellij.usageView.UsageViewDescriptor; @@ -231,31 +230,18 @@ public class ExtractClassProcessor extends FixableUsagesRefactoringProcessor { for (PsiMethod method : methods) { final PsiMethod member = psiClass.findMethodBySignature(method, false); if (member != null) { - fixVisibility(usageInfos, member); + VisibilityUtil.fixVisibility(usageInfos, member, myNewVisibility); } } for (PsiField field : fields) { final PsiField member = psiClass.findFieldByName(field.getName(), false); if (member != null) { - fixVisibility(usageInfos, member); + VisibilityUtil.fixVisibility(usageInfos, member, myNewVisibility); } } } - private void fixVisibility(UsageInfo[] usageInfos, PsiMember member) { - if (VisibilityUtil.ESCALATE_VISIBILITY.equals(myNewVisibility)) { - for (UsageInfo info : usageInfos) { - final PsiElement element = info.getElement(); - if (element != null) { - VisibilityUtil.escalateVisibility(member, element); - } - } - } else { - RefactoringConflictsUtil.setVisibility(member.getModifierList(), myNewVisibility); - } - } - private void buildDelegate() { final PsiManager manager = sourceClass.getManager(); diff --git a/java/java-impl/src/com/intellij/refactoring/inlineSuperClass/InlineSuperClassRefactoringProcessor.java b/java/java-impl/src/com/intellij/refactoring/inlineSuperClass/InlineSuperClassRefactoringProcessor.java index 4a97b3f64c..7c34cf7e99 100644 --- a/java/java-impl/src/com/intellij/refactoring/inlineSuperClass/InlineSuperClassRefactoringProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/inlineSuperClass/InlineSuperClassRefactoringProcessor.java @@ -174,8 +174,9 @@ public class InlineSuperClassRefactoringProcessor extends FixableUsagesRefactori } //todo check accessibility conflicts } - for (PsiElement element : pushDownConflicts.getConflicts().keySet()) { - conflicts.put(element, pushDownConflicts.getConflicts().get(element)); + final MultiMap conflictsMap = pushDownConflicts.getConflicts(); + for (PsiElement element : conflictsMap.keySet()) { + conflicts.put(element, conflictsMap.get(element)); } checkConflicts(refUsages, conflicts); return showConflicts(conflicts); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectDialog.java b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectDialog.java index ea79fec622..b62cc0cf22 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectDialog.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectDialog.java @@ -20,7 +20,9 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.help.HelpManager; import com.intellij.openapi.options.ConfigurationException; import com.intellij.openapi.project.Project; +import com.intellij.openapi.roots.ProjectRootManager; import com.intellij.openapi.util.text.StringUtil; +import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.wm.IdeFocusManager; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; @@ -30,8 +32,10 @@ import com.intellij.refactoring.RefactorJBundle; import com.intellij.refactoring.RefactoringBundle; import com.intellij.refactoring.ui.PackageNameReferenceEditorCombo; import com.intellij.refactoring.ui.RefactoringDialog; +import com.intellij.refactoring.ui.VisibilityPanel; import com.intellij.refactoring.util.ParameterTablePanel; import com.intellij.ui.DocumentAdapter; +import com.intellij.ui.RecentsManager; import com.intellij.ui.ReferenceEditorComboWithBrowseButton; import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; @@ -69,7 +73,11 @@ public class IntroduceParameterObjectDialog extends RefactoringDialog { private JCheckBox keepMethodAsDelegate; private ReferenceEditorComboWithBrowseButton packageTextField; private ReferenceEditorComboWithBrowseButton existingClassField; + private JPanel myVisibilityComponent; + private JCheckBox myGenerateAccessorsCheckBox; + private VisibilityPanel myVisibilityPanel; private static final String RECENTS_KEY = "IntroduceParameterObject.RECENTS_KEY"; + private static final String EXISTING_KEY = "IntroduceParameterObject.EXISTING_KEY"; public IntroduceParameterObjectDialog(PsiMethod sourceMethod) { super(sourceMethod.getProject(), true); @@ -119,9 +127,14 @@ public class IntroduceParameterObjectDialog extends RefactoringDialog { createNewClassButton.addActionListener(listener); myCreateInnerClassRadioButton.addActionListener(listener); toggleRadioEnablement(); + + myVisibilityPanel = new VisibilityPanel(true, true); + myVisibilityPanel.setVisibility(null); + myVisibilityComponent.add(myVisibilityPanel, BorderLayout.WEST); } private void toggleRadioEnablement() { + enableGenerateAccessors(); UIUtil.setEnabled(myUseExistingPanel, useExistingClassButton.isSelected(), true); UIUtil.setEnabled(myCreateNewClassPanel, createNewClassButton.isSelected(), true); UIUtil.setEnabled(myInnerClassPanel, myCreateInnerClassRadioButton.isSelected(), true); @@ -137,22 +150,18 @@ public class IntroduceParameterObjectDialog extends RefactoringDialog { final boolean keepMethod = keepMethodAsDelegate(); final String className; final String packageName; - final List getterNames; final boolean createInnerClass = myCreateInnerClassRadioButton.isSelected(); if (createInnerClass) { className = getInnerClassName(); packageName = ""; - getterNames = null; } else if (useExistingClass) { final String existingClassName = getExistingClassName(); - getterNames = new ArrayList(); className = StringUtil.getShortName(existingClassName); packageName = StringUtil.getPackageName(existingClassName); } else { packageName = getPackageName(); className = getClassName(); - getterNames = null; } List parameters = new ArrayList(); for (ParameterTablePanel.VariableData data : parameterInfo) { @@ -161,8 +170,9 @@ public class IntroduceParameterObjectDialog extends RefactoringDialog { } } invokeRefactoring(new IntroduceParameterObjectProcessor(className, packageName, sourceMethod, - parameters.toArray(new ParameterTablePanel.VariableData[parameters.size()]), getterNames, keepMethod, useExistingClass, - createInnerClass)); + parameters.toArray(new ParameterTablePanel.VariableData[parameters.size()]), + keepMethod, useExistingClass, + createInnerClass, myVisibilityPanel.getVisibility(), myGenerateAccessorsCheckBox.isSelected())); } @Override @@ -291,11 +301,35 @@ public class IntroduceParameterObjectDialog extends RefactoringDialog { if (selectedClass != null) { final String className = selectedClass.getQualifiedName(); existingClassField.setText(className); + RecentsManager.getInstance(myProject).registerRecentEntry(EXISTING_KEY, className); } } - }, "", PsiManager.getInstance(myProject), true, RECENTS_KEY); + }, "", PsiManager.getInstance(myProject), true, EXISTING_KEY); - existingClassField.getChildComponent().getDocument().addDocumentListener(adapter); + existingClassField.getChildComponent().getDocument().addDocumentListener(new com.intellij.openapi.editor.event.DocumentAdapter() { + @Override + public void documentChanged(com.intellij.openapi.editor.event.DocumentEvent e) { + validateButtons(); + enableGenerateAccessors(); + } + }); + } + private void enableGenerateAccessors() { + boolean existingNotALibraryClass = false; + if (useExistingClassButton.isSelected()) { + final PsiClass selectedClass = + JavaPsiFacade.getInstance(myProject).findClass(existingClassField.getText(), GlobalSearchScope.projectScope(myProject)); + if (selectedClass != null) { + final PsiFile containingFile = selectedClass.getContainingFile(); + if (containingFile != null) { + final VirtualFile virtualFile = containingFile.getVirtualFile(); + if (virtualFile != null) { + existingNotALibraryClass = ProjectRootManager.getInstance(myProject).getFileIndex().isInSourceContent(virtualFile); + } + } + } + } + myGenerateAccessorsCheckBox.setEnabled(existingNotALibraryClass); } } diff --git a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectForm.form b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectForm.form index 38799e9ccb..8456c35d4d 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectForm.form +++ b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectForm.form @@ -24,7 +24,7 @@ - + @@ -33,7 +33,7 @@ - + @@ -168,24 +168,42 @@ + + + + + + + + + - - + + - + - - + + - + + + + + + + + + + diff --git a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectProcessor.java b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectProcessor.java index cf2cc5bcf8..b96d56025d 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/IntroduceParameterObjectProcessor.java @@ -16,28 +16,22 @@ package com.intellij.refactoring.introduceparameterobject; import com.intellij.ide.util.PackageUtil; -import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.module.Module; import com.intellij.openapi.module.ModuleUtil; -import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; -import com.intellij.psi.codeStyle.CodeStyleSettings; -import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.searches.OverridingMethodsSearch; import com.intellij.psi.util.PropertyUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.refactoring.RefactorJBundle; -import com.intellij.refactoring.introduceparameterobject.usageInfo.MergeMethodArguments; -import com.intellij.refactoring.introduceparameterobject.usageInfo.ReplaceParameterAssignmentWithCall; -import com.intellij.refactoring.introduceparameterobject.usageInfo.ReplaceParameterIncrementDecrement; -import com.intellij.refactoring.introduceparameterobject.usageInfo.ReplaceParameterReferenceWithCall; +import com.intellij.refactoring.introduceparameterobject.usageInfo.*; import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.refactoring.psi.TypeParametersVisitor; import com.intellij.refactoring.util.FixableUsageInfo; @@ -47,11 +41,16 @@ import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.usageView.UsageInfo; import com.intellij.usageView.UsageViewDescriptor; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.VisibilityUtil; import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; -import java.util.*; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringProcessor { private static final Logger logger = Logger.getInstance("com.siyeh.rpp.introduceparameterobject.IntroduceParameterObjectProcessor"); @@ -59,32 +58,39 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP private final PsiMethod method; private final String className; private final String packageName; - private final List getterNames; private final boolean keepMethodAsDelegate; private final boolean myUseExistingClass; private final boolean myCreateInnerClass; - private final List parameters; + private final String myNewVisibility; + private final boolean myGenerateAccessors; + private final List parameters; private final int[] paramsToMerge; private final List typeParams; private final Set paramsNeedingSetters = new HashSet(); + private final Set paramsNeedingGetters = new HashSet(); private final PsiClass existingClass; - private boolean myExistingClassCompatible = true; + private PsiMethod myExistingClassCompatibleConstructor; public IntroduceParameterObjectProcessor(String className, String packageName, PsiMethod method, - ParameterTablePanel.VariableData[] parameters, - List getterNames, - boolean keepMethodAsDelegate, final boolean useExistingClass, final boolean createInnerClass) { + ParameterTablePanel.VariableData[] parameters, boolean keepMethodAsDelegate, final boolean useExistingClass, + final boolean createInnerClass, + String newVisibility, + boolean generateAccessors) { super(method.getProject()); this.method = method; this.className = className; this.packageName = packageName; - this.getterNames = getterNames; this.keepMethodAsDelegate = keepMethodAsDelegate; myUseExistingClass = useExistingClass; myCreateInnerClass = createInnerClass; - this.parameters = new ArrayList(Arrays.asList(parameters)); + myNewVisibility = newVisibility; + myGenerateAccessors = generateAccessors; + this.parameters = new ArrayList(); + for (ParameterTablePanel.VariableData parameter : parameters) { + this.parameters.add(new ParameterChunk(parameter)); + } final PsiParameterList parameterList = method.getParameterList(); final PsiParameter[] methodParams = parameterList.getParameters(); paramsToMerge = new int[parameters.length]; @@ -123,14 +129,8 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP if (existingClass == null) { conflicts.putValue(null, RefactorJBundle.message("cannot.perform.the.refactoring") + "Could not find the selected class"); } - final String incompatibilityMessage = "Selected class is not compatible with chosen parameters"; - if (!myExistingClassCompatible) { - conflicts - .putValue(existingClass, RefactorJBundle.message("cannot.perform.the.refactoring") + incompatibilityMessage); - - } - if (!paramsNeedingSetters.isEmpty()) { - conflicts.putValue(existingClass, RefactorJBundle.message("cannot.perform.the.refactoring") + incompatibilityMessage); + if (myExistingClassCompatibleConstructor == null) { + conflicts.putValue(existingClass, RefactorJBundle.message("cannot.perform.the.refactoring") + "Selected class has no compatible constructors"); } } else if (existingClass != null) { @@ -138,16 +138,27 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP RefactorJBundle.message("cannot.perform.the.refactoring") + RefactorJBundle.message("there.already.exists.a.class.with.the.chosen.name")); } + for (UsageInfo usageInfo : refUsages.get()) { + if (usageInfo instanceof FixableUsageInfo) { + final String conflictMessage = ((FixableUsageInfo)usageInfo).getConflictMessage(); + if (conflictMessage != null) { + conflicts.putValue(usageInfo.getElement(), conflictMessage); + } + } + } return showConflicts(conflicts); } public void findUsages(@NotNull List usages) { if (myUseExistingClass && existingClass != null) { - myExistingClassCompatible = existingClassIsCompatible(existingClass, parameters, getterNames); - if (!myExistingClassCompatible) return; + myExistingClassCompatibleConstructor = existingClassIsCompatible(existingClass, parameters); } findUsagesForMethod(method, usages); + if (myUseExistingClass && existingClass != null && !(paramsNeedingGetters.isEmpty() && paramsNeedingSetters.isEmpty())) { + usages.add(new AppendAccessorsUsageInfo(existingClass, myGenerateAccessors, paramsNeedingGetters, paramsNeedingSetters, parameters)); + } + final PsiMethod[] overridingMethods = OverridingMethodsSearch.search(method, method.getUseScope(), true).toArray(PsiMethod.EMPTY_ARRAY); for (PsiMethod siblingMethod : overridingMethods) { findUsagesForMethod(siblingMethod, usages); @@ -155,7 +166,12 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP } private void findUsagesForMethod(PsiMethod overridingMethod, List usages) { - final String fixedParamName = calculateNewParamNameForMethod(overridingMethod); + final PsiCodeBlock body = overridingMethod.getBody(); + final String baseParameterName = StringUtil.decapitalize(className); + final String fixedParamName = + body != null + ? JavaCodeStyleManager.getInstance(myProject).suggestUniqueVariableName(baseParameterName, body.getLBrace(), true) + : JavaCodeStyleManager.getInstance(myProject).propertyNameToVariableName(baseParameterName, VariableKind.PARAMETER); usages.add(new MergeMethodArguments(overridingMethod, className, packageName, fixedParamName, paramsToMerge, typeParams, keepMethodAsDelegate, myCreateInnerClass ? method.getContainingClass() : null)); @@ -168,27 +184,28 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP final PsiMethod containingMethod = (PsiMethod)parameter.getDeclarationScope(); final int index = containingMethod.getParameterList().getParameterIndex(parameter); final PsiParameter replacedParameter = method.getParameterList().getParameters()[index]; - @NonNls String getter = null; - if (getterNames != null) { - for (int i = 0; i < parameters.size(); i++) { - ParameterTablePanel.VariableData data = parameters.get(i); - if (data.variable.equals(parameter)) { - getter = getterNames.get(i); - break; - } - } - } + final ParameterChunk parameterChunk = ParameterChunk.getChunkByParameter(parameter, parameters); + + @NonNls String getter = parameterChunk != null ? parameterChunk.getter : null; if (getter == null) { getter = PropertyUtil.suggestGetterName(replacedParameter.getName(), replacedParameter.getType()); + paramsNeedingGetters.add(replacedParameter); + } + @NonNls String setter = parameterChunk != null ? parameterChunk.setter : null; + if (setter == null) { + setter = PropertyUtil.suggestSetterName(replacedParameter.getName()); } - @NonNls final String setter = PropertyUtil.suggestSetterName(replacedParameter.getName()); if (RefactoringUtil.isPlusPlusOrMinusMinus(paramUsage.getParent())) { usages.add(new ReplaceParameterIncrementDecrement(paramUsage, fixedParamName, setter, getter)); - paramsNeedingSetters.add(replacedParameter); + if (parameterChunk == null || parameterChunk.setter == null) { + paramsNeedingSetters.add(replacedParameter); + } } else if (RefactoringUtil.isAssignmentLHS(paramUsage)) { usages.add(new ReplaceParameterAssignmentWithCall(paramUsage, fixedParamName, setter, getter)); - paramsNeedingSetters.add(replacedParameter); + if (parameterChunk == null || parameterChunk.setter == null) { + paramsNeedingSetters.add(replacedParameter); + } } else { usages.add(new ReplaceParameterReferenceWithCall(paramUsage, fixedParamName, getter)); @@ -196,69 +213,28 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP } } - private String calculateNewParamNameForMethod(PsiMethod testMethod) { - final Project project = testMethod.getProject(); - final CodeStyleSettingsManager settingsManager = CodeStyleSettingsManager.getInstance(project); - final CodeStyleSettings settings = settingsManager.getCurrentSettings(); - final String baseParamName = settings.PARAMETER_NAME_PREFIX.length() == 0 ? StringUtil.decapitalize(className) : className; - final String newParamName = settings.PARAMETER_NAME_PREFIX + baseParamName + settings.PARAMETER_NAME_SUFFIX; - if (!isParamNameUsed(newParamName, testMethod)) { - return newParamName; - } - int count = 1; - while (true) { - final String testParamName = settings.PARAMETER_NAME_PREFIX + baseParamName + count + settings.PARAMETER_NAME_SUFFIX; - if (!isParamNameUsed(testParamName, testMethod)) { - return testParamName; - } - count++; - } - } - - private boolean isParamNameUsed(String paramName, PsiMethod testMethod) { - final PsiParameterList testParamList = testMethod.getParameterList(); - final PsiParameter[] testParameters = testParamList.getParameters(); - for (int i = 0; i < testParameters.length; i++) { - if (!isParamToMerge(i)) { - if (testParameters[i].getName().equals(paramName)) { - return true; - } - } - } - final PsiCodeBlock body = testMethod.getBody(); - if (body == null) { - return false; - } - final NameUsageVisitor visitor = new NameUsageVisitor(paramName); - body.accept(visitor); - return visitor.isNameUsed(); - } - - private boolean isParamToMerge(int i) { - for (int j : paramsToMerge) { - if (i == j) { - return true; - } - } - return false; - } - protected void performRefactoring(UsageInfo[] usageInfos) { - if (buildClass()) { + final PsiClass psiClass = buildClass(); + if (psiClass != null) { super.performRefactoring(usageInfos); + VisibilityUtil.fixVisibility(usageInfos, psiClass, myNewVisibility); + if (myExistingClassCompatibleConstructor != null) { + VisibilityUtil.fixVisibility(usageInfos, myExistingClassCompatibleConstructor, myNewVisibility); + } } } - private boolean buildClass() { + private PsiClass buildClass() { if (existingClass != null) { - return true; + return existingClass; } final ParameterObjectBuilder beanClassBuilder = new ParameterObjectBuilder(); beanClassBuilder.setProject(myProject); beanClassBuilder.setTypeArguments(typeParams); beanClassBuilder.setClassName(className); beanClassBuilder.setPackageName(packageName); - for (ParameterTablePanel.VariableData parameter : parameters) { + for (ParameterChunk parameterChunk : parameters) { + final ParameterTablePanel.VariableData parameter = parameterChunk.parameter; final boolean setterRequired = paramsNeedingSetters.contains(parameter.variable); beanClassBuilder.addField((PsiParameter)parameter.variable, parameter.name, parameter.type, setterRequired); } @@ -272,7 +248,7 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP assert classes.length > 0 : classString; final PsiClass innerClass = (PsiClass)containingClass.add(classes[0]); PsiUtil.setModifierProperty(innerClass, PsiModifier.STATIC, true); - JavaCodeStyleManager.getInstance(newFile.getProject()).shortenClassReferences(innerClass); + return (PsiClass)JavaCodeStyleManager.getInstance(newFile.getProject()).shortenClassReferences(innerClass); } else { final PsiFile containingFile = method.getContainingFile(); final PsiDirectory containingDirectory = containingFile.getContainingDirectory(); @@ -284,17 +260,14 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP final CodeStyleManager codeStyleManager = method.getManager().getCodeStyleManager(); final PsiElement shortenedFile = JavaCodeStyleManager.getInstance(newFile.getProject()).shortenClassReferences(newFile); final PsiElement reformattedFile = codeStyleManager.reformat(shortenedFile); - directory.add(reformattedFile); - } else { - return false; + return ((PsiJavaFile)directory.add(reformattedFile)).getClasses()[0]; } } } catch (IncorrectOperationException e) { logger.info(e); - return false; } - return true; + return null; } protected String getCommandName() { @@ -303,7 +276,7 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP } - private static class ParamUsageVisitor extends JavaRecursiveElementWalkingVisitor { + private static class ParamUsageVisitor extends JavaRecursiveElementVisitor { private final Set paramsToMerge = new HashSet(); private final Set parameterUsages = new HashSet(4); @@ -333,52 +306,17 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP } } - private static class NameUsageVisitor extends JavaRecursiveElementWalkingVisitor { - private boolean nameUsed = false; - private final String paramName; - - NameUsageVisitor(String paramName) { - super(); - this.paramName = paramName; - } - - public void visitElement(PsiElement element) { - if (nameUsed) { - return; - } - super.visitElement(element); - } - - public void visitVariable(PsiVariable variable) { - super.visitVariable(variable); - final String variableName = variable.getName(); - if (paramName.equals(variableName)) { - nameUsed = true; - } - } - - public void visitReferenceExpression(PsiReferenceExpression expression) { - super.visitReferenceExpression(expression); - if (expression.getQualifier() == null) { - return; - } - final String referenceName = expression.getReferenceName(); - if (paramName.equals(referenceName)) { - nameUsed = true; - } - } - - public boolean isNameUsed() { - return nameUsed; - } - } - - private static boolean existingClassIsCompatible(PsiClass aClass, List params, @NonNls List getterNames) { + @Nullable + private static PsiMethod existingClassIsCompatible(PsiClass aClass, List params) { if (params.size() == 1) { - final PsiType paramType = params.get(0).type; + final ParameterChunk parameterChunk = params.get(0); + final PsiType paramType = parameterChunk.parameter.type; if (TypeConversionUtil.isPrimitiveWrapper(aClass.getQualifiedName())) { - getterNames.add(paramType.getCanonicalText() + "Value"); - return true; + parameterChunk.setField(aClass.findFieldByName("value", false)); + parameterChunk.setGetter(paramType.getCanonicalText() + "Value"); + for (PsiMethod constructor : aClass.getConstructors()) { + if (constructorIsCompatible(constructor, params)) return constructor; + } } } final PsiMethod[] constructors = aClass.getConstructors(); @@ -390,42 +328,85 @@ public class IntroduceParameterObjectProcessor extends FixableUsagesRefactoringP } } if (compatibleConstructor == null) { - return false; + return null; } final PsiParameterList parameterList = compatibleConstructor.getParameterList(); final PsiParameter[] constructorParams = parameterList.getParameters(); - for (PsiParameter param : constructorParams) { + for (int i = 0; i < constructorParams.length; i++) { + final PsiParameter param = constructorParams[i]; + final ParameterChunk parameterChunk = params.get(i); + final PsiField field = findFieldAssigned(param, compatibleConstructor); if (field == null) { - return false; + return null; } - final PsiMethod getter = PropertyUtils.findGetterForField(field); - if (getter == null) { - return false; + + parameterChunk.setField(field); + + final PsiMethod getterForField = PropertyUtils.findGetterForField(field); + if (getterForField != null) { + parameterChunk.setGetter(getterForField.getName()); + } + + final PsiMethod setterForField = PropertyUtils.findSetterForField(field); + if (setterForField != null) { + parameterChunk.setSetter(setterForField.getName()); } - getterNames.add(getter.getName()); } - //TODO: this fails if there are any setters required - return true; + return compatibleConstructor; } - private static boolean constructorIsCompatible(PsiMethod constructor, List params) { - if (!constructor.hasModifierProperty(PsiModifier.PUBLIC)) { - return false; - } + private static boolean constructorIsCompatible(PsiMethod constructor, List params) { final PsiParameterList parameterList = constructor.getParameterList(); final PsiParameter[] constructorParams = parameterList.getParameters(); if (constructorParams.length != params.size()) { return false; } for (int i = 0; i < constructorParams.length; i++) { - if (!TypeConversionUtil.isAssignable(constructorParams[i].getType(), params.get(i).type)) { + if (!TypeConversionUtil.isAssignable(constructorParams[i].getType(), params.get(i).parameter.type)) { return false; } } return true; } + public static class ParameterChunk { + private ParameterTablePanel.VariableData parameter; + private PsiField field; + private String getter; + private String setter; + + public ParameterChunk(ParameterTablePanel.VariableData parameter) { + this.parameter = parameter; + } + + public void setField(PsiField field) { + this.field = field; + } + + public void setGetter(String getter) { + this.getter = getter; + } + + public void setSetter(String setter) { + this.setter = setter; + } + + public PsiField getField() { + return field; + } + + @Nullable + public static ParameterChunk getChunkByParameter(PsiParameter param, List params) { + for (ParameterChunk chunk : params) { + if (chunk.parameter.variable.equals(param)) { + return chunk; + } + } + return null; + } + } + private static PsiField findFieldAssigned(PsiParameter param, PsiMethod constructor) { final ParamAssignmentFinder visitor = new ParamAssignmentFinder(param); constructor.accept(visitor); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/AppendAccessorsUsageInfo.java b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/AppendAccessorsUsageInfo.java new file mode 100644 index 0000000000..fdba8c82c6 --- /dev/null +++ b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/AppendAccessorsUsageInfo.java @@ -0,0 +1,109 @@ +/* + * Copyright 2000-2009 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * User: anna + * Date: 02-Nov-2009 + */ +package com.intellij.refactoring.introduceparameterobject.usageInfo; + +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiField; +import com.intellij.psi.PsiParameter; +import com.intellij.psi.util.PropertyUtil; +import com.intellij.refactoring.RefactorJBundle; +import com.intellij.refactoring.introduceparameterobject.IntroduceParameterObjectProcessor; +import com.intellij.refactoring.util.FixableUsageInfo; +import com.intellij.util.Function; +import com.intellij.util.IncorrectOperationException; + +import java.util.List; +import java.util.Set; + +public class AppendAccessorsUsageInfo extends FixableUsageInfo{ + private final boolean myGenerateAccessors; + private final Set paramsNeedingSetters; + private final Set paramsNeedingGetters; + private final List parameters; + private static final Logger LOGGER = Logger.getInstance("#" + AppendAccessorsUsageInfo.class.getName()); + + + public AppendAccessorsUsageInfo(PsiElement psiClass, boolean generateAccessors, Set paramsNeedingGetters, + Set paramsNeedingSetters, List parameters) { + super(psiClass); + myGenerateAccessors = generateAccessors; + this.paramsNeedingGetters = paramsNeedingGetters; + this.paramsNeedingSetters = paramsNeedingSetters; + this.parameters = parameters; + } + + @Override + public void fixUsage() throws IncorrectOperationException { + if (myGenerateAccessors) { + appendAccessors(paramsNeedingGetters, true); + appendAccessors(paramsNeedingSetters, false); + } + } + + private void appendAccessors(final Set params, boolean isGetter) { + final PsiElement element = getElement(); + if (element != null) { + for (PsiParameter parameter : params) { + final IntroduceParameterObjectProcessor.ParameterChunk parameterChunk = + IntroduceParameterObjectProcessor.ParameterChunk.getChunkByParameter(parameter, parameters); + LOGGER.assertTrue(parameterChunk != null); + element.add(isGetter + ? PropertyUtil.generateGetterPrototype(parameterChunk.getField()) + : PropertyUtil.generateSetterPrototype(parameterChunk.getField())); + + } + } + } + + @Override + public String getConflictMessage() { + if (!myGenerateAccessors && (!paramsNeedingSetters.isEmpty() || !paramsNeedingGetters.isEmpty())) { + final StringBuffer buf = new StringBuffer(); + appendConflicts(buf, paramsNeedingGetters); + appendConflicts(buf, paramsNeedingSetters); + return RefactorJBundle.message("cannot.perform.the.refactoring") + buf.toString(); + } + return null; + } + + private void appendConflicts(StringBuffer buf, final Set paramsNeeding) { + if (!paramsNeeding.isEmpty()) { + buf.append(paramsNeeding == paramsNeedingGetters ? "Getters" : "Setters"); + buf.append(" for the following fields are required:\n"); + buf.append(StringUtil.join(paramsNeeding, new Function() { + public String fun(PsiParameter psiParameter) { + final IntroduceParameterObjectProcessor.ParameterChunk chunk = + IntroduceParameterObjectProcessor.ParameterChunk.getChunkByParameter(psiParameter, parameters); + if (chunk != null) { + final PsiField field = chunk.getField(); + if (field != null) { + return field.getName(); + } + } + return psiParameter.getName(); + } + }, ", ")); + buf.append(".\n"); + } + } +} \ No newline at end of file diff --git a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/MergeMethodArguments.java b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/MergeMethodArguments.java index 41dd69175f..fa6313aeef 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/MergeMethodArguments.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceparameterobject/usageInfo/MergeMethodArguments.java @@ -18,6 +18,7 @@ package com.intellij.refactoring.introduceparameterobject.usageInfo; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.impl.source.PsiImmediateClassType; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.refactoring.changeSignature.ChangeSignatureProcessor; @@ -93,7 +94,7 @@ public class MergeMethodArguments extends FixableUsageInfo { parametersInfo.add(new ParameterInfoImpl(-1, parameterName, new PsiImmediateClassType(psiClass, subst), null) { @Override public PsiExpression getValue(final PsiCallExpression expr) throws IncorrectOperationException { - return psiFacade.getElementFactory().createExpressionFromText(getMergedParam(expr), expr); + return (PsiExpression)JavaCodeStyleManager.getInstance(getProject()).shortenClassReferences(psiFacade.getElementFactory().createExpressionFromText(getMergedParam(expr), expr)); } }); final PsiParameter[] parameters = method.getParameterList().getParameters(); diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java index 4bb95d0651..ab617c81fa 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java @@ -22,7 +22,6 @@ import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtilBase; import com.intellij.refactoring.util.EnumConstantsUtil; -import com.intellij.refactoring.util.RefactoringConflictsUtil; import com.intellij.refactoring.util.RefactoringHierarchyUtil; import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.IncorrectOperationException; @@ -125,7 +124,7 @@ public class MoveJavaMemberHandler implements MoveMemberHandler { assert list != null; list.setModifierProperty(PsiModifier.STATIC, member.hasModifierProperty(PsiModifier.STATIC)); list.setModifierProperty(PsiModifier.FINAL, member.hasModifierProperty(PsiModifier.FINAL)); - RefactoringConflictsUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); + VisibilityUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); } } member.delete(); diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java index f742138768..edbd90db1e 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java @@ -225,18 +225,7 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor { if(myNewVisibility == null) return; - if (VisibilityUtil.ESCALATE_VISIBILITY.equals(myNewVisibility)) { - for (UsageInfo usage : usages) { - if (usage instanceof MoveMembersUsageInfo) { - final PsiElement place = usage.getElement(); - if (place != null) { - VisibilityUtil.escalateVisibility(newMember, place); - } - } - } - } else { - RefactoringConflictsUtil.setVisibility(modifierList, myNewVisibility); - } + VisibilityUtil.fixVisibility(usages, newMember, myNewVisibility); } protected boolean preprocessUsages(Ref refUsages) { @@ -264,7 +253,7 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor { PsiModifierList copy = member.getModifierList(); if (copy!=null) copy= (PsiModifierList)copy.copy(); if (newVisibility != null) { - if (copy!=null) RefactoringConflictsUtil.setVisibility(copy, newVisibility); + if (copy!=null) VisibilityUtil.setVisibility(copy, newVisibility); } modifierListCopies.put(member, copy); } diff --git a/java/java-impl/src/com/intellij/refactoring/util/FixableUsagesRefactoringProcessor.java b/java/java-impl/src/com/intellij/refactoring/util/FixableUsagesRefactoringProcessor.java index c849cdf33d..520ec65049 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/FixableUsagesRefactoringProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/util/FixableUsagesRefactoringProcessor.java @@ -42,11 +42,13 @@ public abstract class FixableUsagesRefactoringProcessor extends BaseRefactoringP protected void performRefactoring(UsageInfo[] usageInfos) { RefactoringUtil.sortDepthFirstRightLeftOrder(usageInfos); for (UsageInfo usageInfo : usageInfos) { - try { - ((FixableUsageInfo)usageInfo).fixUsage(); - } - catch (IncorrectOperationException e) { - LOG.info(e); + if (usageInfo instanceof FixableUsageInfo) { + try { + ((FixableUsageInfo)usageInfo).fixUsage(); + } + catch (IncorrectOperationException e) { + LOG.info(e); + } } } } diff --git a/java/java-impl/src/com/intellij/refactoring/util/RefactoringConflictsUtil.java b/java/java-impl/src/com/intellij/refactoring/util/RefactoringConflictsUtil.java index 36e72d0fae..4a2bfd0e11 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/RefactoringConflictsUtil.java +++ b/java/java-impl/src/com/intellij/refactoring/util/RefactoringConflictsUtil.java @@ -46,12 +46,6 @@ import java.util.Collection; import java.util.Set; public class RefactoringConflictsUtil { - public static void setVisibility(PsiModifierList modifierList, @Modifier String newVisibility) throws IncorrectOperationException { - modifierList.setModifierProperty(PsiModifier.PRIVATE, false); - modifierList.setModifierProperty(PsiModifier.PUBLIC, false); - modifierList.setModifierProperty(PsiModifier.PROTECTED, false); - modifierList.setModifierProperty(newVisibility, true); - } public static void analyzeAccessibilityConflicts(@NotNull Set membersToMove, @NotNull final PsiClass targetClass, @@ -75,7 +69,7 @@ public class RefactoringConflictsUtil { if (newVisibility != null) { try { - if (modifierList!=null) setVisibility(modifierList, newVisibility); + if (modifierList!=null) VisibilityUtil.setVisibility(modifierList, newVisibility); } catch (IncorrectOperationException ex) { /* do nothing and hope for the best */ diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Param.java new file mode 100644 index 0000000000..0c888bfb95 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Param.java @@ -0,0 +1,11 @@ +public class Param { + private final int[] i; + + public Param(int... i) { + this.i = i; + } + + public int[] getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Test.java new file mode 100644 index 0000000000..8e0a4db702 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/after/Test.java @@ -0,0 +1,10 @@ +class Test { + void foo(Param param) { + if (param.getI().lenght == 0) { + } + } + + void bar(){ + foo(new Param(1, 2)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Param.java new file mode 100644 index 0000000000..fb6c135d55 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Param.java @@ -0,0 +1,11 @@ +public class Param { + private final int i; + + public Param(int i) { + this.i = i; + } + + public int getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Test.java new file mode 100644 index 0000000000..9cf1f8cfd3 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanIfNoGeneration/before/Test.java @@ -0,0 +1,11 @@ +class Test { + void foo(int i) { + if (i == 0) { + i++; + } + } + + void bar(){ + foo(1, 2); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p/Param.java new file mode 100644 index 0000000000..a295650b4a --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p/Param.java @@ -0,0 +1,12 @@ +package p; +public class Param { + private final int[] i; + + public Param(int... i) { + this.i = i; + } + + public int[] getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p2/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p2/Test.java new file mode 100644 index 0000000000..7b6d6a5790 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/after/p2/Test.java @@ -0,0 +1,14 @@ +package p2; + +import p.Param; + +class Test { + void foo(Param param) { + if (param.getI().lenght == 0) { + } + } + + void bar(){ + foo(new Param(1, 2)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p/Param.java new file mode 100644 index 0000000000..e9eb50cb6c --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p/Param.java @@ -0,0 +1,12 @@ +package p; +class Param { + private final int[] i; + + Param(int... i) { + this.i = i; + } + + public int[] getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p2/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p2/Test.java new file mode 100644 index 0000000000..ce60f491fc --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/existingBeanVisibility/before/p2/Test.java @@ -0,0 +1,11 @@ +package p2; +class Test { + void foo(int... i) { + if (i.lenght == 0) { + } + } + + void bar(){ + foo(1, 2); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Param.java new file mode 100644 index 0000000000..4c8d9d12f9 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Param.java @@ -0,0 +1,15 @@ +public class Param { + private final int i; + + public Param(int i) { + this.i = i; + } + + public int getI() { + return i; + } + + public void setI(int i) { + this.i = i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Test.java new file mode 100644 index 0000000000..4cf890bd40 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/after/Test.java @@ -0,0 +1,11 @@ +class Test { + void foo(Param param) { + if (param.getI() == 0) { + param.setI(param.getI() + 1); + } + } + + void bar(){ + foo(new Param(1)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Param.java new file mode 100644 index 0000000000..45194605d7 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Param.java @@ -0,0 +1,7 @@ +public class Param { + private final int i; + + public Param(int i) { + this.i = i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Test.java new file mode 100644 index 0000000000..47791aecfb --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/generateGetterSetterForExistingBean/before/Test.java @@ -0,0 +1,11 @@ +class Test { + void foo(int i) { + if (i == 0) { + i++; + } + } + + void bar(){ + foo(1); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Param.java new file mode 100644 index 0000000000..0c888bfb95 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Param.java @@ -0,0 +1,11 @@ +public class Param { + private final int[] i; + + public Param(int... i) { + this.i = i; + } + + public int[] getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Test.java new file mode 100644 index 0000000000..0f186b196f --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/after/Test.java @@ -0,0 +1,11 @@ +class Test { + void foo(Param param1) { + if (param1.getI().lenght == 0) { + } + Param param = null; + } + + void bar(){ + foo(new Param(1, 2)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Param.java b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Param.java new file mode 100644 index 0000000000..0c888bfb95 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Param.java @@ -0,0 +1,11 @@ +public class Param { + private final int[] i; + + public Param(int... i) { + this.i = i; + } + + public int[] getI() { + return i; + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Test.java b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Test.java new file mode 100644 index 0000000000..f5c585cc9c --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameterObject/paramNameConflict/before/Test.java @@ -0,0 +1,11 @@ +class Test { + void foo(int... i) { + if (i.lenght == 0) { + } + Param param = null; + } + + void bar(){ + foo(1, 2); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterObjectTest.java b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterObjectTest.java index 4d368a7773..f45e658416 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterObjectTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterObjectTest.java @@ -14,8 +14,7 @@ import com.intellij.psi.search.GlobalSearchScope; import com.intellij.refactoring.introduceparameterobject.IntroduceParameterObjectProcessor; import com.intellij.refactoring.util.ParameterTablePanel; import com.intellij.JavaTestUtil; - -import java.util.ArrayList; +import com.intellij.util.VisibilityUtil; public class IntroduceParameterObjectTest extends MultiFileTestCase{ protected String getTestRoot() { @@ -39,9 +38,8 @@ public class IntroduceParameterObjectTest extends MultiFileTestCase{ final PsiMethod method = aClass.findMethodsByName("foo", false)[0]; final ParameterTablePanel.VariableData[] datas = generateParams(method); - IntroduceParameterObjectProcessor processor = new IntroduceParameterObjectProcessor("Param", "", method, datas, - null, delegate, false, - createInner); + IntroduceParameterObjectProcessor processor = new IntroduceParameterObjectProcessor("Param", "", method, datas, delegate, false, + createInner, null, false); processor.run(); LocalFileSystem.getInstance().refresh(false); FileDocumentManager.getInstance().saveAllDocuments(); @@ -106,17 +104,24 @@ public class IntroduceParameterObjectTest extends MultiFileTestCase{ doTest(true, false); } - private void doTestExistingClass(final String existingClassName, final String existingClassPackage) throws Exception { + private void doTestExistingClass(final String existingClassName, final String existingClassPackage, final boolean generateAccessors) throws Exception { + doTestExistingClass(existingClassName, existingClassPackage, generateAccessors, null); + } + + private void doTestExistingClass(final String existingClassName, final String existingClassPackage, final boolean generateAccessors, + final String newVisibility) throws Exception { doTest(new PerformAction() { public void performAction(final VirtualFile rootDir, final VirtualFile rootAfter) throws Exception { PsiClass aClass = myJavaFacade.findClass("Test", GlobalSearchScope.projectScope(getProject())); + if (aClass == null) { + aClass = myJavaFacade.findClass("p2.Test", GlobalSearchScope.projectScope(getProject())); + } assertNotNull("Class Test not found", aClass); final PsiMethod method = aClass.findMethodsByName("foo", false)[0]; IntroduceParameterObjectProcessor processor = new IntroduceParameterObjectProcessor(existingClassName, existingClassPackage, method, - generateParams(method), - new ArrayList(), false, true, - false); + generateParams(method), false, true, + false, newVisibility, generateAccessors); processor.run(); LocalFileSystem.getInstance().refresh(false); FileDocumentManager.getInstance().saveAllDocuments(); @@ -125,17 +130,18 @@ public class IntroduceParameterObjectTest extends MultiFileTestCase{ } public void testIntegerWrapper() throws Exception { - doTestExistingClass("Integer", "java.lang"); + doTestExistingClass("Integer", "java.lang", false); } public void testIntegerIncremental() throws Exception { checkExceptionThrown("Integer", "java.lang", "Cannot perform the refactoring.\n" + - "Selected class is not compatible with chosen parameters"); + "Setters for the following fields are required:\n" + + "value.\n"); } private void checkExceptionThrown(String existingClassName, String existingClassPackage, String exceptionMessage) throws Exception { try { - doTestExistingClass(existingClassName, existingClassPackage); + doTestExistingClass(existingClassName, existingClassPackage, false); } catch (BaseRefactoringProcessor.ConflictsInTestsException e) { assertEquals(exceptionMessage, e.getMessage()); @@ -144,12 +150,28 @@ public class IntroduceParameterObjectTest extends MultiFileTestCase{ fail("Conflict was not found"); } + public void testGenerateGetterSetterForExistingBean() throws Exception { + doTestExistingClass("Param", "", true); + } + + public void testExistingBeanVisibility() throws Exception { + doTestExistingClass("Param", "p", false, VisibilityUtil.ESCALATE_VISIBILITY); + } + + public void testExistingBeanIfNoGeneration() throws Exception { + checkExceptionThrown("Param", "", "Cannot perform the refactoring.\n" + "Setters for the following fields are required:\n" + "i.\n"); + } + + public void testParamNameConflict() throws Exception { + doTestExistingClass("Param", "", true); + } + public void testExistentBean() throws Exception { - doTestExistingClass("Param", ""); + doTestExistingClass("Param", "", false); } public void testWrongBean() throws Exception { - checkExceptionThrown("Param", "", "Cannot perform the refactoring.\n" + "Selected class is not compatible with chosen parameters"); + checkExceptionThrown("Param", "", "Cannot perform the refactoring.\n" + "Getters for the following fields are required:\n" + "i.\n"); } } \ No newline at end of file diff --git a/java/openapi/src/com/intellij/util/VisibilityUtil.java b/java/openapi/src/com/intellij/util/VisibilityUtil.java index 0f92de1111..a5505ac27d 100644 --- a/java/openapi/src/com/intellij/util/VisibilityUtil.java +++ b/java/openapi/src/com/intellij/util/VisibilityUtil.java @@ -28,6 +28,7 @@ import com.intellij.psi.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; +import com.intellij.usageView.UsageInfo; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; @@ -121,4 +122,25 @@ public class VisibilityUtil { public static String toPresentableText(@Modifier String modifier) { return PsiBundle.visibilityPresentation(modifier); } + + public static void fixVisibility(UsageInfo[] usageInfos, PsiMember member, final String newVisibility) { + if (newVisibility == null) return; + if (ESCALATE_VISIBILITY.equals(newVisibility)) { + for (UsageInfo info : usageInfos) { + final PsiElement element = info.getElement(); + if (element != null) { + escalateVisibility(member, element); + } + } + } else { + setVisibility(member.getModifierList(), newVisibility); + } + } + + public static void setVisibility(PsiModifierList modifierList, @Modifier String newVisibility) throws IncorrectOperationException { + modifierList.setModifierProperty(PsiModifier.PRIVATE, false); + modifierList.setModifierProperty(PsiModifier.PUBLIC, false); + modifierList.setModifierProperty(PsiModifier.PROTECTED, false); + modifierList.setModifierProperty(newVisibility, true); + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/move/MoveGroovyMemberHandler.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/move/MoveGroovyMemberHandler.java index 888eb943a2..333f4026e3 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/move/MoveGroovyMemberHandler.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/move/MoveGroovyMemberHandler.java @@ -25,7 +25,6 @@ import com.intellij.refactoring.move.moveMembers.MoveMemberHandler; import com.intellij.refactoring.move.moveMembers.MoveMembersOptions; import com.intellij.refactoring.move.moveMembers.MoveMembersProcessor; import com.intellij.refactoring.util.EnumConstantsUtil; -import com.intellij.refactoring.util.RefactoringConflictsUtil; import com.intellij.refactoring.util.RefactoringHierarchyUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.VisibilityUtil; @@ -155,7 +154,7 @@ public class MoveGroovyMemberHandler implements MoveMemberHandler { //might need to make modifiers explicit, see IDEADEV-11416 final PsiModifierList list = memberCopy.getModifierList(); assert list != null; - RefactoringConflictsUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); + VisibilityUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); list.setModifierProperty(PsiModifier.STATIC, member.hasModifierProperty(PsiModifier.STATIC)); list.setModifierProperty(PsiModifier.FINAL, member.hasModifierProperty(PsiModifier.FINAL)); } @@ -172,7 +171,7 @@ public class MoveGroovyMemberHandler implements MoveMemberHandler { assert list != null; list.setModifierProperty(PsiModifier.STATIC, member.hasModifierProperty(PsiModifier.STATIC)); list.setModifierProperty(PsiModifier.FINAL, member.hasModifierProperty(PsiModifier.FINAL)); - RefactoringConflictsUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); + VisibilityUtil.setVisibility(list, VisibilityUtil.getVisibilityModifier(member.getModifierList())); } member.delete(); -- 2.11.4.GIT