diff --git a/java/java-impl/src/com/intellij/codeInspection/CollectionAddAllCanBeReplacedWithConstructorInspection.java b/java/java-impl/src/com/intellij/codeInspection/CollectionAddAllCanBeReplacedWithConstructorInspection.java index 31af775d703d..c0f51c9ade03 100644 --- a/java/java-impl/src/com/intellij/codeInspection/CollectionAddAllCanBeReplacedWithConstructorInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/CollectionAddAllCanBeReplacedWithConstructorInspection.java @@ -20,10 +20,10 @@ import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.daemon.QuickFixBundle; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.InvalidDataException; -import com.intellij.openapi.util.Pair; -import com.intellij.openapi.util.WriteExternalException; +import com.intellij.openapi.util.*; import com.intellij.psi.*; +import com.intellij.psi.search.LocalSearchScope; +import com.intellij.psi.search.SearchScope; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.NullableFunction; @@ -85,6 +85,10 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base if (!(qualifierExpression instanceof PsiReferenceExpression)) { return; } + final PsiElement parent = expression.getParent(); + if (!(parent instanceof PsiExpressionStatement)) { + return; + } final PsiElement resolvedReference = ((PsiReferenceExpression)qualifierExpression).resolve(); if (!(resolvedReference instanceof PsiLocalVariable)) { return; @@ -112,7 +116,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base } else { return; } - if (!checkUsages(variable, expression, assignmentExpression)) { + if (!isAddAllReplaceable(expression, assignmentExpression) || !checkUsages(variable, expression, assignmentExpression)) { return; } final PsiMethod method = expression.resolveMethod(); @@ -183,6 +187,28 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base return Pair.create(true, null); } + private boolean isAddAllReplaceable(final PsiExpression addAllExpression, PsiNewExpression newExpression) { + final boolean[] isReplaceable = new boolean[]{true}; + final PsiFile newExpressionContainingFile = newExpression.getContainingFile(); + final TextRange newExpressionTextRange = newExpression.getTextRange(); + + addAllExpression.accept(new JavaRecursiveElementVisitor() { + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + final PsiElement resolved = expression.resolve(); + if (resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter) { + PsiVariable variable = (PsiVariable) resolved; + final LocalSearchScope useScope = (LocalSearchScope)variable.getUseScope(); + if (!useScope.containsRange(newExpressionContainingFile, newExpressionTextRange)) { + isReplaceable[0] = false; + } + } + } + }); + + return isReplaceable[0]; + } + private static class ReplaceAddAllWithConstructorFix implements LocalQuickFix { private final SmartPsiElementPointer myMethodCallExpression; private final SmartPsiElementPointer myAssignmentExpression; diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/afterUseParameter.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/afterUseParameter.java new file mode 100644 index 000000000000..c81c64a5346e --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/afterUseParameter.java @@ -0,0 +1,13 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "true" +import java.lang.String; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.HashSet; + +class C { + void m(String s) { + final List strings; + strings = new ArrayList(Arrays.asList(s, ",")); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown1.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown1.java new file mode 100644 index 000000000000..88e6dd53b40c --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown1.java @@ -0,0 +1,14 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "false" +import java.lang.String; +import java.util.ArrayList; +import java.util.List; +import java.util.HashSet; +import java.util.Arrays; + + +class C { + void m() { + final List list = new ArrayList<>(); + Arrays.asList(1, 2, 3).forEach(x -> list.addAll(Arrays.asList(String.valueOf(x), ","))); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown2.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown2.java new file mode 100644 index 000000000000..59412f760078 --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown2.java @@ -0,0 +1,14 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "false" +import java.lang.String; +import java.util.ArrayList; +import java.util.List; +import java.util.HashSet; +import java.util.Arrays; + + +class C { + void m() { + final List list = new ArrayList<>(); + Arrays.asList(1, 2, 3).forEach(x -> list.addAll(Arrays.asList(".", ","))); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown3.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown3.java new file mode 100644 index 000000000000..ea46a5e4ac28 --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown3.java @@ -0,0 +1,16 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "false" +import java.lang.String; +import java.util.ArrayList; +import java.util.List; +import java.util.HashSet; +import java.util.Arrays; + + +class C { + void m() { + final List list = new ArrayList<>(); + if (list.addAll(Arrays.asList(".", ","))) { + System.out.println("foo"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown4.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown4.java new file mode 100644 index 000000000000..fdc4f123bffb --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeNotShown4.java @@ -0,0 +1,14 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "false" +import java.lang.String; +import java.util.ArrayList; +import java.util.List; +import java.util.HashSet; +import java.util.Arrays; + + +class C { + void m() { + final List list = new ArrayList<>(); + Boolean.hashCode(list.addAll(Arrays.asList(".", ","))); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeUseParameter.java b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeUseParameter.java new file mode 100644 index 000000000000..0cdace362abd --- /dev/null +++ b/java/java-tests/testData/inspection/collectionAddAllCanBeReplacedWithConstructor/beforeUseParameter.java @@ -0,0 +1,14 @@ +// "Replace 'addAll/putAll' method with parametrized constructor call" "true" +import java.lang.String; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.HashSet; + +class C { + void m(String s) { + final List strings; + strings = new ArrayList(); + strings.addAll(Arrays.asList(s, ",")); + } +} \ No newline at end of file