stream: do not suggest collect when filter depends on collection (IDEA-122410)

java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java

@@ -129,6 +129,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
   }
 
   private static boolean isCollectCall(PsiStatement body) {
+    final PsiIfStatement ifStatement = extractIfStatement(body);
     final PsiMethodCallExpression methodCallExpression = extractAddCall(body);
     if (methodCallExpression != null) {
       final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression();
@@ -146,6 +147,11 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
       if (qualifierClass != null && 
           InheritanceUtil.isInheritor(qualifierClass, false, CommonClassNames.JAVA_UTIL_COLLECTION)) {
 
+        if (ifStatement != null) {
+          final PsiExpression condition = ifStatement.getCondition();
+          if (condition != null && isConditionDependsOnUpdatedCollections(condition, qualifierExpression)) return false;
+        }
+
         final PsiElement resolve = methodExpression.resolve();
         if (resolve instanceof PsiMethod &&
             "add".equals(((PsiMethod)resolve).getName()) &&
@@ -163,7 +169,51 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
     }
     return false;
   }
-  
+
+  private static boolean isConditionDependsOnUpdatedCollections(PsiExpression condition,
+                                                                PsiExpression qualifierExpression) {
+    final PsiElement collection = qualifierExpression != null
+                                  ? ((PsiReferenceExpression)qualifierExpression).resolve()
+                                  : null;
+    final boolean[] dependsOnCollection = {false};
+    condition.accept(new JavaRecursiveElementWalkingVisitor() {
+      @Override
+      public void visitReferenceExpression(PsiReferenceExpression expression) {
+        super.visitReferenceExpression(expression);
+        if (collection != null && collection == expression.resolve()) {
+          dependsOnCollection[0] = true;
+        }
+      }
+
+      @Override
+      public void visitMethodCallExpression(PsiMethodCallExpression expression) {
+        super.visitMethodCallExpression(expression);
+        final PsiExpression callQualifier = expression.getMethodExpression().getQualifierExpression();
+        if (collection == callQualifier) {
+          dependsOnCollection[0] = true;
+        }
+
+        if (collection == null && (callQualifier instanceof PsiThisExpression && ((PsiThisExpression)callQualifier).getQualifier() == null || 
+                                   callQualifier instanceof PsiSuperExpression && ((PsiSuperExpression)callQualifier).getQualifier() == null)) {
+          dependsOnCollection[0] = true;
+        }
+      }
+
+      @Override
+      public void visitThisExpression(PsiThisExpression expression) {
+        super.visitThisExpression(expression);
+        if (collection == null && expression.getQualifier() == null && expression.getParent() instanceof PsiExpressionList) {
+          dependsOnCollection[0] = true;
+        }
+      }
+
+      @Override
+      public void visitClass(PsiClass aClass) {}
+    });
+
+    return dependsOnCollection[0];
+  }
+
   private static boolean isTrivial(PsiStatement body, PsiParameter parameter, PsiType iteratedValueType) {
     final PsiIfStatement ifStatement = extractIfStatement(body);
     //stream

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter.java

@@ -0,0 +1,13 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample {
+  public static void main(List<String> testTags) {
+    final List<String> resultJava7 = new ArrayList<>(testTags.size());
+      testTags.stream().filter(tag -> !resultJava7.contains(tag.trim())).forEach(tag -> {
+          resultJava7.add(tag.trim());
+      });
+
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter1.java

@@ -0,0 +1,11 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+abstract class Sample implements List<String> {
+  void main() {
+      this.stream().filter(tag -> !contains(tag.trim())).forEach(tag -> {
+          add(tag.trim());
+      });
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter2.java

@@ -0,0 +1,13 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+abstract class Sample implements List<String> {
+  void main() {
+      this.stream().filter(tag -> !foo(this)).forEach(tag -> {
+          add(tag.trim());
+      });
+  }
+  
+  static boolean foo(List<String> a){ return false;}
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter3.java

@@ -0,0 +1,13 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample extends ArrayList<String> {
+  void main() {
+      this.stream().filter(tag -> !super.contains(tag)).forEach(tag -> {
+          add(tag.trim());
+      });
+  }
+  
+  static boolean foo(List<String> a){ return false;}
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter4.java

@@ -0,0 +1,17 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample {
+  public static void main(List<String> testTags) {
+    final List<String> resultJava7 = new ArrayList<>(testTags.size());
+      testTags.stream().filter(tag -> !foo(resultJava7)).forEach(tag -> {
+          resultJava7.add(tag.trim());
+      });
+
+  }
+  
+  static boolean foo(List<String> l) {
+    return false;
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter.java

@@ -0,0 +1,15 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample {
+  public static void main(List<String> testTags) {
+    final List<String> resultJava7 = new ArrayList<>(testTags.size());
+    for (final String tag : tes<caret>tTags) {
+      if (!resultJava7.contains(tag.trim())) {
+        resultJava7.add(tag.trim());
+      }
+    }
+
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter1.java

@@ -0,0 +1,13 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+abstract class Sample implements List<String> {
+  void main() {
+    for (final String tag : t<caret>his) {
+      if (!contains(tag.trim())) {
+        add(tag.trim());
+      }
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter2.java

@@ -0,0 +1,15 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+abstract class Sample implements List<String> {
+  void main() {
+    for (final String tag : t<caret>his) {
+      if (!foo(this)) {
+        add(tag.trim());
+      }
+    }
+  }
+  
+  static boolean foo(List<String> a){ return false;}
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter3.java

@@ -0,0 +1,15 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample extends ArrayList<String> {
+  void main() {
+    for (final String tag : t<caret>his) {
+      if (!super.contains(tag)) {
+        add(tag.trim());
+      }
+    }
+  }
+  
+  static boolean foo(List<String> a){ return false;}
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter4.java

@@ -0,0 +1,19 @@
+// "Replace with forEach" "true"
+import java.util.ArrayList;
+import java.util.List;
+
+class Sample {
+  public static void main(List<String> testTags) {
+    final List<String> resultJava7 = new ArrayList<>(testTags.size());
+    for (final String tag : tes<caret>tTags) {
+      if (!foo(resultJava7)) {
+        resultJava7.add(tag.trim());
+      }
+    }
+
+  }
+  
+  static boolean foo(List<String> l) {
+    return false;
+  }
+}