From 4a2a11c85fe9aa5425276b1f4d563ce7156a2aef Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 23 Dec 2016 12:13:09 +0700 Subject: [PATCH] IDEA-163833 Replace Collectors.toCollection() for custom collections: check implementation --- ...SimplifyStreamApiCallChainsInspection.java | 53 ++++++++++++++++--- ...terStreamToCollectionMyTypeAddAllCtor.java | 18 +++++++ ...oreStreamToCollectionMyTypeAddAllCtor.java | 18 +++++++ ...foreStreamToCollectionMyTypeEmptyCtor.java | 17 ++++++ 4 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterStreamToCollectionMyTypeAddAllCtor.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeAddAllCtor.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeEmptyCtor.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index f012f3eb8b5a..5adddf91a57e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -17,6 +17,7 @@ package com.intellij.codeInspection; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Key; import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; @@ -34,6 +35,7 @@ import org.jetbrains.annotations.Nullable; import java.text.MessageFormat; import java.util.Arrays; +import java.util.Objects; import java.util.stream.Stream; /** @@ -422,18 +424,54 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns return null; } + @Contract("null -> false") private static boolean isCollectionConstructor(PsiMethod ctor) { - if(!ctor.getModifierList().hasExplicitModifier(PsiModifier.PUBLIC)) return false; + if (ctor == null || !ctor.getModifierList().hasExplicitModifier(PsiModifier.PUBLIC)) return false; PsiParameterList list = ctor.getParameterList(); - if(list.getParametersCount() != 1) return false; + if (list.getParametersCount() != 1) return false; PsiParameter parameter = list.getParameters()[0]; PsiTypeElement typeElement = parameter.getTypeElement(); - if(typeElement == null) return false; - PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(typeElement.getType()); - if(aClass == null) return false; - return CommonClassNames.JAVA_UTIL_COLLECTION.equals(aClass.getQualifiedName()); + if (typeElement == null) return false; + PsiType type = typeElement.getType(); + PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(type); + if(aClass == null || !CommonClassNames.JAVA_UTIL_COLLECTION.equals(aClass.getQualifiedName())) return false; + PsiClass curClass = ctor.getContainingClass(); + LOG.assertTrue(curClass != null); + // Do not perform deep check for standard Collections + if(Objects.requireNonNull(curClass.getQualifiedName()).startsWith("java.util.")) return true; + PsiSubstitutor substitutor = TypeConversionUtil.getSuperClassSubstitutor(aClass, curClass, PsiSubstitutor.EMPTY); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(ctor.getProject()); + PsiClassType collectionType = factory.createType(aClass, substitutor); + if (!(type.isAssignableFrom(collectionType))) return false; + // Check that body either contains addAll(arg) or super(arg) + return !PsiTreeUtil.processElements(ctor.getBody(), e -> { + if(e instanceof PsiMethodCallExpression) { + PsiMethodCallExpression call = (PsiMethodCallExpression)e; + PsiExpression[] args = call.getArgumentList().getExpressions(); + if(args.length == 1 && ExpressionUtils.isReferenceTo(args[0], parameter)) { + PsiReferenceExpression methodExpression = call.getMethodExpression(); + if("super".equals(methodExpression.getReferenceName())) { + return !isCollectionConstructor(call.resolveMethod()); + } + if("addAll".equals(methodExpression.getReferenceName())) { + PsiExpression qualifier = methodExpression.getQualifierExpression(); + if(qualifier == null || qualifier instanceof PsiThisExpression) { + return false; + } + } + } + } + return true; + }); } + private static final Key> HAS_COLLECTION_CONSTRUCTOR = Key.create("HasCollectionConstructor"); + private static final ParameterizedCachedValueProvider HAS_COLLECTION_CONSTRUCTOR_PROVIDER = psiClass -> { + boolean hasCollectionConstructor = + Stream.of(psiClass.getConstructors()).anyMatch(SimplifyStreamApiCallChainsInspection::isCollectionConstructor); + return CachedValueProvider.Result.create(hasCollectionConstructor, psiClass); + }; + @Nullable private static String collectorToCollection(PsiMethodCallExpression call) { PsiMethod method = call.resolveMethod(); @@ -454,7 +492,8 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns if(ctor.getParameterList().getParametersCount() == 0) { PsiClass aClass = ctor.getContainingClass(); if (aClass != null && - Stream.of(aClass.getConstructors()).anyMatch(SimplifyStreamApiCallChainsInspection::isCollectionConstructor)) { + CachedValuesManager.getManager(aClass.getProject()) + .getParameterizedCachedValue(aClass, HAS_COLLECTION_CONSTRUCTOR, HAS_COLLECTION_CONSTRUCTOR_PROVIDER, false, aClass)) { return aClass.getQualifiedName(); } } diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToCollectionMyTypeAddAllCtor.java b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToCollectionMyTypeAddAllCtor.java new file mode 100644 index 000000000000..610c7c27cee6 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToCollectionMyTypeAddAllCtor.java @@ -0,0 +1,18 @@ +// "Replace with 'Test.MyType' constructor" "true" + +import java.util.*; +import java.util.stream.*; + +class Test { + static class MyType extends ArrayList { + public MyType() {} + + public MyType(Collection coll) { + this.addAll(coll); + } + } + + public static void test(List s) { + new MyType(s).contains("abc"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeAddAllCtor.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeAddAllCtor.java new file mode 100644 index 000000000000..8cf3c518c408 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeAddAllCtor.java @@ -0,0 +1,18 @@ +// "Replace with 'Test.MyType' constructor" "true" + +import java.util.*; +import java.util.stream.*; + +class Test { + static class MyType extends ArrayList { + public MyType() {} + + public MyType(Collection coll) { + this.addAll(coll); + } + } + + public static void test(List s) { + s.stream().collect(Collectors.toCollection(MyType::new)).contains("abc"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeEmptyCtor.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeEmptyCtor.java new file mode 100644 index 000000000000..cd00f309e4fb --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToCollectionMyTypeEmptyCtor.java @@ -0,0 +1,17 @@ +// "Replace with 'Test.MyType' constructor" "false" + +import java.util.*; +import java.util.stream.*; + +class Test { + static class MyType extends ArrayList { + public MyType() {} + + public MyType(Collection coll) { + } + } + + public static void test(List s) { + s.stream().collect(Collectors.toCollection(MyType::new)).contains("abc"); + } +} \ No newline at end of file