From 9c78db8af09c69c7aba7268c650449dd423422e2 Mon Sep 17 00:00:00 2001
From: Tagir Valeev <Tagir.Valeev@jetbrains.com>
Date: Fri, 16 Dec 2016 11:12:15 +0700
Subject: [PATCH] FunctionHelper: fix renaming in lambdas if autogenerated name
 conflicts with the existing one

---
 .../streamToLoop/FunctionHelper.java          | 34 +++++++++++--------
 .../streamToLoop/afterCollectExtends.java     | 24 +++++++++++++
 .../streamToLoop/beforeCollectExtends.java    | 17 ++++++++++
 3 files changed, 60 insertions(+), 15 deletions(-)
 create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectExtends.java
 create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeCollectExtends.java

diff --git a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java
index aa30c6ac21d2..23c0968d4e0f 100644
--- a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java
+++ b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java
@@ -34,7 +34,10 @@ import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
 import java.util.function.Consumer;
 
 /**
@@ -183,6 +186,14 @@ abstract class FunctionHelper {
     };
   }
 
+  static boolean hasVarReference(PsiExpression expression, String name, StreamToLoopReplacementContext context) {
+    PsiLambdaExpression lambda = (PsiLambdaExpression)context.createExpression(name+"->"+expression.getText());
+    PsiParameter var = lambda.getParameterList().getParameters()[0];
+    PsiElement body = lambda.getBody();
+    LOG.assertTrue(body != null);
+    return ReferencesSearch.search(var, new LocalSearchScope(body)).findFirst() != null;
+  }
+
   /**
    * Replaces all the references to the variable {@code name} in given expression with {@code replacement}.
    *
@@ -480,22 +491,15 @@ abstract class FunctionHelper {
     }
 
     void rename(String oldName, String newName, StreamToLoopReplacementContext context) {
-      OptionalLong idx = StreamEx.of(myParameters).indexOf(newName);
-      if(idx.isPresent()) {
+      int idx = ArrayUtil.indexOf(myParameters, newName);
+      if(idx >= 0) {
+        // If new name collides with existing parameter, rename it
         for(int i = 1;; i++) {
           String paramName = newName+'$'+i;
-          if (!paramName.equals(oldName) &&
-              !StreamEx.of(myParameters).has(paramName)) {
-            try {
-              myBody = replaceVarReference(myBody, newName, paramName, context);
-              myParameters[(int)idx.getAsLong()] = paramName;
-              break;
-            }
-            catch(IllegalStateException ise) {
-              // something is really wrong if we already have references to all newName$1, newName$2, ... newName$50
-              // or probably IllegalStateException was thrown by something else: at least we don't stuck in endless loop
-              if(i > 50) throw ise;
-            }
+          if (!paramName.equals(oldName) && !StreamEx.of(myParameters).has(paramName) && !hasVarReference(myBody, paramName, context)) {
+            myBody = replaceVarReference(myBody, newName, paramName, context);
+            myParameters[idx] = paramName;
+            break;
           }
         }
       }
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectExtends.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectExtends.java
new file mode 100644
index 000000000000..197358b2bc1e
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectExtends.java
@@ -0,0 +1,24 @@
+// "Replace Stream API chain with loop" "true"
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+public class Main {
+  public List<? extends CharSequence> getList() {
+    return Collections.emptyList();
+  }
+
+  private void collect() {
+      List<CharSequence> list = new ArrayList<>();
+      for (CharSequence charSequence : getList()) {
+          if (Objects.nonNull(charSequence)) {
+              list.add(charSequence);
+          }
+      }
+      List<? extends CharSequence> res = list;
+    System.out.println(res);
+  }
+}
diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeCollectExtends.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeCollectExtends.java
new file mode 100644
index 000000000000..14485bc9cf0d
--- /dev/null
+++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeCollectExtends.java
@@ -0,0 +1,17 @@
+// "Replace Stream API chain with loop" "true"
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+public class Main {
+  public List<? extends CharSequence> getList() {
+    return Collections.emptyList();
+  }
+
+  private void collect() {
+    List<? extends CharSequence> res = getList().stream().filter(Objects::nonNull).col<caret>lect(Collectors.toList());
+    System.out.println(res);
+  }
+}