Java: Support parameter folding in parametrized duplicates of the extracted method (IDEA-179924)

2026-09-18 09:34:34 +07:00 · 2017-10-04 16:58:16 +03:00
parent 3bcf150c91
commit 32f0a9f3ef
15 changed files with 321 additions and 31 deletions
@@ -63,7 +63,7 @@ public class InputVariables {
  /**
   * copy use only
   */
-  public InputVariables(List<VariableData> inputVariables,
+  private InputVariables(List<VariableData> inputVariables,
                        Project project,
                        LocalSearchScope scope) {
    myProject = project;
@@ -50,6 +50,8 @@ public class DuplicatesFinder {
  private boolean myMultipleExitPoints;
  @Nullable private final ReturnValue myReturnValue;
  private final boolean myWithExtractedParameters;
+  private ParameterFolding myPatternParameterFolding;
+  private ParameterFolding myCandidateParameterFolding;

  public DuplicatesFinder(@NotNull PsiElement[] pattern,
                          InputVariables parameters,
@@ -321,10 +323,12 @@ public class DuplicatesFinder {
    if (pattern == null || candidate == null) return pattern == candidate;
    if (pattern.getUserData(PARAMETER) != null) {
      final Pair<PsiVariable, PsiType> parameter = pattern.getUserData(PARAMETER);
-      return match.putParameter(parameter, candidate);
+      if(!myWithExtractedParameters || parameter.second.equals(parameter.first.getType())) {
+        return match.putParameter(parameter, candidate);
+      }
    }

-    Boolean matchedExtractablePart = matchExtractableExpression(pattern, candidate, candidates, match);
+    Boolean matchedExtractablePart = matchExtractableExpression(pattern, candidate, candidates, match, false);
    if (matchedExtractablePart != null) return matchedExtractablePart;

    if (!canBeEquivalent(pattern, candidate)) return false; // Q : is it correct to check implementation classes?
@@ -539,7 +543,10 @@ public class DuplicatesFinder {
    for (int i = 0; i < children1.length; i++) {
      PsiElement child1 = children1[i];
      PsiElement child2 = children2[i];
-      if (!matchPattern(child1, child2, candidates, match)) return false;
+      if (!matchPattern(child1, child2, candidates, match)) {
+        matchedExtractablePart = matchExtractableExpression(child1, child2, candidates, match, true);
+        return matchedExtractablePart != null && matchedExtractablePart;
+      }
    }

    if (children1.length == 0) {
@@ -553,24 +560,43 @@ public class DuplicatesFinder {
  }

  @Nullable
-  private Boolean matchExtractableExpression(PsiElement pattern, PsiElement candidate, List<PsiElement> candidates, Match match) {
-    if (!(pattern instanceof PsiExpression) || !(candidate instanceof PsiExpression)) {
+  private Boolean matchExtractableExpression(PsiElement pattern, PsiElement candidate,
+                                             List<PsiElement> candidates, Match match,
+                                             boolean withFolding) {
+    if (!(pattern instanceof PsiExpression) || !(candidate instanceof PsiExpression) || withFolding && !myWithExtractedParameters) {
      return null;
    }
-    ExtractableExpressionPart part1 = ExtractableExpressionPart.match((PsiExpression)pattern, myPatternAsList);
-    if (part1 == null) {
+
+    ParameterFolding patternFolding = null;
+    if (withFolding) {
+      if (myPatternParameterFolding == null) {
+        myPatternParameterFolding = new ParameterFolding(myPatternAsList);
+      }
+      patternFolding = myPatternParameterFolding;
+    }
+    ExtractableExpressionPart patternPart = ExtractableExpressionPart.match((PsiExpression)pattern, myPatternAsList, patternFolding);
+    if (patternPart == null) {
      return null;
    }
-    ExtractableExpressionPart part2 = ExtractableExpressionPart.match((PsiExpression)candidate, candidates);
-    if (part2 == null) {
+
+    ParameterFolding candidatesFolding = null;
+    if (withFolding) {
+      if (myCandidateParameterFolding == null || myCandidateParameterFolding.getScope() != candidates) {
+        myCandidateParameterFolding = new ParameterFolding(candidates);
+      }
+      candidatesFolding = myCandidateParameterFolding;
+    }
+    ExtractableExpressionPart candidatePart = ExtractableExpressionPart.match((PsiExpression)candidate, candidates, candidatesFolding);
+    if (candidatePart == null) {
      return null;
    }
-    if (part1.myValue != null && part2.myValue != null && part1.myValue.equals(part2.myValue)) {
+
+    if (patternPart.myValue != null && candidatePart.myValue != null && patternPart.myValue.equals(candidatePart.myValue)) {
      return true;
    }
-    if (part1.myVariable == null || part2.myVariable == null) {
+    if (patternPart.myVariable == null || candidatePart.myVariable == null) {
      return myWithExtractedParameters &&
-             match.putExtractedParameter(part1, part2);
+             match.putExtractedParameter(patternPart, candidatePart);
    }
    return null;
  }
@@ -15,6 +15,7 @@
 */
 package com.intellij.refactoring.util.duplicates;

+import com.intellij.codeInsight.JavaPsiEquivalenceUtil;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
@@ -63,11 +64,14 @@ public class ExtractableExpressionPart {
    if (myValue != null && myValue.equals(part.myValue)) {
      return true;
    }
-    return false;
+    return JavaPsiEquivalenceUtil.areExpressionsEquivalent(PsiUtil.skipParenthesizedExprDown(myUsage),
+                                                           PsiUtil.skipParenthesizedExprDown(part.myUsage));
  }

  @Nullable
-  static ExtractableExpressionPart match(@NotNull PsiExpression expression, @Nullable List<PsiElement> scope) {
+  static ExtractableExpressionPart match(@NotNull PsiExpression expression,
+                                         @NotNull List<PsiElement> scope,
+                                         @Nullable ParameterFolding parameterFolding) {
    if (PsiUtil.isConstantExpression(expression)) {
      if (PsiTreeUtil.findChildOfType(expression, PsiJavaCodeReferenceElement.class) != null) {
        return null;
@@ -77,6 +81,12 @@ public class ExtractableExpressionPart {
    if (expression instanceof PsiReferenceExpression) {
      return matchVariable((PsiReferenceExpression)expression, scope);
    }
+    if (parameterFolding != null && parameterFolding.isAcceptableComplexity(expression)) {
+      PsiType type = expression.getType();
+      if (type != null && !PsiType.VOID.equals(type)) {
+        return new ExtractableExpressionPart(expression, null, null, type);
+      }
+    }
    return null;
  }

@@ -0,0 +1,130 @@
+// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.refactoring.util.duplicates;
+
+import com.intellij.psi.*;
+import com.intellij.psi.tree.IElementType;
+import com.intellij.psi.util.PsiUtil;
+import com.intellij.util.containers.ObjectIntHashMap;
+
+import java.util.List;
+
+/**
+ * @author Pavel.Dolgov
+ */
+class ParameterFolding {
+  static final int MAX_ACCEPTABLE = 9;
+  static final int TOO_COMPLEX = 100;
+
+  private final ObjectIntHashMap<PsiExpression> myCache = new ObjectIntHashMap<>();
+  private final List<PsiElement> myScope;
+
+  ParameterFolding(List<PsiElement> scope) {myScope = scope;}
+
+  boolean isAcceptableComplexity(PsiExpression expression) {
+    return getComplexity(expression) <= MAX_ACCEPTABLE;
+  }
+
+  private int getComplexity(PsiExpression expression) {
+    int complexity = myCache.get(expression);
+    if (complexity < 0) {
+      complexity = computeComplexity(expression);
+      myCache.put(expression, complexity);
+    }
+    return complexity;
+  }
+
+  private int computeComplexity(PsiExpression expression) {
+    expression = PsiUtil.skipParenthesizedExprDown(expression);
+
+    if (expression instanceof PsiLiteralExpression || expression instanceof PsiQualifiedExpression) {
+      return 1;
+    }
+
+    if (expression instanceof PsiAssignmentExpression) {
+      return TOO_COMPLEX;
+    }
+
+    if (expression instanceof PsiUnaryExpression) {
+      IElementType tokenType = ((PsiUnaryExpression)expression).getOperationTokenType();
+      if (JavaTokenType.PLUSPLUS.equals(tokenType) || JavaTokenType.MINUSMINUS.equals(tokenType)) {
+        return TOO_COMPLEX;
+      }
+      return 1 + getComplexity(((PsiUnaryExpression)expression).getOperand());
+    }
+
+    if (expression instanceof PsiBinaryExpression) {
+      int complexity = 1 + getComplexity(((PsiBinaryExpression)expression).getLOperand());
+      if (complexity > MAX_ACCEPTABLE) return complexity;
+      return complexity + getComplexity(((PsiBinaryExpression)expression).getROperand());
+    }
+
+    if (expression instanceof PsiConditionalExpression) {
+      int complexity = 1 + getComplexity(((PsiConditionalExpression)expression).getCondition());
+      if (complexity > MAX_ACCEPTABLE) return complexity;
+
+      complexity += getComplexity(((PsiConditionalExpression)expression).getThenExpression());
+      if (complexity > MAX_ACCEPTABLE) return complexity;
+
+      return complexity + getComplexity(((PsiConditionalExpression)expression).getElseExpression());
+    }
+
+    if (expression instanceof PsiArrayAccessExpression) {
+      int complexity = 3 + getComplexity(((PsiArrayAccessExpression)expression).getArrayExpression());
+      if (complexity > MAX_ACCEPTABLE) return complexity;
+
+      return complexity + getComplexity(((PsiArrayAccessExpression)expression).getIndexExpression());
+    }
+
+    if (expression instanceof PsiReferenceExpression) {
+      PsiElement resolved = ((PsiReferenceExpression)expression).resolve();
+      if (resolved == null || isWithinScope(resolved)) {
+        return TOO_COMPLEX;
+      }
+      if (resolved instanceof PsiVariable &&
+          ((PsiVariable)resolved).hasModifierProperty(PsiModifier.STATIC) &&
+          ((PsiVariable)resolved).hasModifierProperty(PsiModifier.FINAL)) {
+        return 1;
+      }
+
+      PsiExpression qualifier = ((PsiReferenceExpression)expression).getQualifierExpression();
+      if (qualifier == null) {
+        return 2;
+      }
+      return 2 + getComplexity(qualifier);
+    }
+
+    if (expression instanceof PsiMethodCallExpression) {
+      PsiReferenceExpression methodExpression = ((PsiMethodCallExpression)expression).getMethodExpression();
+      PsiElement resolved = methodExpression.resolve();
+      if (resolved == null || isWithinScope(resolved)) {
+        return TOO_COMPLEX;
+      }
+      int complexity = 3;
+      PsiExpression qualifier = methodExpression.getQualifierExpression();
+      if (qualifier != null) {
+        complexity += getComplexity(qualifier);
+        if (complexity > MAX_ACCEPTABLE) {
+          return complexity;
+        }
+      }
+      PsiExpression[] arguments = ((PsiMethodCallExpression)expression).getArgumentList().getExpressions();
+      for (PsiExpression argument : arguments) {
+        complexity += getComplexity(argument);
+        if (complexity > MAX_ACCEPTABLE) {
+          return complexity;
+        }
+      }
+      return complexity;
+    }
+
+    return TOO_COMPLEX;
+  }
+
+  private boolean isWithinScope(PsiElement resolved) {
+    return DuplicatesFinder.isUnder(resolved, myScope);
+  }
+
+  public List<PsiElement> getScope() {
+    return myScope;
+  }
+}
@@ -6,6 +6,8 @@ package com.intellij.refactoring.extractMethod;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
+import com.intellij.psi.search.LocalSearchScope;
+import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.HelpID;
@@ -15,7 +17,9 @@ import com.intellij.refactoring.util.VariableData;
 import com.intellij.refactoring.util.duplicates.*;
 import com.intellij.util.ArrayUtil;
 import com.intellij.util.IncorrectOperationException;
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;

 import java.util.*;

@@ -42,24 +46,28 @@ public class JavaDuplicatesExtractMethodProcessor extends ExtractMethodProcessor
    myOutputVariable = ArrayUtil.getFirstElement(myOutputVariables);
    myArtificialOutputVariable = variablesMapping.getOrDefault(from.myArtificialOutputVariable, from.myArtificialOutputVariable);

-    int parameterCount = Math.max(myInputVariables.getInputVariables().size(), from.myVariableDatum.length);
-    myVariableDatum = new VariableData[parameterCount];
+    List<VariableData> variableDatum = new ArrayList<>();
    for (int i = 0; i < from.myVariableDatum.length; i++) {
      VariableData fromData = from.myVariableDatum[i];
      PsiVariable mappedVariable = variablesMapping.get(fromData.variable);
-      if (mappedVariable == null) {
-        myVariableDatum[i] = fromData;
-      }
-      else {
-        myVariableDatum[i] = new VariableData(mappedVariable, fromData.type);
-        myVariableDatum[i].name = fromData.name;
-        myVariableDatum[i].originalName = fromData.originalName;
-        myVariableDatum[i].passAsParameter = fromData.passAsParameter;
+      if (isReferenced(mappedVariable)) {
+        VariableData newData = new VariableData(mappedVariable, fromData.type);
+        newData.name = fromData.name;
+        newData.originalName = fromData.originalName;
+        newData.passAsParameter = fromData.passAsParameter;
+        variableDatum.add(newData);
      }
    }
-    for (int i = from.myVariableDatum.length; i < myVariableDatum.length; i++) {
-      myVariableDatum[i] = myInputVariables.getInputVariables().get(i);
+    List<VariableData> inputVariables = getInputVariables().getInputVariables();
+    for (int i = variableDatum.size(); i < inputVariables.size(); i++) {
+      variableDatum.add(inputVariables.get(i));
    }
+    myVariableDatum = variableDatum.toArray(new VariableData[0]);
+  }
+
+  @Contract("null -> false")
+  private boolean isReferenced(@Nullable PsiVariable variable) {
+    return variable != null && ReferencesSearch.search(variable, new LocalSearchScope(myElements)).findFirst() != null;
  }

  public void applyDefaults(@NotNull String methodName, @PsiModifier.ModifierConstant @NotNull String visibility) {
@@ -22,6 +22,7 @@ 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.search.LocalSearchScope;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.introduceParameter.IntroduceParameterHandler;
@@ -30,6 +31,7 @@ import com.intellij.refactoring.util.duplicates.DuplicatesFinder;
 import com.intellij.refactoring.util.duplicates.ExtractedParameter;
 import com.intellij.refactoring.util.duplicates.Match;
 import com.intellij.refactoring.util.duplicates.VariableReturnValue;
+import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.text.UniqueNameGenerator;
 import gnu.trove.THashMap;
 import gnu.trove.THashSet;
@@ -53,7 +55,7 @@ public class ParametrizedDuplicates {
  private PsiMethodCallExpression myParametrizedCall;
  private VariableData[] myVariableData;

-  public ParametrizedDuplicates(PsiElement[] pattern) {
+  private ParametrizedDuplicates(PsiElement[] pattern) {
    if (pattern[0] instanceof PsiStatement) {
      Project project = pattern[0].getProject();
      PsiElement[] copy = IntroduceParameterHandler.getElementsInCopy(project, pattern[0].getContainingFile(), pattern);
@@ -90,10 +92,12 @@ public class ParametrizedDuplicates {
  private static List<Match> findOriginalDuplicates(@NotNull ExtractMethodProcessor processor) {
    PsiElement[] elements = getFilteredElements(processor.myElements);

-    DuplicatesFinder finder = new DuplicatesFinder(elements, processor.myInputVariables.copy(),
+    List<PsiVariable> variables = ContainerUtil.map(processor.myInputVariables.getInputVariables(), iv -> iv.variable);
+    InputVariables inputVariables = new InputVariables(variables, processor.myProject, new LocalSearchScope(processor.myElements), false);
+    DuplicatesFinder finder = new DuplicatesFinder(elements, inputVariables,
                                                   processor.myOutputVariable != null
                                                   ? new VariableReturnValue(processor.myOutputVariable) : null,
-                                                   Arrays.asList(processor.myOutputVariables), true) {
+                                                   Collections.emptyList(), true) {
      @Override
      protected boolean isSelf(@NotNull PsiElement candidate) {
        for (PsiElement element : elements) {
@@ -0,0 +1,6 @@
+class Test {
+  public static void main(String[] args, int i) {
+    <selection>System.out.println("hi");</selection>
+    System.out.println(args[i]);
+  }
+}
@@ -0,0 +1,10 @@
+class Test {
+  public static void main(String[] args, int i) {
+      newMethod("hi");
+      newMethod(args[i]);
+  }
+
+    private static void newMethod(String hi) {
+        System.out.println(hi);
+    }
+}
@@ -0,0 +1,8 @@
+import java.util.List;
+
+class Test {
+  public static void foo(List<String> args, int i) {
+    <selection>System.out.println("hi");</selection>
+    System.out.println(args.get(i));
+  }
+}
@@ -0,0 +1,12 @@
+import java.util.List;
+
+class Test {
+  public static void foo(List<String> args, int i) {
+      newMethod("hi");
+      newMethod(args.get(i));
+  }
+
+    private static void newMethod(String hi) {
+        System.out.println(hi);
+    }
+}
@@ -0,0 +1,13 @@
+import java.util.List;
+
+class Test {
+  void foo(String[] a, List<String> b) {
+    for (int i = 0; i < a.length; i++) {
+      <selection>System.out.println("a:" + a[i]);</selection>
+    }
+
+    for (int i = 0; i < b.size(); i++) {
+      System.out.println("b:" + b.get(i));
+    }
+  }
+}
@@ -0,0 +1,17 @@
+import java.util.List;
+
+class Test {
+  void foo(String[] a, List<String> b) {
+    for (int i = 0; i < a.length; i++) {
+        newMethod("a:", a[i]);
+    }
+
+    for (int i = 0; i < b.size(); i++) {
+        newMethod("b:", b.get(i));
+    }
+  }
+
+    private void newMethod(String s, String s2) {
+        System.out.println(s + s2);
+    }
+}
@@ -0,0 +1,13 @@
+import java.util.List;
+
+class Test {
+  void foo(String[] a, List<String> b) {
+    for (int i = 0; i < a.length; i++) {
+      <selection>System.out.println(a[i]);</selection>
+    }
+
+    for (int i = 0; i < b.size(); i++) {
+      System.out.println(b.get(i));
+    }
+  }
+}
@@ -0,0 +1,17 @@
+import java.util.List;
+
+class Test {
+  void foo(String[] a, List<String> b) {
+    for (int i = 0; i < a.length; i++) {
+        newMethod(a[i]);
+    }
+
+    for (int i = 0; i < b.size(); i++) {
+        newMethod(b.get(i));
+    }
+  }
+
+    private void newMethod(String s) {
+        System.out.println(s);
+    }
+}
@@ -783,7 +783,23 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
    doDuplicatesTest();
  }

-  public void _testSuggestChangeSignatureWithFolding() throws Exception {
+  public void testSuggestChangeSignatureWithFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureWithArrayFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureWithGetterFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureWithMultiFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureWithTwoWayFolding() throws Exception {
    doDuplicatesTest();
  }