Java: Fixed extracting method from duplicates containing reused variables (IDEA-188894)

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

@@ -1369,13 +1369,52 @@ public class ExtractMethodProcessor implements MatchProvider {
         methodCallExpression.getArgumentList().add(myElementFactory.createExpressionFromText(data.variable.getName(), methodCallExpression));
       }
     }
+    List<String> reusedVariables = findReusedVariables(match, myOutputVariable);
     PsiElement replacedMatch = match.replace(myExtractedMethod, methodCallExpression, myOutputVariable);
 
-    addNotNullConditionalCheck(match, replacedMatch);
+    PsiElement appendLocation = addNotNullConditionalCheck(match, replacedMatch);
+    declareReusedVariables(appendLocation, reusedVariables);
     return replacedMatch;
   }
 
-  private void addNotNullConditionalCheck(Match match, PsiElement replacedMatch) {
+  @NotNull
+  private static List<String> findReusedVariables(@NotNull Match match, @Nullable PsiVariable outputVariable) {
+    Set<PsiLocalVariable> ignoreVariables = Collections.emptySet();
+    ReturnValue returnValue = match.getOutputVariableValue(outputVariable);
+    if (returnValue instanceof VariableReturnValue) {
+      PsiVariable returnedVariable = ((VariableReturnValue)returnValue).getVariable();
+      if (returnedVariable instanceof PsiLocalVariable) {
+        ignoreVariables = Collections.singleton((PsiLocalVariable)returnedVariable);
+      }
+    }
+    List<ReusedLocalVariable> reusedLocalVariables =
+      ReusedLocalVariablesFinder.findReusedLocalVariables(match.getMatchStart(), match.getMatchEnd(), ignoreVariables);
+    if (!reusedLocalVariables.isEmpty()) {
+      List<String> result = new ArrayList<>();
+      for (ReusedLocalVariable variable : reusedLocalVariables) {
+        if (!variable.reuseValue()) {
+          result.add(variable.getDeclarationText());
+        }
+      }
+      return result;
+    }
+
+    return Collections.emptyList();
+  }
+
+  private static void declareReusedVariables(@NotNull PsiElement appendLocation, @NotNull List<String> reusedVariables) {
+    if (reusedVariables.isEmpty()) {
+      return;
+    }
+    PsiElementFactory factory = JavaPsiFacade.getElementFactory(appendLocation.getProject());
+
+    for (String variable : reusedVariables) {
+      PsiStatement declaration = factory.createStatementFromText(variable, appendLocation);
+      appendLocation = appendLocation.getParent().addAfter(declaration, appendLocation);
+    }
+  }
+
+  private PsiElement addNotNullConditionalCheck(Match match, PsiElement replacedMatch) {
     if ((myNotNullConditionalCheck || myGenerateConditionalExit) && myOutputVariable != null) {
       ReturnValue returnValue = match.getOutputVariableValue(myOutputVariable);
       if (returnValue instanceof VariableReturnValue) {
@@ -1385,10 +1424,11 @@ public class ExtractMethodProcessor implements MatchProvider {
         if (statement != null) {
           PsiStatement conditionalExit = myNotNullConditionalCheck ?
                                          generateNotNullConditionalStatement(varName) : generateConditionalExitStatement(varName);
-          statement.getParent().addAfter(conditionalExit, statement);
+          return statement.getParent().addAfter(conditionalExit, statement);
         }
       }
     }
+    return replacedMatch;
   }
 
   @Nullable

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

@@ -0,0 +1,21 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+        <selection>
+        String s = a.get(1);
+        if (s == null) return;
+        System.out.println(s.charAt(1));
+        </selection>
+        s = "";
+        System.out.println(s.length());
+    }
+
+    void bar(List<String> a) {
+        String s = a.get(1);
+        if (s == null) return;
+        System.out.println(s.charAt(1));
+        s = "";
+        System.out.println(s.length());
+    }
+}

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

@@ -0,0 +1,26 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+
+        if (newMethod(a)) return;
+        String s;
+
+        s = "";
+        System.out.println(s.length());
+    }
+
+    private boolean newMethod(List<String> a) {
+        String s = a.get(1);
+        if (s == null) return true;
+        System.out.println(s.charAt(1));
+        return false;
+    }
+
+    void bar(List<String> a) {
+        if (newMethod(a)) return;
+        String s;
+        s = "";
+        System.out.println(s.length());
+    }
+}

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

@@ -0,0 +1,25 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+        <selection>
+        int n = 1;
+        String s = a.get(n);
+        if (s == null) return;
+        System.out.println(s.charAt(n));
+        </selection>
+        s = "";
+        n = 2;
+        System.out.println(s.length() + n);
+    }
+
+    void bar(List<String> a) {
+        int n = 1;
+        String s = a.get(n);
+        if (s == null) return;
+        System.out.println(s.charAt(n));
+        s = "";
+        n = 2;
+        System.out.println(s.length() + n);
+    }
+}

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

@@ -0,0 +1,31 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+
+        if (newMethod(a)) return;
+        String s;
+        int n;
+
+        s = "";
+        n = 2;
+        System.out.println(s.length() + n);
+    }
+
+    private boolean newMethod(List<String> a) {
+        int n = 1;
+        String s = a.get(n);
+        if (s == null) return true;
+        System.out.println(s.charAt(n));
+        return false;
+    }
+
+    void bar(List<String> a) {
+        if (newMethod(a)) return;
+        int n;
+        String s;
+        s = "";
+        n = 2;
+        System.out.println(s.length() + n);
+    }
+}

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

@@ -0,0 +1,21 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+        <selection>
+        String s = a.get(1);
+        if (s == null) return;
+        System.out.println(s.charAt(1));
+        </selection>
+        s = "";
+        System.out.println(s.length());
+    }
+
+    void bar(List<String> a) {
+        String s = a.get(2);
+        if (s == null) return;
+        System.out.println(s.charAt(2));
+        s = "";
+        System.out.println(s.length());
+    }
+}

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

@@ -0,0 +1,26 @@
+import java.util.List;
+
+class DeclaredOutputVariable {
+    void foo(List<String> a) {
+
+        if (newMethod(a, 1)) return;
+        String s;
+
+        s = "";
+        System.out.println(s.length());
+    }
+
+    private boolean newMethod(List<String> a, int i) {
+        String s = a.get(i);
+        if (s == null) return true;
+        System.out.println(s.charAt(i));
+        return false;
+    }
+
+    void bar(List<String> a) {
+        if (newMethod(a, 2)) return;
+        String s;
+        s = "";
+        System.out.println(s.length());
+    }
+}

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

@@ -877,6 +877,18 @@ public class ExtractMethodTest extends LightCodeInsightTestCase {
     doDuplicatesTest();
   }
 
+  public void testParametrizedDuplicateDeclaredReusedVariable() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testExactDuplicateDeclaredReusedVariable() throws Exception {
+    doDuplicatesTest();
+  }
+
+  public void testExactDuplicateTwoDeclaredReusedVariables() throws Exception {
+    doDuplicatesTest();
+  }
+
   public void testSuggestChangeSignatureWithChangedParameterName() throws Exception {
     configureByFile(BASE_PATH + getTestName(false) + ".java");
     boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, "p");