extract method: disable array access folding by default for cases where access is performed under condition

pessimistic approach for now: even if the condition consists of array access only; do not depend on the order of found references

GitOrigin-RevId: 0d111aad6b69729404183520d1f4d01cba583a1c

java/java-analysis-impl/src/com/intellij/refactoring/extractMethod/ParametersFolder.java

@@ -126,12 +126,8 @@ class ParametersFolder {
     PsiExpression mostRanked = null;
     for (int i = mentionedInExpressions.size() - 1; i >= 0; i--) {
       PsiExpression expression = mentionedInExpressions.get(i);
-      boolean arrayAccess = expression instanceof PsiArrayAccessExpression && !isConditional(expression, scope);
-      if (arrayAccess) {
-        myFoldingSelectedByDefault = true;
-      }
       final int r = findUsedVariables(data, inputVariables, expression).size();
-      if (currentRank < r || arrayAccess && currentRank == r) {
+      if (currentRank < r || expression instanceof PsiArrayAccessExpression && myFoldingSelectedByDefault && currentRank == r) {
         currentRank = r;
         mostRanked = expression;
       }
@@ -154,27 +150,6 @@ class ParametersFolder {
     return mostRanked != null;
   }
 
-  private static boolean isConditional(@NotNull PsiElement expr, @NotNull LocalSearchScope scope) {
-    while (true) {
-      final PsiElement parent = expr.getParent();
-      if (parent != null && scope.containsRange(parent.getContainingFile(), parent.getTextRange())) {
-        if (parent instanceof PsiIfStatement) {
-          if (((PsiIfStatement)parent).getCondition() != expr) return true;
-        }
-        else if (parent instanceof PsiConditionalExpression) {
-          if (((PsiConditionalExpression)parent).getCondition() != expr) return true;
-        }
-        else if (parent instanceof PsiSwitchStatement) {
-          if (((PsiSwitchStatement)parent).getExpression() != expr) return true;
-        }
-      }
-      else {
-        return false;
-      }
-      expr = parent;
-    }
-  }
-
   private static void setUniqueName(@NotNull VariableData data, @NotNull UniqueNameGenerator nameGenerator,
                                     @NotNull PsiExpression expr, @NotNull JavaCodeStyleManager codeStyleManager) {
     String name = data.name;
@@ -218,6 +193,7 @@ class ParametersFolder {
     final PsiElement[] scopeElements = scope.getScope();
 
     List<PsiExpression> expressions = null;
+    Boolean arrayAccess = null;
     for (PsiReference reference : ReferencesSearch.search(var, scope)) {
       PsiElement expression = reference.getElement();
       if (expressions == null) {
@@ -246,21 +222,49 @@ class ParametersFolder {
           if (!isMethodNameExpression(expression)) {
             expressions.add((PsiExpression)expression);
           }
+          if (expression instanceof PsiArrayAccessExpression && (arrayAccess == null || arrayAccess)) {
+            arrayAccess = isSafeToFoldArrayAccess(scope, expression);
+          }
           expression = PsiTreeUtil.getParentOfType(expression, PsiExpression.class);
         }
       }
       else {
         for (Iterator<PsiExpression> iterator = expressions.iterator(); iterator.hasNext();) {
-          if (findEquivalent(iterator.next(), expression) == null) {
+          PsiExpression equivalent = findEquivalent(iterator.next(), expression);
+          if (equivalent == null) {
             iterator.remove();
           }
+          else if (equivalent instanceof PsiArrayAccessExpression && (arrayAccess == null || arrayAccess)) {
+            arrayAccess = isSafeToFoldArrayAccess(scope, equivalent);
+          }
         }
       }
     }
+    if (arrayAccess != null && arrayAccess) {
+      myFoldingSelectedByDefault = true;
+    }
     myMentionedInExpressions.put(var, expressions);
     return expressions;
   }
 
+  private static boolean isSafeToFoldArrayAccess(@NotNull LocalSearchScope scope,
+                                                 PsiElement expression) {
+    while (true) {
+      final PsiElement parent = expression.getParent();
+      if (parent != null && scope.containsRange(parent.getContainingFile(), parent.getTextRange())) {
+        if (parent instanceof PsiIfStatement ||
+            parent instanceof PsiConditionalExpression ||
+            parent instanceof PsiSwitchStatement) {
+          return false;
+        }
+      }
+      else {
+        return true;
+      }
+      expression = parent;
+    }
+  }
+
   private static boolean isAccessedForWriting(@NotNull PsiExpression expression) {
     final PsiExpression[] exprWithWriteAccessInside = new PsiExpression[1];
     expression.accept(new JavaRecursiveElementWalkingVisitor() {

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

@@ -2,14 +2,14 @@ class Foo {
     boolean bar(String[][] a) {
         for (int i = 0; i < a.length; i++)
             for (int j = 0; i < a[i].length; j++) {
-                if (newMethod(a[i][j], i)) return true;
+                if (newMethod(a, i, j)) return true;
 
             }
         return false;
     }
 
-    private boolean newMethod(String s, int i) {
+    private boolean newMethod(String[][] a, int i, int j) {
-        if (s.length() > 3 && i % 3 == 0)
+        if (a[i][j].length() > 3 && i % 3 == 0)
             return true;
         return false;
     }

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

@@ -1,13 +1,13 @@
 class Foo {
     boolean bar(String[] a) {
         for (int i = 0; i < a.length; i++) {
-            if (newMethod(a[i], i)) return true;
+            if (newMethod(a, i)) return true;
         }
         return false;
     }
 
-    private boolean newMethod(String s, int i) {
+    private boolean newMethod(String[] a, int i) {
-        if (s.length() > 3 && i % 3 == 0)
+        if (a[i].length() > 3 && i % 3 == 0)
             return true;
         return false;
     }

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

@@ -2,13 +2,13 @@ class Main {
     private String [] args;
 
     void foo(Main m, int i) {
-        newMethod(m.args[i]);
+        newMethod(m, i);
 
     }
 
-    private void newMethod(String arg) {
+    private void newMethod(Main m, int i) {
-        if (arg != null) {
+        if (m.args[i] != null) {
-            System.out.println(arg);
+            System.out.println(m.args[i]);
         }
     }
 }

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

@@ -3,22 +3,22 @@ import org.jetbrains.annotations.Nullable;
 class DeclaredOutputVariable {
     void foo(String[] a, int j) {
 
-        String s = newMethod(a[j], 1, "X");
+        String s = newMethod(a, j, 1, "X");
         if (s == null) return;
 
         System.out.println(s.length());
     }
 
     @Nullable
-    private String newMethod(String s1, int i, String x) {
+    private String newMethod(String[] a, int j, int i, String x) {
-        if (s1 == null) return null;
+        if (a[j] == null) return null;
-        String s = s1;
+        String s = a[j];
         System.out.println(s.charAt(i) + x);
         return s;
     }
 
     void bar(String[] a, int k) {
-        String s = newMethod(a[k], 2, "Y");
+        String s = newMethod(a, k, 2, "Y");
         if (s == null) return;
         System.out.println(s.length());
     }