From c5adb3e2eeb6a476cd6d4110e19d3db50bf91f47 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Mon, 28 Sep 2020 21:45:35 +0200 Subject: [PATCH] java introduce parameter: optimization + progress (IDEA-251661) postpone search for overriding methods until usages in current method is checked GitOrigin-RevId: 1a68af844d1b590203f42a322812b0b065124f5d --- .../refactoring/introduceParameter/Util.java | 74 +++++++++++-------- .../afterRemoveParameterInHierarchy1.java | 11 +++ .../beforeRemoveParameterInHierarchy1.java | 11 +++ .../refactoring/IntroduceParameterTest.java | 4 + .../resources/messages/JavaBundle.properties | 2 +- 5 files changed, 70 insertions(+), 32 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceParameter/afterRemoveParameterInHierarchy1.java create mode 100644 java/java-tests/testData/refactoring/introduceParameter/beforeRemoveParameterInHierarchy1.java 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 02965bcee52f..b8c6b19c5df2 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceParameter/Util.java @@ -3,6 +3,9 @@ package com.intellij.refactoring.introduceParameter; import com.intellij.codeInsight.generation.GenerateMembersUtil; +import com.intellij.java.JavaBundle; +import com.intellij.openapi.application.ReadAction; +import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import com.intellij.psi.search.searches.OverridingMethodsSearch; @@ -102,9 +105,6 @@ public final class Util { final PsiParameter[] parameters = method.getParameterList().getParameters(); if (parameters.length == 0) return new TIntArrayList(); - PsiMethod[] overridingMethods = OverridingMethodsSearch.search(method).toArray(PsiMethod.EMPTY_ARRAY); - final PsiMethod[] allMethods = ArrayUtil.append(overridingMethods, method); - final TIntHashSet suspects = new TIntHashSet(); expr.accept(new JavaRecursiveElementWalkingVisitor() { @Override public void visitReferenceExpression(final PsiReferenceExpression expression) { @@ -119,36 +119,48 @@ public final class Util { } }); - final TIntIterator iterator = suspects.iterator(); - while(iterator.hasNext()) { - final int paramNum = iterator.next(); - for (PsiMethod psiMethod : allMethods) { - PsiParameter[] psiParameters = psiMethod.getParameterList().getParameters(); - if (paramNum >= psiParameters.length) continue; - PsiParameter parameter = psiParameters[paramNum]; - if (!ReferencesSearch.search(parameter, parameter.getResolveScope(), false).forEach(reference -> { - PsiElement element = reference.getElement(); - boolean stillCanBeRemoved = false; - if (element != null) { - stillCanBeRemoved = isAncestor(expr, element, false) || PsiUtil.isInsideJavadocComment(getPhysical(element)); - if (!stillCanBeRemoved && occurences != null) { - for (PsiExpression occurence : occurences) { - if (isAncestor(occurence, element, false)) { - stillCanBeRemoved = true; - break; - } - } - } - } - if (!stillCanBeRemoved) { - iterator.remove(); - return false; - } - return true; - })) break; - } + removeUsed(method, expr, occurences, suspects); + + if (suspects.isEmpty()) return new TIntArrayList(); + + if (!ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> { + OverridingMethodsSearch.search(method).forEach(psiMethod -> { + ReadAction.run(() -> removeUsed(psiMethod, expr, occurences, suspects)); + return !suspects.isEmpty(); + }); + }, JavaBundle.message("progress.title.search.for.overriding.methods"), true, method.getProject())) { + return new TIntArrayList(); } return new TIntArrayList(suspects.toArray()); } + + private static void removeUsed(PsiMethod containingMethod, @NotNull PsiExpression expr, + PsiExpression @Nullable [] occurences, + TIntHashSet suspects) { + final TIntIterator iterator = suspects.iterator(); + while (iterator.hasNext()) { + final int paramNum = iterator.next(); + PsiParameter[] psiParameters = containingMethod.getParameterList().getParameters(); + if (paramNum >= psiParameters.length) continue; + PsiParameter parameter = psiParameters[paramNum]; + ReferencesSearch.search(parameter, parameter.getResolveScope(), false).forEach(reference -> { + PsiElement element = reference.getElement(); + boolean stillCanBeRemoved = isAncestor(expr, element, false) || PsiUtil.isInsideJavadocComment(getPhysical(element)); + if (!stillCanBeRemoved && occurences != null) { + for (PsiExpression occurence : occurences) { + if (isAncestor(occurence, element, false)) { + stillCanBeRemoved = true; + break; + } + } + } + if (!stillCanBeRemoved) { + iterator.remove(); + return false; + } + return true; + }); + } + } } diff --git a/java/java-tests/testData/refactoring/introduceParameter/afterRemoveParameterInHierarchy1.java b/java/java-tests/testData/refactoring/introduceParameter/afterRemoveParameterInHierarchy1.java new file mode 100644 index 000000000000..f245f693ed8b --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/afterRemoveParameterInHierarchy1.java @@ -0,0 +1,11 @@ +public class Bar { + public int baz(byte blah1, int anObject) { + return anObject; + } +} +class S extends Bar { + public int baz(byte blah1, int anObject) { + System.out.println(blah1); + return super.baz((byte) 0, anObject); //To change body of overridden methods use File | Settings | File Templates. + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceParameter/beforeRemoveParameterInHierarchy1.java b/java/java-tests/testData/refactoring/introduceParameter/beforeRemoveParameterInHierarchy1.java new file mode 100644 index 000000000000..3a5d1f79894c --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/beforeRemoveParameterInHierarchy1.java @@ -0,0 +1,11 @@ +public class Bar { + public int baz(byte blah, byte blah1, byte blah2) { + return blah + blah1 + blah2; + } +} +class S extends Bar { + public int baz(byte blah, byte blah1, byte blah2) { + System.out.println(blah1); + return super.baz((byte) 0, (byte) 0, (byte) 0); //To change body of overridden methods use File | Settings | File Templates. + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceParameterTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceParameterTest.java index 94c6bbe2e407..5a6b53a937f8 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceParameterTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceParameterTest.java @@ -210,6 +210,10 @@ public class IntroduceParameterTest extends LightRefactoringTestCase { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, true, false, false, false); } + public void testRemoveParameterInHierarchy1() { + doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, true, false, false, false); + } + public void testRemoveParameterWithJavadoc() { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, true, false, false, false); } diff --git a/java/openapi/resources/messages/JavaBundle.properties b/java/openapi/resources/messages/JavaBundle.properties index 0f7c77bd099f..e216dd0c7303 100644 --- a/java/openapi/resources/messages/JavaBundle.properties +++ b/java/openapi/resources/messages/JavaBundle.properties @@ -1052,7 +1052,7 @@ progress.title.looking.for.jdk.locations=Looking for JDK locations... progress.title.looking.for.libraries=Looking for Libraries progress.title.optimize.imports=Optimize Imports... progress.title.preprocess.usages=Preprocess Usages -progress.title.search.for.overriding.methods=Search for Overriding Methods... +progress.title.search.for.overriding.methods=Search for overriding methods... progress.title.searching.for.sub.classes=Searching for Sub-Classes prompt.choose.base.class.of.the.hierarchy=Choose Base Class of the Hierarchy to Search In prompt.create.non.existing.package=Package {0} does not exist.\nDo you want to create it?