Collections.addAll inspection checks context for possibility of addAll() movement deeply IDEA-147770

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<PsiMethodCallExpression> myMethodCallExpression;
     private final SmartPsiElementPointer<PsiNewExpression> myAssignmentExpression;

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<String> strings;
+    strings = new ArrayList<String>(Arrays.asList(s, ","));
+  }
+}

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<String> list = new ArrayList<>();
+    Arrays.asList(1, 2, 3).forEach(x -> list.addA<caret>ll(Arrays.asList(String.valueOf(x), ",")));
+  }
+}

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<String> list = new ArrayList<>();
+    Arrays.asList(1, 2, 3).forEach(x -> list.ad<caret>dAll(Arrays.asList(".", ",")));
+  }
+}

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<String> list = new ArrayList<>();
+    if (list.addA<caret>ll(Arrays.asList(".", ","))) {
+      System.out.println("foo");
+    }
+  }
+}

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<String> list = new ArrayList<>();
+    Boolean.hashCode(list.add<caret>All(Arrays.asList(".", ",")));
+  }
+}

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<String> strings;
+    strings = new ArrayList<String>();
+    string<caret>s.addAll(Arrays.asList(s, ","));
+  }
+}