From 77009d153eba5a472106d15d3c62062026f6c721 Mon Sep 17 00:00:00 2001 From: anna Date: Wed, 22 Dec 2010 22:20:03 +0300 Subject: [PATCH] allow to delete unused params if replaceAll is selected (IDEA-62186) --- .../IntroduceParameterDialog.java | 16 +++++++++++---- .../IntroduceParameterHandler.java | 11 +++++----- .../refactoring/introduceParameter/Util.java | 18 +++++++++++++++-- .../afterReplaceAllAndDeleteUnused.java | 11 ++++++++++ .../beforeReplaceAllAndDeleteUnused.java | 11 ++++++++++ .../refactoring/IntroduceParameterTest.java | 20 ++++++++++++++++--- .../IntroduceParameterTest.java | 2 +- 7 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceParameter/afterReplaceAllAndDeleteUnused.java create mode 100644 java/java-tests/testData/refactoring/introduceParameter/beforeReplaceAllAndDeleteUnused.java diff --git a/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterDialog.java b/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterDialog.java index 0c1686fa719d..ea50ab941de6 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterDialog.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterDialog.java @@ -300,11 +300,13 @@ public class IntroduceParameterDialog extends RefactoringDialog { myCbGenerateDelegate = new NonFocusableCheckBox(RefactoringBundle.message("delegation.panel.delegate.via.overloading.method")); panel.add(myCbGenerateDelegate, gbConstraints); + final JCheckBox[] removeParamsCb = new JCheckBox[myParametersToRemove.length]; for (int i = 0; i < myParametersToRemove.length; i++) { PsiParameter parameter = myParametersToRemove[i]; if (parameter == null) continue; final NonFocusableCheckBox cb = new NonFocusableCheckBox(RefactoringBundle.message("remove.parameter.0.no.longer.used", parameter.getName())); + removeParamsCb[i] = cb; cb.setSelected(true); gbConstraints.gridy++; panel.add(cb, gbConstraints); @@ -317,12 +319,13 @@ public class IntroduceParameterDialog extends RefactoringDialog { myParametersToRemoveChecked[i] = true; } - updateControls(); + updateControls(removeParamsCb); if (myCbReplaceAllOccurences != null) { myCbReplaceAllOccurences.addItemListener( new ItemListener() { public void itemStateChanged(ItemEvent e) { - updateControls(); + updateControls(removeParamsCb); + } } ); @@ -330,8 +333,13 @@ public class IntroduceParameterDialog extends RefactoringDialog { return panel; } - private void updateControls() { - if(myCbReplaceAllOccurences != null) { + private void updateControls(JCheckBox[] removeParamsCb) { + if (myCbReplaceAllOccurences != null) { + for (JCheckBox box : removeParamsCb) { + if (box != null) { + box.setEnabled(myCbReplaceAllOccurences.isSelected()); + } + } myTypeSelectorManager.setAllOccurences(myCbReplaceAllOccurences.isSelected()); if(myCbReplaceAllOccurences.isSelected()) { if (myCbDeleteLocalVariable != null) { diff --git a/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterHandler.java b/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterHandler.java index a6af24a45a59..9113660522d7 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceParameter/IntroduceParameterHandler.java @@ -141,12 +141,6 @@ public class IntroduceParameterHandler extends IntroduceHandlerBase implements R if (methodToSearchFor == null) return false; if (!CommonRefactoringUtil.checkReadOnlyStatus(project, methodToSearchFor)) return false; - PsiExpression expressionToRemoveParamFrom = expr; - if (expr == null) { - expressionToRemoveParamFrom = localVar.getInitializer(); - } - TIntArrayList parametersToRemove = expressionToRemoveParamFrom == null ? new TIntArrayList() : Util.findParametersToRemove(method, expressionToRemoveParamFrom); - PsiExpression[] occurences; if (expr != null) { occurences = new ExpressionOccurenceManager(expr, method, null).findExpressionOccurrences(); @@ -154,6 +148,11 @@ public class IntroduceParameterHandler extends IntroduceHandlerBase implements R else { // local variable occurences = CodeInsightUtil.findReferenceExpressions(method, localVar); } + PsiExpression expressionToRemoveParamFrom = expr; + if (expr == null) { + expressionToRemoveParamFrom = localVar.getInitializer(); + } + TIntArrayList parametersToRemove = expressionToRemoveParamFrom == null ? new TIntArrayList() : Util.findParametersToRemove(method, expressionToRemoveParamFrom, occurences); if (editor != null) { RefactoringUtil.highlightAllOccurences(myProject, occurences, editor); } diff --git a/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java b/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java index 5d56382918f0..fb94ecbf37b5 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java @@ -37,6 +37,7 @@ import gnu.trove.TIntArrayList; import gnu.trove.TIntHashSet; import gnu.trove.TIntIterator; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.List; @@ -102,7 +103,9 @@ public class Util { // returns parameters that are used solely in specified expression @NotNull - public static TIntArrayList findParametersToRemove(@NotNull PsiMethod method, @NotNull final PsiExpression expr) { + public static TIntArrayList findParametersToRemove(@NotNull PsiMethod method, + @NotNull final PsiExpression expr, + @Nullable final PsiExpression[] occurences) { final PsiParameter[] parameters = method.getParameterList().getParameters(); if (parameters.length == 0) return new TIntArrayList(); @@ -133,7 +136,18 @@ public class Util { if (!ReferencesSearch.search(parameter, parameter.getResolveScope(), false).forEach(new Processor() { public boolean process(final PsiReference reference) { PsiElement element = reference.getElement(); - boolean stillCanBeRemoved = element != null && (PsiTreeUtil.isAncestor(expr, element, false) || PsiUtil.isInsideJavadocComment(element)); + boolean stillCanBeRemoved = false; + if (element != null) { + stillCanBeRemoved = PsiTreeUtil.isAncestor(expr, element, false) || PsiUtil.isInsideJavadocComment(element); + if (!stillCanBeRemoved && occurences != null) { + for (PsiExpression occurence : occurences) { + if (PsiTreeUtil.isAncestor(occurence, element, false)) { + stillCanBeRemoved = true; + break; + } + } + } + } if (!stillCanBeRemoved) { iterator.remove(); return false; diff --git a/java/java-tests/testData/refactoring/introduceParameter/afterReplaceAllAndDeleteUnused.java b/java/java-tests/testData/refactoring/introduceParameter/afterReplaceAllAndDeleteUnused.java new file mode 100644 index 000000000000..8f99f871ea4c --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/afterReplaceAllAndDeleteUnused.java @@ -0,0 +1,11 @@ +public class Parameters { + public void subject(final int anObject) { + System.out.println(anObject); + System.out.println(anObject); + } + + public void context() { + subject(1 +1); + subject(2 +1); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameter/beforeReplaceAllAndDeleteUnused.java b/java/java-tests/testData/refactoring/introduceParameter/beforeReplaceAllAndDeleteUnused.java new file mode 100644 index 000000000000..820f5e59bcdc --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/beforeReplaceAllAndDeleteUnused.java @@ -0,0 +1,11 @@ +public class Parameters { + public void subject(int p) { + System.out.println(p+1); + System.out.println(p+1); + } + + public void context() { + subject(1); + subject(2); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java index db8f8863a438..cba854352cc2 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java @@ -21,6 +21,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.introduceParameter.IntroduceParameterHandler; import com.intellij.refactoring.introduceParameter.IntroduceParameterProcessor; import com.intellij.refactoring.introduceParameter.Util; +import com.intellij.refactoring.util.occurences.ExpressionOccurenceManager; import com.intellij.testFramework.LightCodeInsightTestCase; import com.intellij.testFramework.TestDataPath; import gnu.trove.TIntArrayList; @@ -253,6 +254,10 @@ public class IntroduceParameterTest extends LightCodeInsightTestCase { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, true, true, true); } + public void testReplaceAllAndDeleteUnused() throws Exception { + doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, false, true, false); + } + private void doTestThroughHandler() throws Exception { configureByFile("/refactoring/introduceParameter/before" + getTestName(false) + ".java"); new IntroduceParameterHandler().invoke(getProject(), myEditor, myFile, new DataContext() { @@ -296,8 +301,17 @@ public class IntroduceParameterTest extends LightCodeInsightTestCase { else { methodToSearchFor = method; } - PsiExpression initializer = expr == null ? localVariable.getInitializer() : expr; - TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, initializer) : new TIntArrayList(); + PsiExpression[] occurences = null; + PsiExpression initializer; + if (expr == null) { + initializer = localVariable.getInitializer(); + occurences = CodeInsightUtil.findReferenceExpressions(method, localVariable); + } + else { + initializer = expr; + occurences = new ExpressionOccurenceManager(expr, method, null).findExpressionOccurrences(); + } + TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, initializer, occurences) : new TIntArrayList(); new IntroduceParameterProcessor( getProject(), method, methodToSearchFor, initializer, expr, localVariable, true, parameterName, replaceAllOccurences, replaceFieldsWithGetters, @@ -325,7 +339,7 @@ public class IntroduceParameterTest extends LightCodeInsightTestCase { assertNotNull(methodToSearchFor); final PsiLocalVariable localVariable = (PsiLocalVariable)element; final PsiExpression parameterInitializer = localVariable.getInitializer(); - TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, parameterInitializer) : new TIntArrayList(); + TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, parameterInitializer, null) : new TIntArrayList(); new IntroduceParameterProcessor( getProject(), method, methodToSearchFor, diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/IntroduceParameterTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/IntroduceParameterTest.java index 82ead2c02a9f..2a5f58f23970 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/IntroduceParameterTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/IntroduceParameterTest.java @@ -97,7 +97,7 @@ public class IntroduceParameterTest extends LightCodeInsightFixtureTestCase { } PsiExpression initializer = (expr == null) ? localVariable.getInitializer() : expr; - TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, initializer) : new TIntArrayList(); + TIntArrayList parametersToRemove = removeUnusedParameters ? Util.findParametersToRemove(method, initializer, null) : new TIntArrayList(); final Project project = myFixture.getProject(); final IntroduceParameterProcessor processor = new IntroduceParameterProcessor(project, method, methodToSearchFor, initializer, expr, localVariable, true, parameterName,