IDEA-185812 Allow to force explicit toCollection(ArrayList::new) instead of toList() in "Subsequent steps can be fused into Stream API"

java/java-impl/src/com/intellij/codeInspection/streamMigration/CollectMigration.java

@@ -1,18 +1,4 @@
-/*
- * Copyright 2000-2017 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.codeInspection.streamMigration;
 
 import com.intellij.codeInsight.intention.impl.StreamRefactoringUtil;
@@ -82,7 +68,7 @@ class CollectMigration extends BaseStreamApiMigration {
     CollectTerminal terminal = extractCollectTerminal(tb, null);
     if (terminal == null) return null;
     CommentTracker ct = new CommentTracker();
-    String stream = tb.generate(ct) + terminal.generateIntermediate(ct) + terminal.generateTerminal(ct);
+    String stream = tb.generate(ct) + terminal.generateIntermediate(ct) + terminal.generateTerminal(ct, false);
     PsiElement toReplace = terminal.getElementToReplace();
     PsiElement result;
     if (toReplace != null) {
@@ -214,7 +200,13 @@ class CollectMigration extends BaseStreamApiMigration {
       return INTERMEDIATE_STEPS.get(aClass.getQualifiedName());
     }
 
-    abstract String generateTerminal(CommentTracker ct);
+    /**
+     * Generate terminal stream call starting with '.' (e.g. {@code ".collect(java.util.stream.Collectors.toList())"}
+     * @param ct comment tracker to use
+     * @param strictMode if true, toList/toSet collectors will not be used to replace ArrayList/HashSet
+     * @return generated call
+     */
+    abstract String generateTerminal(CommentTracker ct, boolean strictMode);
 
     StreamEx<PsiElement> usedElements() {
       return StreamEx.ofNullable(myLoop);
@@ -278,13 +270,13 @@ class CollectMigration extends BaseStreamApiMigration {
       return StreamRefactoringUtil.generateMapOperation(myElement, addedType, ct.markUnchanged(mapping));
     }
 
-    public String generateCollector(CommentTracker ct) {
-      return getCollectionCollector(ct, myInitializer, myTargetType);
+    public String generateCollector(CommentTracker ct, boolean strictMode) {
+      return getCollectionCollector(ct, myInitializer, myTargetType, strictMode);
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
-      return ".collect(" + generateCollector(ct) + ")";
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
+      return ".collect(" + generateCollector(ct, strictMode) + ")";
     }
 
     @Nullable
@@ -309,18 +301,18 @@ class CollectMigration extends BaseStreamApiMigration {
   }
 
   @NotNull
-  private static String getCollectionCollector(CommentTracker ct, PsiExpression initializer, PsiType type) {
+  private static String getCollectionCollector(CommentTracker ct, PsiExpression initializer, PsiType type, boolean strictMode) {
     String collector;
     PsiType initializerType = initializer.getType();
     PsiClassType rawType = initializerType instanceof PsiClassType ? ((PsiClassType)initializerType).rawType() : null;
     PsiClassType rawVarType = type instanceof PsiClassType ? ((PsiClassType)type).rawType() : null;
-    if (rawType != null && rawVarType != null &&
+    if (!strictMode && rawType != null && rawVarType != null &&
         rawType.equalsToText(CommonClassNames.JAVA_UTIL_ARRAY_LIST) &&
         (rawVarType.equalsToText(CommonClassNames.JAVA_UTIL_LIST) || rawVarType.equalsToText(CommonClassNames.JAVA_UTIL_COLLECTION)) &&
         !ConstructionUtils.isCustomizedEmptyCollectionInitializer(initializer)) {
       collector = "toList()";
     }
-    else if (rawType != null && rawVarType != null &&
+    else if (!strictMode && rawType != null && rawVarType != null &&
              rawType.equalsToText(CommonClassNames.JAVA_UTIL_HASH_SET) &&
              (rawVarType.equalsToText(CommonClassNames.JAVA_UTIL_SET) || rawVarType.equalsToText(CommonClassNames.JAVA_UTIL_COLLECTION)) &&
              !ConstructionUtils.isCustomizedEmptyCollectionInitializer(initializer)) {
@@ -416,8 +408,8 @@ class CollectMigration extends BaseStreamApiMigration {
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
-      String downstreamCollector = myDownstream.generateCollector(ct);
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
+      String downstreamCollector = myDownstream.generateCollector(ct, strictMode);
       PsiVariable elementVariable = myDownstream.getElementVariable();
       if (!ExpressionUtils.isReferenceTo(myDownstream.getMapping(), myDownstream.getElementVariable())) {
         downstreamCollector = CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS + ".mapping(" +
@@ -592,7 +584,7 @@ class CollectMigration extends BaseStreamApiMigration {
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
       PsiExpression[] args = myMapUpdateCall.getArgumentList().getExpressions();
       LOG.assertTrue(args.length >= 2);
       String methodName = myMapUpdateCall.getMethodExpression().getReferenceName();
@@ -666,8 +658,8 @@ class CollectMigration extends BaseStreamApiMigration {
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
-      return myDownstream.generateTerminal(ct);
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
+      return myDownstream.generateTerminal(ct, strictMode);
     }
 
     @Override
@@ -787,7 +779,7 @@ class CollectMigration extends BaseStreamApiMigration {
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
       return ".toArray(" + mySupplier + ")";
     }
 
@@ -859,8 +851,8 @@ class CollectMigration extends BaseStreamApiMigration {
     }
 
     @Override
-    public String generateTerminal(CommentTracker ct) {
-      return ".collect(" + getCollectionCollector(ct, ct.markUnchanged(myCreateExpression), myResultType) + ")";
+    public String generateTerminal(CommentTracker ct, boolean strictMode) {
+      return ".collect(" + getCollectionCollector(ct, ct.markUnchanged(myCreateExpression), myResultType, strictMode) + ")";
     }
 
     @Override

java/java-impl/src/com/intellij/codeInspection/streamMigration/FuseStreamOperationsInspection.java

@@ -1,8 +1,9 @@
-// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.codeInspection.streamMigration;
 
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.streamMigration.CollectMigration.CollectTerminal;
+import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
@@ -17,6 +18,7 @@ import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import javax.swing.*;
 import java.util.Objects;
 import java.util.function.Function;
 
@@ -30,6 +32,9 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
       CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toList", "toSet").parameterCount(0),
       CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toCollection").parameterCount(1));
 
+  @SuppressWarnings("PublicField")
+  public boolean myStrictMode = false;
+
   private static class StreamCollectChain extends CollectTerminal {
     final PsiMethodCallExpression myCollector;
     final PsiMethodCallExpression myChain;
@@ -49,7 +54,7 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
     }
 
     @Override
-    String generateTerminal(CommentTracker ct) {
+    String generateTerminal(CommentTracker ct, boolean strictMode) {
       return ".collect(" + ct.text(myCollector) + ")";
     }
 
@@ -107,6 +112,13 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
     }
   }
 
+  @Nullable
+  @Override
+  public JComponent createOptionsPanel() {
+    return new SingleCheckboxOptionsPanel(InspectionsBundle.message("inspection.fuse.stream.operations.option.strict.mode"), this,
+                                          "myStrictMode");
+  }
+
   @NotNull
   @Override
   public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
@@ -129,7 +141,7 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
               .flatMap(Function.identity()).skip(1).joining();
             holder.registerProblem(nameElement,
                                    InspectionsBundle.message("inspection.fuse.stream.operations.message", fusedSteps),
-                                   new FuseStreamOperationsFix(fusedSteps));
+                                   new FuseStreamOperationsFix(fusedSteps, myStrictMode));
           }
         }
       }
@@ -161,10 +173,12 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
   }
 
   private static class FuseStreamOperationsFix implements LocalQuickFix {
-    private String myFusedSteps;
+    private final String myFusedSteps;
+    private final boolean myStrictMode;
 
-    public FuseStreamOperationsFix(String fusedSteps) {
+    public FuseStreamOperationsFix(String fusedSteps, boolean strictMode) {
       myFusedSteps = fusedSteps;
+      myStrictMode = strictMode;
     }
 
     @Nls
@@ -188,7 +202,7 @@ public class FuseStreamOperationsInspection extends AbstractBaseJavaLocalInspect
       CollectTerminal terminal = extractTerminal(chain);
       if (terminal == null) return;
       CommentTracker ct = new CommentTracker();
-      String stream = terminal.generateIntermediate(ct) + terminal.generateTerminal(ct);
+      String stream = terminal.generateIntermediate(ct) + terminal.generateTerminal(ct, myStrictMode);
       PsiElement toReplace = terminal.getElementToReplace();
       PsiElement result;
       if (toReplace != null) {

java/java-impl/src/inspectionDescriptions/FuseStreamOperations.html

@@ -10,6 +10,13 @@ Could be converted to
 <pre>
   return stream.sorted().toArray(String[]::new);
 </pre>
+<p>
+  Note that sometimes converted stream chain may replace explicit <b>ArrayList</b> with <b>Collectors.toList()</b> or explicit
+  <b>HashSet</b> with <b>Collectors.toSet()</b>. While current library implementation uses these collections internally,
+  this is not specified, thus can be changed in future possibly changing the semantics of your code. If you are concerned about this,
+  use the checkbox below to suppress usages of <b>toList</b> and <b>toSet</b> collectors. In this case <b>Collectors.toCollection()</b>
+  will be suggested instead.
+</p>
 <!-- tooltip end -->
 <p><small>New in 2017.3</small></p>
 </body>

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations/afterJustArrayListStrict.java

@@ -0,0 +1,18 @@
+// "Fix all 'Subsequent steps can be fused into Stream API chain' problems in file" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  interface Foo {
+  }
+
+  // IDEA-179303
+  void test1(Stream<Foo> fooStream) {
+    ArrayList<Foo> collectedFoos = fooStream.collect(Collectors.toCollection(ArrayList::new));
+  }
+
+  void test2(Stream<Foo> fooStream) {
+    List<Foo> collectedFoos = fooStream.collect(Collectors.toCollection(ArrayList::new));
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations/beforeJustArrayListStrict.java

@@ -0,0 +1,18 @@
+// "Fix all 'Subsequent steps can be fused into Stream API chain' problems in file" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  interface Foo {
+  }
+
+  // IDEA-179303
+  void test1(Stream<Foo> fooStream) {
+    ArrayList<Foo> collectedFoos = new ArrayList<>(fooStream.co<caret>llect(Collectors.toList()));
+  }
+
+  void test2(Stream<Foo> fooStream) {
+    List<Foo> collectedFoos = new ArrayList<>(fooStream.collect(Collectors.toList()));
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/FuseStreamOperationsInspectionTest.java

@@ -1,18 +1,4 @@
-/*
- * Copyright 2000-2017 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.java.codeInsight.daemon.quickFix;
 
 import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
@@ -25,7 +11,9 @@ public class FuseStreamOperationsInspectionTest extends LightQuickFixParameteriz
   @NotNull
   @Override
   protected LocalInspectionTool[] configureLocalInspectionTools() {
-    return new LocalInspectionTool[]{new FuseStreamOperationsInspection()};
+    FuseStreamOperationsInspection inspection = new FuseStreamOperationsInspection();
+    inspection.myStrictMode = getTestName(false).contains("Strict");
+    return new LocalInspectionTool[]{inspection};
   }
 
   public void test() { doAllTests(); }

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -925,6 +925,8 @@ inspection.fuse.stream.operations.fix.family.name=Fuse more statements to the St
 inspection.fuse.stream.operations.fix.name=Fuse {0} into the Stream API chain
 inspection.fuse.stream.operations.message=Stream may be extended replacing {0}
 inspection.fuse.stream.operations.display.name=Subsequent steps can be fused into Stream API chain
+inspection.fuse.stream.operations.option.strict.mode=Do not suggest 'toList()' or 'toSet()' collectors
+
 inspection.overwritten.key.set.message=Duplicate Set element
 inspection.overwritten.key.map.message=Duplicate Map key
 navigate.to.duplicate.fix=Navigate to duplicate
@@ -946,4 +948,4 @@ inspection.capturing.cleaner=Runnable passed to Cleaner.register() captures ''{0
 inspection.capturing.cleaner.description=Cleaner captures object reference
 
 inspection.redundant.explicit.close=Redundant close
-inspection.redundant.explicit.close.fix.name=Remove redundant close
+inspection.redundant.explicit.close.fix.name=Remove redundant close