Java: Fixed safe-deleting variable used in 'for' loop's update or init clause (IDEA-180217)

java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveUnusedVariableUtil.java

@@ -208,7 +208,8 @@ public class RemoveUnusedVariableUtil {
     return true;
   }
 
-  private static boolean isForLoopUpdate(PsiElement element) {
+  public static boolean isForLoopUpdate(@Nullable PsiElement element) {
+    if(element == null) return false;
     PsiElement parent = element.getParent();
     return parent instanceof PsiForStatement &&
            ((PsiForStatement)parent).getUpdate() == element;

java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java

@@ -88,7 +88,13 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase {
     else if (element instanceof PsiLocalVariable) {
       for (PsiReference reference : ReferencesSearch.search(element)) {
         PsiReferenceExpression referencedElement = (PsiReferenceExpression)reference.getElement();
-        final PsiStatement statement = PsiTreeUtil.getParentOfType(referencedElement, PsiStatement.class);
+        PsiElement statementOrExprInList = PsiTreeUtil.getParentOfType(referencedElement, PsiStatement.class);
+        if (statementOrExprInList instanceof PsiExpressionListStatement) {
+          PsiExpressionList expressionList = ((PsiExpressionListStatement)statementOrExprInList).getExpressionList();
+          if (expressionList != null) {
+            statementOrExprInList = PsiTreeUtil.findPrevParent(expressionList, referencedElement);
+          }
+        }
 
         boolean isSafeToDelete = PsiUtil.isAccessedForWriting(referencedElement);
         boolean hasSideEffects = false;
@@ -98,7 +104,7 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase {
               .checkSideEffects(((PsiAssignmentExpression)referencedElement.getParent()).getRExpression(), ((PsiLocalVariable)element),
                                 new ArrayList<>());
         }
-        usages.add(new SafeDeleteReferenceJavaDeleteUsageInfo(statement, element, isSafeToDelete && !hasSideEffects));
+        usages.add(new SafeDeleteReferenceJavaDeleteUsageInfo(statementOrExprInList, element, isSafeToDelete && !hasSideEffects));
       }
     }
     return new NonCodeUsageSearchInfo(insideDeletedCondition, element);

java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteReferenceJavaDeleteUsageInfo.java

@@ -62,7 +62,9 @@ public class SafeDeleteReferenceJavaDeleteUsageInfo extends SafeDeleteReferenceS
         }
       }
       else {
-        if (element instanceof PsiExpressionStatement && RefactoringUtil.isLoopOrIf(element.getParent())) {
+        if (element instanceof PsiExpressionStatement &&
+            RefactoringUtil.isLoopOrIf(element.getParent()) &&
+            !RemoveUnusedVariableUtil.isForLoopUpdate(element)) {
           final PsiStatement emptyTest = JavaPsiFacade.getInstance(getProject()).getElementFactory().createStatementFromText(";", null);
           element.replace(emptyTest);
         } else {

java/java-tests/testData/refactoring/safeDelete/ForInitExpr.java

@@ -0,0 +1,11 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        Object problematic;
+        int i = 10;
+        for(<caret>problematic = foo; (--i) > 0; ) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForInitExpr_after.java

@@ -0,0 +1,10 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        int i = 10;
+        for(; (--i) > 0; ) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForInitList.java

@@ -0,0 +1,11 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        Object problematic;
+        int i;
+        for(i = 10, <caret>problematic = foo; (--i) > 0; ) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForInitList_after.java

@@ -0,0 +1,10 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        int i;
+        for(i = 10; (--i) > 0; ) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForUpdateExpr.java

@@ -0,0 +1,10 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        Object problematic;
+        for(int i = 10; (--i) > 0; <caret>problematic = foo) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForUpdateExpr_after.java

@@ -0,0 +1,9 @@
+class C {
+    Object foo = null;
+
+    void case01() {
+        for(int i = 10; (--i) > 0; ) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForUpdateList.java

@@ -0,0 +1,10 @@
+class C {
+    Object foo = null;
+
+    void case02() {
+        Object problematic;
+        for(int i = 10; i > 0; i--, <caret>problematic = foo) {
+            System.out.println("index = " + i);
+        }
+    }
+}

java/java-tests/testData/refactoring/safeDelete/ForUpdateList_after.java

@@ -0,0 +1,9 @@
+class C {
+    Object foo = null;
+
+    void case02() {
+        for(int i = 10; i > 0; i--) {
+            System.out.println("index = " + i);
+        }
+    }
+}

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

@@ -382,6 +382,22 @@ public class SafeDeleteTest extends MultiFileTestCase {
     doSingleFileTest();
   }
 
+  public void testForInitExpr() throws Exception {
+    doSingleFileTest();
+  }
+
+  public void testForInitList() throws Exception {
+    doSingleFileTest();
+  }
+
+  public void testForUpdateExpr() throws Exception {
+    doSingleFileTest();
+  }
+
+  public void testForUpdateList() throws Exception {
+    doSingleFileTest();
+  }
+
   private void doTest(@NonNls final String qClassName) {
     doTest((rootDir, rootAfter) -> this.performAction(qClassName));
   }