Java: Extract method from duplicates where some of parameters are hard-coded (IDEA-180092)

java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java

@@ -44,4 +44,16 @@ public class VariableData extends AbstractVariableData {
       this.type = type;
     }
   }
+
+  @NotNull
+  public VariableData substitute(@Nullable PsiVariable var) {
+    if (var == null) {
+      return this;
+    }
+    VariableData data = new VariableData(var, type);
+    data.name = name;
+    data.originalName = originalName;
+    data.passAsParameter = passAsParameter;
+    return data;
+  }
 }

java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java

@@ -56,6 +56,17 @@ public class ExtractableExpressionPart {
     return null;
   }
 
+  @NotNull
+  ExtractableExpressionPart copy() {
+    return new ExtractableExpressionPart(myUsage, myVariable, myValue, myType);
+  }
+
+  @NotNull
+  ExtractableExpressionPart deepCopy() {
+    PsiElementFactory factory = JavaPsiFacade.getElementFactory(myUsage.getProject());
+    PsiExpression usageCopy = factory.createExpressionFromText(myUsage.getText(), myUsage);
+    return new ExtractableExpressionPart(usageCopy, myVariable, myValue, myType);
+  }
 
   boolean isEquivalent(@NotNull ExtractableExpressionPart part) {
     if (myVariable != null && myVariable.equals(part.myVariable)) {
@@ -84,10 +95,7 @@ public class ExtractableExpressionPart {
       }
     }
     if (complexityHolder != null && (isConstant || complexityHolder.isAcceptableExpression(expression))) {
-      PsiType type = expression.getType();
-      if (type != null && !PsiType.VOID.equals(type)) {
-        return new ExtractableExpressionPart(expression, null, null, type);
-      }
+      return matchExpression(expression);
     }
     return null;
   }
@@ -115,6 +123,15 @@ public class ExtractableExpressionPart {
     return null;
   }
 
+  @Nullable
+  private static ExtractableExpressionPart matchExpression(@NotNull PsiExpression expression) {
+    PsiType type = expression.getType();
+    if (type != null && !PsiType.VOID.equals(type)) {
+      return new ExtractableExpressionPart(expression, null, null, type);
+    }
+    return null;
+  }
+
   @NotNull
   public PsiExpression getUsage() {
     return myUsage;

java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractedParameter.java

@@ -32,7 +32,7 @@ public class ExtractedParameter {
   @NotNull public final PsiType myType;
   @NotNull public final ExtractableExpressionPart myPattern;
   @NotNull public final ExtractableExpressionPart myCandidate;
-  @NotNull public final Map<PsiExpression, PsiExpression> myUsages = new HashMap<>();
+  @NotNull public final Set<PsiExpression> myPatternUsages = new HashSet<>();
 
   public ExtractedParameter(@NotNull ExtractableExpressionPart patternPart,
                             @NotNull ExtractableExpressionPart candidatePart,
@@ -40,7 +40,7 @@ public class ExtractedParameter {
     myType = type;
     myPattern = patternPart;
     myCandidate = candidatePart;
-    addUsages(patternPart, candidatePart);
+    addUsages(patternPart);
   }
 
   public static boolean match(@NotNull ExtractableExpressionPart patternPart,
@@ -60,7 +60,7 @@ public class ExtractedParameter {
       boolean samePattern = parameter.samePattern(patternPart);
       boolean sameCandidate = parameter.sameCandidate(candidatePart);
       if (samePattern && sameCandidate) {
-        parameter.addUsages(patternPart, candidatePart);
+        parameter.addUsages(patternPart);
         return true;
       }
       if (samePattern || sameCandidate) {
@@ -71,8 +71,20 @@ public class ExtractedParameter {
     return true;
   }
 
-  private void addUsages(ExtractableExpressionPart patternPart, ExtractableExpressionPart candidatePart) {
-    myUsages.put(patternPart.getUsage(), candidatePart.getUsage());
+  @NotNull
+  public ExtractedParameter mapPatternToItself(@NotNull Match match) {
+    ExtractableExpressionPart copy = myPattern.copy();
+    ExtractableExpressionPart deepCopy = myPattern.deepCopy();
+
+    ExtractedParameter parameter = new ExtractedParameter(copy, deepCopy, copy.myType);
+    parameter.myPatternUsages.addAll(myPatternUsages);
+
+    match.getExtractedParameters().add(parameter);
+    return parameter;
+  }
+
+  private void addUsages(ExtractableExpressionPart patternPart) {
+    myPatternUsages.add(patternPart.getUsage());
   }
 
   private boolean sameCandidate(ExtractableExpressionPart part) {

java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java

@@ -1972,7 +1972,7 @@ public class ExtractMethodProcessor implements MatchProvider {
           myExtractedMethod = myParametrizedDuplicates.replaceMethod(myExtractedMethod);
           myMethodCall = myParametrizedDuplicates.replaceCall(myMethodCall);
         });
-        myVariableDatum = myParametrizedDuplicates.getVariableData();
+        myVariableDatum = myParametrizedDuplicates.getVariableDatum();
         return null;
       }
     }

java/java-impl/src/com/intellij/refactoring/extractMethod/JavaDuplicatesExtractMethodProcessor.java

@@ -50,10 +50,7 @@ public class JavaDuplicatesExtractMethodProcessor extends ExtractMethodProcessor
       VariableData fromData = from.myVariableDatum[i];
       PsiVariable mappedVariable = variablesMapping.get(fromData.variable);
       if (isReferenced(mappedVariable, fromData.variable)) {
-        VariableData newData = new VariableData(mappedVariable, fromData.type);
-        newData.name = fromData.name;
-        newData.originalName = fromData.originalName;
-        newData.passAsParameter = fromData.passAsParameter;
+        VariableData newData = fromData.substitute(mappedVariable);
         variableDatum.add(newData);
       }
     }

java/java-impl/src/com/intellij/refactoring/extractMethod/ParametrizedDuplicates.java

@@ -36,6 +36,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.text.UniqueNameGenerator;
 import gnu.trove.THashMap;
 import gnu.trove.THashSet;
+import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -54,7 +55,7 @@ public class ParametrizedDuplicates {
   private List<ClusterOfUsages> myUsagesList;
   private PsiMethod myParametrizedMethod;
   private PsiMethodCallExpression myParametrizedCall;
-  private VariableData[] myVariableData;
+  private VariableData[] myVariableDatum;
 
   private ParametrizedDuplicates(@NotNull PsiElement[] pattern) {
     LOG.assertTrue(pattern.length != 0, "pattern length");
@@ -158,14 +159,28 @@ public class ParametrizedDuplicates {
         }
       }
     }
-    myUsagesList.sort(Comparator.comparing(usages -> usages.myFirstOffset));
 
     if (!badMatches.isEmpty()) {
       matches = new ArrayList<>(matches);
       matches.removeAll(badMatches);
     }
     myMatches = matches;
-    return !myMatches.isEmpty() && !myUsagesList.isEmpty();
+    if (myMatches.isEmpty() || myUsagesList.isEmpty()) {
+      return false;
+    }
+
+    for (ClusterOfUsages usages : myUsagesList) {
+      for (Match match : myMatches) {
+        ExtractedParameter parameter = usages.myParameters.get(match);
+        if (parameter == null) {
+          parameter = usages.myParameter.mapPatternToItself(match);
+          usages.putParameter(match, parameter);
+        }
+      }
+    }
+
+    myUsagesList.sort(Comparator.comparing(usages -> usages.myFirstOffset));
+    return true;
   }
 
   @Nullable
@@ -181,7 +196,7 @@ public class ParametrizedDuplicates {
       if (usages == null) {
         result.add(usages = new ClusterOfUsages(parameter));
       }
-      usages.add(parameter);
+      usages.putParameter(match, parameter);
     }
     return result;
   }
@@ -208,12 +223,21 @@ public class ParametrizedDuplicates {
     parametrizedProcessor.setDataFromInputVariables();
     myParametrizedMethod = parametrizedProcessor.getExtractedMethod();
     myParametrizedCall = parametrizedProcessor.getMethodCall();
-    myVariableData = parametrizedProcessor.myVariableDatum;
+    myVariableDatum = unmapVariableData(parametrizedProcessor.myVariableDatum, variablesMapping);
     replaceArguments(parameterDeclarations, myParametrizedCall);
 
     return true;
   }
 
+  @NotNull
+  private static VariableData[] unmapVariableData(@NotNull VariableData[] variableDatum,
+                                                  @NotNull Map<PsiVariable, PsiVariable> variablesMapping) {
+    Map<PsiVariable, PsiVariable> reverseMapping = ContainerUtil.reverseMap(variablesMapping);
+    return StreamEx.of(variableDatum)
+      .map(data -> data.substitute(reverseMapping.get(data.variable)))
+      .toArray(VariableData[]::new);
+  }
+
   private static void replaceArguments(@NotNull Map<PsiLocalVariable, ClusterOfUsages> parameterDeclarations,
                                        @NotNull PsiMethodCallExpression parametrizedCall) {
     PsiExpression[] arguments = parametrizedCall.getArgumentList().getExpressions();
@@ -233,7 +257,7 @@ public class ParametrizedDuplicates {
   private void putMatchParameters(@NotNull Map<PsiLocalVariable, ClusterOfUsages> parameterDeclarations) {
     Map<PsiExpression, PsiLocalVariable> patternUsageToParameter = new THashMap<>();
     for (Map.Entry<PsiLocalVariable, ClusterOfUsages> entry : parameterDeclarations.entrySet()) {
-      PsiExpression usage = entry.getValue().myParameters.get(0).myPattern.getUsage();
+      PsiExpression usage = entry.getValue().myParameter.myPattern.getUsage();
       patternUsageToParameter.put(usage, entry.getKey());
     }
 
@@ -257,8 +281,8 @@ public class ParametrizedDuplicates {
     return myParametrizedCall;
   }
 
-  public VariableData[] getVariableData() {
-    return myVariableData;
+  public VariableData[] getVariableDatum() {
+    return myVariableDatum;
   }
 
   public int getSize() {
@@ -347,7 +371,7 @@ public class ParametrizedDuplicates {
     LOG.assertTrue(parent instanceof PsiCodeBlock, "first statement's parent isn't a code block");
 
     for (ClusterOfUsages usages : myUsagesList) {
-      ExtractedParameter parameter = usages.myParameters.get(0);
+      ExtractedParameter parameter = usages.myParameter;
       PsiExpression patternUsage = parameter.myPattern.getUsage();
       String initializerText = patternUsage.getText();
       PsiExpression initializer = factory.createExpressionFromText(initializerText, parent);
@@ -361,7 +385,7 @@ public class ParametrizedDuplicates {
       PsiLocalVariable localVariable = (PsiLocalVariable)paramDeclaration.getDeclaredElements()[0];
       parameterDeclarations.put(localVariable, usages);
 
-      for (PsiExpression expression : parameter.myUsages.keySet()) {
+      for (PsiExpression expression : parameter.myPatternUsages) {
         PsiExpression mapped = expressionsMapping.get(expression);
         if (mapped != null) {
           PsiExpression replacement = factory.createExpressionFromText(parameterName, expression);
@@ -444,25 +468,27 @@ public class ParametrizedDuplicates {
 
   private static class ClusterOfUsages {
     @NotNull private final Set<PsiExpression> myPatterns;
-    @NotNull private final List<ExtractedParameter> myParameters;
+    @NotNull private final Map<Match, ExtractedParameter> myParameters;
+    @NotNull private final ExtractedParameter myParameter;
     private final int myFirstOffset;
 
     public ClusterOfUsages(@NotNull ExtractedParameter parameter) {
-      myPatterns = parameter.myUsages.keySet();
-      myParameters = new ArrayList<>();
+      myPatterns = parameter.myPatternUsages;
+      myParameters = new THashMap<>();
+      myParameter = parameter;
       myFirstOffset = myPatterns.stream().mapToInt(PsiElement::getTextOffset).min().orElse(0);
     }
 
-    public void add(ExtractedParameter parameter) {
-      myParameters.add(parameter);
+    public void putParameter(Match match, ExtractedParameter parameter) {
+      myParameters.put(match, parameter);
     }
 
     public boolean isEquivalent(ExtractedParameter parameter) {
-      return myPatterns.equals(parameter.myUsages.keySet());
+      return myPatterns.equals(parameter.myPatternUsages);
     }
 
     public static boolean isPresent(Map<PsiExpression, ClusterOfUsages> usagesMap, @NotNull ExtractedParameter parameter) {
-      return parameter.myUsages.keySet().stream().anyMatch(expression -> usagesMap.get(expression) != null);
+      return parameter.myPatternUsages.stream().anyMatch(usagesMap::containsKey);
     }
   }
 }

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureFourOccurrencesTwoLiteralFolding.java

@@ -0,0 +1,23 @@
+public class C {
+    void second() {
+        <selection> test(0);
+        test(1);</selection>
+    }
+
+    void none() {
+        test(0);
+        test(0);
+    }
+
+    void both() {
+        test(1);
+        test(1);
+    }
+
+    void first() {
+        test(1);
+        test(0);
+    }
+
+    private void test(int i) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureFourOccurrencesTwoLiteralFolding_after.java

@@ -0,0 +1,24 @@
+public class C {
+    void second() {
+        newMethod(0, 1);
+    }
+
+    private void newMethod(int i, int i2) {
+        test(i);
+        test(i2);
+    }
+
+    void none() {
+        newMethod(0, 0);
+    }
+
+    void both() {
+        newMethod(1, 1);
+    }
+
+    void first() {
+        newMethod(1, 0);
+    }
+
+    private void test(int i) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureFourOccurrencesTwoVariableFolding.java

@@ -0,0 +1,27 @@
+public class C {
+    void both() {
+        int x=0,y=0;
+        a(x);b(y);
+        a(y);b(x);
+    }
+
+    void first() {
+        int k=0;
+        a(k);b(1);
+        a(1);b(k);
+    }
+
+    void second() {
+        int m=1;
+        <selection>a(0);b(m);
+        a(m);b(0);</selection>
+    }
+
+    void baz(int e, int f) {
+        a(e);b(f);
+        a(f);b(e);
+    }
+
+    private void a(int i) {}
+    private void b(int n) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureFourOccurrencesTwoVariableFolding_after.java

@@ -0,0 +1,30 @@
+public class C {
+    void both() {
+        int x=0,y=0;
+        newMethod(y, x);
+    }
+
+    void first() {
+        int k=0;
+        newMethod(1, k);
+    }
+
+    void second() {
+        int m=1;
+        newMethod(m, 0);
+    }
+
+    private void newMethod(int m, int i) {
+        a(i);
+        b(m);
+        a(m);
+        b(i);
+    }
+
+    void baz(int e, int f) {
+        newMethod(f, e);
+    }
+
+    private void a(int i) {}
+    private void b(int n) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureThreeOccurrencesTwoLiteralFolding.java

@@ -0,0 +1,18 @@
+public class C {
+    void second() {
+        <selection> test(0);
+        test(1);</selection>
+    }
+
+    void none() {
+        test(0);
+        test(0);
+    }
+
+    void both() {
+        test(1);
+        test(1);
+    }
+
+    private void test(int i) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureThreeOccurrencesTwoLiteralFolding_after.java

@@ -0,0 +1,20 @@
+public class C {
+    void second() {
+        newMethod(0, 1);
+    }
+
+    private void newMethod(int i, int i2) {
+        test(i);
+        test(i2);
+    }
+
+    void none() {
+        newMethod(0, 0);
+    }
+
+    void both() {
+        newMethod(1, 1);
+    }
+
+    private void test(int i) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureThreeOccurrencesTwoVariableFolding.java

@@ -0,0 +1,21 @@
+public class C {
+    void foo() {
+        int x=0,y=0;
+        <selection>a(x);b(y);
+        a(y);b(x);</selection>
+    }
+
+    void bar() {
+        int k=0,m=1;
+        a(k);b(m);
+        a(m);b(k);
+    }
+
+    void baz(int e, int f) {
+        a(e);b(f);
+        a(f);b(e);
+    }
+
+    private void a(int i) {}
+    private void b(int n) {}
+}

java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureThreeOccurrencesTwoVariableFolding_after.java

@@ -0,0 +1,25 @@
+public class C {
+    void foo() {
+        int x=0,y=0;
+        newMethod(x, y);
+    }
+
+    private void newMethod(int x, int y) {
+        a(x);
+        b(y);
+        a(y);
+        b(x);
+    }
+
+    void bar() {
+        int k=0,m=1;
+        newMethod(k, m);
+    }
+
+    void baz(int e, int f) {
+        newMethod(e, f);
+    }
+
+    private void a(int i) {}
+    private void b(int n) {}
+}

java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java

@@ -811,6 +811,22 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
     doDuplicatesTest();
   }
 
+  public void testSuggestChangeSignatureThreeOccurrencesTwoLiteralFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureFourOccurrencesTwoLiteralFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureThreeOccurrencesTwoVariableFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testSuggestChangeSignatureFourOccurrencesTwoVariableFolding() throws Exception {
+    doDuplicatesTest();
+  }
+
   public void testSuggestChangeSignatureWithOutputVariables() throws Exception {
     doDuplicatesTest();
   }