From d8747dfea657e71777de51a35e3a164912b3fac5 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 19 Mar 2014 21:47:58 +0100 Subject: [PATCH] stream: do not suggest collect when filter depends on collection (IDEA-122410) --- .../StreamApiMigrationInspection.java | 52 ++++++++++++++++++- .../afterCollectionDependencyInFilter.java | 13 +++++ .../afterCollectionDependencyInFilter1.java | 11 ++++ .../afterCollectionDependencyInFilter2.java | 13 +++++ .../afterCollectionDependencyInFilter3.java | 13 +++++ .../afterCollectionDependencyInFilter4.java | 17 ++++++ .../beforeCollectionDependencyInFilter.java | 15 ++++++ .../beforeCollectionDependencyInFilter1.java | 13 +++++ .../beforeCollectionDependencyInFilter2.java | 15 ++++++ .../beforeCollectionDependencyInFilter3.java | 15 ++++++ .../beforeCollectionDependencyInFilter4.java | 19 +++++++ 11 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter3.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter4.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter3.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter4.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java index b06ba2a5358b..3e48c52553ac 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java +++ b/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 diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter.java new file mode 100644 index 000000000000..868657074157 --- /dev/null +++ b/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 testTags) { + final List resultJava7 = new ArrayList<>(testTags.size()); + testTags.stream().filter(tag -> !resultJava7.contains(tag.trim())).forEach(tag -> { + resultJava7.add(tag.trim()); + }); + + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter1.java new file mode 100644 index 000000000000..c081b1bab5e6 --- /dev/null +++ b/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 { + void main() { + this.stream().filter(tag -> !contains(tag.trim())).forEach(tag -> { + add(tag.trim()); + }); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter2.java new file mode 100644 index 000000000000..c0a135ff5b76 --- /dev/null +++ b/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 { + void main() { + this.stream().filter(tag -> !foo(this)).forEach(tag -> { + add(tag.trim()); + }); + } + + static boolean foo(List a){ return false;} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter3.java new file mode 100644 index 000000000000..2813cdf0957c --- /dev/null +++ b/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 { + void main() { + this.stream().filter(tag -> !super.contains(tag)).forEach(tag -> { + add(tag.trim()); + }); + } + + static boolean foo(List a){ return false;} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter4.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterCollectionDependencyInFilter4.java new file mode 100644 index 000000000000..8488d78593f5 --- /dev/null +++ b/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 testTags) { + final List resultJava7 = new ArrayList<>(testTags.size()); + testTags.stream().filter(tag -> !foo(resultJava7)).forEach(tag -> { + resultJava7.add(tag.trim()); + }); + + } + + static boolean foo(List l) { + return false; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter.java new file mode 100644 index 000000000000..b0844f2e080f --- /dev/null +++ b/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 testTags) { + final List resultJava7 = new ArrayList<>(testTags.size()); + for (final String tag : testTags) { + if (!resultJava7.contains(tag.trim())) { + resultJava7.add(tag.trim()); + } + } + + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter1.java new file mode 100644 index 000000000000..0cfc956dcd61 --- /dev/null +++ b/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 { + void main() { + for (final String tag : this) { + if (!contains(tag.trim())) { + add(tag.trim()); + } + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter2.java new file mode 100644 index 000000000000..aae55cf5fe36 --- /dev/null +++ b/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 { + void main() { + for (final String tag : this) { + if (!foo(this)) { + add(tag.trim()); + } + } + } + + static boolean foo(List a){ return false;} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter3.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter3.java new file mode 100644 index 000000000000..16f612ac56d4 --- /dev/null +++ b/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 { + void main() { + for (final String tag : this) { + if (!super.contains(tag)) { + add(tag.trim()); + } + } + } + + static boolean foo(List a){ return false;} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter4.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeCollectionDependencyInFilter4.java new file mode 100644 index 000000000000..92831a2936cf --- /dev/null +++ b/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 testTags) { + final List resultJava7 = new ArrayList<>(testTags.size()); + for (final String tag : testTags) { + if (!foo(resultJava7)) { + resultJava7.add(tag.trim()); + } + } + + } + + static boolean foo(List l) { + return false; + } +}