[java-refactoring] Introduce variable: extract loop invariants (at least simple ones) outside of while loop

Fixes some of annoying cases like reported in IDEA-210010

GitOrigin-RevId: b083327807d85666d1f71f96a90d0eddd616754f

java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java

@@ -318,7 +318,8 @@ final class VariableExtractor {
       PsiWhileStatement whileStatement = (PsiWhileStatement)anchor;
       PsiExpression condition = whileStatement.getCondition();
       if (condition != null && allOccurrences.stream().allMatch(occurrence -> PsiTreeUtil.isAncestor(whileStatement, occurrence, true))) {
-        if (firstOccurrence != null && PsiTreeUtil.isAncestor(condition, firstOccurrence, false)) {
+        if (firstOccurrence != null && PsiTreeUtil.isAncestor(condition, firstOccurrence, false) &&
+            !ExpressionUtils.isLoopInvariant(firstOccurrence, whileStatement)) {
           PsiPolyadicExpression polyadic = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(condition), PsiPolyadicExpression.class);
           if (polyadic != null && JavaTokenType.ANDAND.equals(polyadic.getOperationTokenType())) {
             PsiExpression operand = ContainerUtil.find(polyadic.getOperands(), op -> PsiTreeUtil.isAncestor(op, firstOccurrence, false));

java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue.java

@@ -0,0 +1,5 @@
+public class whileTrue {
+  void test() {
+    while(<caret>true) {}
+  }
+}

java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue_after.java

@@ -0,0 +1,6 @@
+public class whileTrue {
+  void test() {
+      boolean b = true;
+      while(b) {}
+  }
+}

java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.after.java

@@ -5,7 +5,7 @@ class Test {
         while (x > 0) {
             boolean temp = z > 0;
             if (!(y < 0 || temp)) break;
-
+            z++;
         }
     }
 }

java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.java

@@ -3,7 +3,7 @@ import java.util.Arrays;
 class Test {
     void test(int x, int y, int z) {
         while (x > 0 && (y < 0 || <selection>z > 0</selection>)) {
-
+          z++;
         }
     }
 }

java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.after.java

@@ -1,8 +1,10 @@
 class Test {
-    void test(boolean foo) {
+    void test() {
         while (true) {
-            boolean temp = foo;
+            boolean temp = foo();
             if (!temp) break;
         }
     }
+    
+    native boolean foo();
 }

java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.java

@@ -1,5 +1,7 @@
 class Test {
-    void test(boolean foo) {
-        while(<selection>foo</selection>)
+    void test() {
+        while(<selection>foo()</selection>)
     }
+    
+    native boolean foo();
 }

java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.after.java

@@ -1,6 +1,7 @@
 class Test {
+  int[] a = new int[10];
+  
     void foo() {
-        int[] a = new int[10];
         int log = 0;
         while (true) {
             int temp = a.length;

java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.java

@@ -1,6 +1,7 @@
 class Test {
+  int[] a = new int[10];
+  
     void foo() {
-        int[] a = new int[10];
         int log = 0;
         while (1 << log < <selection>a.length</selection>) log++;
     }

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

@@ -219,6 +219,10 @@ public class InplaceIntroduceVariableTest extends AbstractJavaInplaceIntroduceTe
   public void testLambdaParameterAddCast() {
     doTestReplaceChoice("Replace all 0 occurrences");
   }
+  
+  public void testWhileTrue() {
+    doTest(null);
+  }
 
   private void doTestStopEditing(Consumer<? super AbstractInplaceIntroducer> pass) {
     String name = getTestName(true);

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java

@@ -4,6 +4,7 @@ package com.siyeh.ig.psiutils;
 import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInsight.CodeInsightUtilCore;
 import com.intellij.codeInsight.NullableNotNullManager;
+import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
 import com.intellij.codeInspection.dataFlow.ContractReturnValue;
 import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
 import com.intellij.openapi.project.Project;
@@ -27,6 +28,7 @@ import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
@@ -1674,4 +1676,29 @@ public final class ExpressionUtils {
     }
     return null;
   }
+
+  /**
+   * @param expression expression to test
+   * @param loopStatement loop statement
+   * @return true if given expression is likely to be a loop invariant. False if it's not invariant, or not known.
+   */
+  public static boolean isLoopInvariant(PsiExpression expression, @SuppressWarnings("unused") PsiLoopStatement loopStatement) {
+    if (PsiUtil.isConstantExpression(expression)) return true;
+    if (SideEffectChecker.mayHaveSideEffects(expression)) return false;
+    Collection<PsiReferenceExpression> refs = PsiTreeUtil.collectElementsOfType(expression, PsiReferenceExpression.class);
+    for (PsiReferenceExpression ref : refs) {
+      PsiElement target = ref.resolve();
+      // TODO: more sophisticated analysis
+      if (target instanceof PsiField && ((PsiField)target).hasModifierProperty(PsiModifier.FINAL)) continue;
+      if (target instanceof PsiLocalVariable || target instanceof PsiParameter) {
+        PsiVariable var = (PsiVariable)target;
+        if (var.hasModifierProperty(PsiModifier.FINAL) ||
+            HighlightControlFlowUtil.isEffectivelyFinal(var, PsiUtil.getVariableCodeBlock(var, null), null)) {
+          continue;
+        }
+      }
+      return false;
+    }
+    return true;
+  }
 }