FuseStreamOperationsInspection

Fixes IDEA-179303 Suggest replacing Collectors.toList()/toSet() + collection constructor with Collectors.toCollection

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

@@ -221,6 +221,10 @@ class CollectMigration extends BaseStreamApiMigration {
       return StreamEx.ofNullable(myLoop);
     }
 
+    StreamEx<String> fusedElements() {
+      return StreamEx.empty();
+    }
+
     public InitializerUsageStatus getStatus() { return myStatus; }
 
     void cleanUp() {}
@@ -672,6 +676,11 @@ class CollectMigration extends BaseStreamApiMigration {
       myStatement.delete();
     }
 
+    @Override
+    StreamEx<String> fusedElements() {
+      return myDownstream.fusedElements().append("'sort'");
+    }
+
     @Nullable
     public static CollectTerminal tryWrap(CollectTerminal terminal, PsiElement element) {
       PsiVariable containerVariable = terminal.getTargetVariable();
@@ -718,7 +727,7 @@ class CollectMigration extends BaseStreamApiMigration {
   }
 
   static abstract class RecreateTerminal extends CollectTerminal {
-    private final CollectTerminal myUpstream;
+    final CollectTerminal myUpstream;
     private final String myIntermediate;
     final PsiExpression myCreateExpression;
 
@@ -777,6 +786,11 @@ class CollectMigration extends BaseStreamApiMigration {
       return ".toArray(" + mySupplier + ")";
     }
 
+    @Override
+    StreamEx<String> fusedElements() {
+      return myUpstream.fusedElements().append("'toArray'");
+    }
+
     @Contract("_, null -> null")
     @Nullable
     public static ToArrayTerminal tryWrap(CollectTerminal terminal, PsiElement element) {
@@ -834,7 +848,7 @@ class CollectMigration extends BaseStreamApiMigration {
     NewListTerminal(CollectTerminal upstream,
                     PsiLocalVariable variable,
                     String intermediate,
-                    PsiExpression newListExpression,
+                    PsiNewExpression newListExpression,
                     PsiType resultType) {
       super(upstream, variable, intermediate, newListExpression);
       myResultType = resultType;
@@ -845,6 +859,12 @@ class CollectMigration extends BaseStreamApiMigration {
       return ".collect(" + getCollectionCollector(myCreateExpression, myResultType) + ")";
     }
 
+    @Override
+    StreamEx<String> fusedElements() {
+      PsiJavaCodeReferenceElement reference = ((PsiNewExpression)myCreateExpression).getClassReference();
+      return myUpstream.fusedElements().append(Objects.requireNonNull(reference).getReferenceName());
+    }
+
     @Nullable
     public static NewListTerminal tryWrap(CollectTerminal terminal, PsiElement element) {
       if (terminal.getStatus() == ControlFlowUtils.InitializerUsageStatus.UNKNOWN) return null;
@@ -880,11 +900,12 @@ class CollectMigration extends BaseStreamApiMigration {
       }
       if (!(candidate instanceof PsiNewExpression)) return null;
       if (!InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_UTIL_COLLECTION)) return null;
-      PsiExpressionList argumentList = ((PsiNewExpression)candidate).getArgumentList();
+      PsiNewExpression newExpression = (PsiNewExpression)candidate;
+      PsiExpressionList argumentList = newExpression.getArgumentList();
       if (argumentList == null) return null;
       PsiExpression[] args = argumentList.getExpressions();
       if (args.length != 1 || !terminal.isTargetReference(args[0])) return null;
-      return new NewListTerminal(terminal, var, intermediateSteps, candidate, type);
+      return new NewListTerminal(terminal, var, intermediateSteps, newExpression, type);
     }
   }
 }

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

@@ -0,0 +1,218 @@
+/*
+ * 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.
+ */
+package com.intellij.codeInspection.streamMigration;
+
+import com.intellij.codeInspection.*;
+import com.intellij.codeInspection.streamMigration.CollectMigration.CollectTerminal;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiUtil;
+import com.intellij.refactoring.util.RefactoringUtil;
+import com.intellij.util.ArrayUtil;
+import com.siyeh.ig.callMatcher.CallMatcher;
+import com.siyeh.ig.psiutils.ControlFlowUtils;
+import one.util.streamex.StreamEx;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Objects;
+import java.util.function.Function;
+
+import static com.intellij.util.ObjectUtils.tryCast;
+
+public class FuseStreamOperationsInspection extends BaseJavaBatchLocalInspectionTool {
+  private static final CallMatcher STREAM_COLLECT =
+    CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "collect").parameterTypes("java.util.stream.Collector");
+  private static final CallMatcher COLLECT_TO_COLLECTION =
+    CallMatcher.anyOf(
+      CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toList", "toSet").parameterCount(0),
+      CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toCollection").parameterCount(1));
+
+  private static class StreamCollectChain extends CollectTerminal {
+    final PsiMethodCallExpression myCollector;
+    final PsiMethodCallExpression myChain;
+
+    protected StreamCollectChain(PsiLocalVariable variable,
+                                 PsiMethodCallExpression chain,
+                                 PsiMethodCallExpression collector) {
+      super(variable, null, ControlFlowUtils.InitializerUsageStatus.DECLARED_JUST_BEFORE);
+      myCollector = collector;
+      myChain = chain;
+    }
+
+    @Override
+    String generateIntermediate() {
+      PsiExpression qualifier = myChain.getMethodExpression().getQualifierExpression();
+      return Objects.requireNonNull(qualifier).getText();
+    }
+
+    @Override
+    String generateTerminal() {
+      return ".collect(" + myCollector.getText() + ")";
+    }
+
+    private static PsiClass resolveClassCreatedByFunction(PsiExpression function) {
+      function = PsiUtil.skipParenthesizedExprDown(function);
+      if (function instanceof PsiMethodReferenceExpression && ((PsiMethodReferenceExpression)function).isConstructor()) {
+        PsiExpression qualifier = ((PsiMethodReferenceExpression)function).getQualifierExpression();
+        if (qualifier instanceof PsiReferenceExpression) {
+          return tryCast(((PsiReferenceExpression)qualifier).resolve(), PsiClass.class);
+        }
+      }
+      if (function instanceof PsiLambdaExpression) {
+        PsiExpression body = LambdaUtil.extractSingleExpressionFromBody(((PsiLambdaExpression)function).getBody());
+        PsiNewExpression newExpression = tryCast(PsiUtil.skipParenthesizedExprDown(body), PsiNewExpression.class);
+        if (newExpression != null && newExpression.getAnonymousClass() == null && newExpression.getQualifier() == null &&
+            newExpression.getArgumentList() != null && newExpression.getArgumentList().getExpressions().length == 0) {
+          PsiJavaCodeReferenceElement classReference = newExpression.getClassReference();
+          if (classReference != null) {
+            return tryCast(classReference.resolve(), PsiClass.class);
+          }
+        }
+      }
+      return null;
+    }
+
+    @Override
+    String getIntermediateStepsFromCollection() {
+      String name = myCollector.getMethodExpression().getReferenceName();
+      if ("toList".equals(name)) return "";
+      if ("toSet".equals(name)) return ".distinct()";
+      if ("toCollection".equals(name)) {
+        PsiExpression collectionFunction = myCollector.getArgumentList().getExpressions()[0];
+        PsiClass psiClass = resolveClassCreatedByFunction(collectionFunction);
+        if (psiClass == null) return null;
+        return CollectMigration.INTERMEDIATE_STEPS.get(psiClass.getQualifiedName());
+      }
+      return null;
+    }
+  }
+
+  private static class StreamCollectChainNoVar extends StreamCollectChain {
+
+    protected StreamCollectChainNoVar(PsiMethodCallExpression chain, PsiMethodCallExpression collector) {
+      super(null, chain, collector);
+    }
+
+    @Override
+    StreamEx<PsiExpression> targetReferences() {
+      return StreamEx.of(myChain);
+    }
+
+    @Override
+    boolean isTargetReference(PsiExpression expression) {
+      return expression == myChain;
+    }
+  }
+
+  @NotNull
+  @Override
+  public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
+    if (!PsiUtil.isLanguageLevel8OrHigher(holder.getFile())) {
+      return PsiElementVisitor.EMPTY_VISITOR;
+    }
+    return new JavaElementVisitor() {
+      @Override
+      public void visitMethodCallExpression(PsiMethodCallExpression call) {
+        if (STREAM_COLLECT.test(call)) {
+          PsiMethodCallExpression arg =
+            tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiMethodCallExpression.class);
+          if (COLLECT_TO_COLLECTION.test(arg)) {
+            CollectTerminal newTerminal = extractTerminal(call);
+            if (newTerminal == null) return;
+            PsiElement nameElement = call.getMethodExpression().getReferenceNameElement();
+            if (nameElement == null) return;
+            String fusedSteps = newTerminal.fusedElements()
+              .mapLastOrElse(s -> StreamEx.of(", ", s), s -> StreamEx.of(" and ", s))
+              .flatMap(Function.identity()).skip(1).joining();
+            holder.registerProblem(nameElement, "Stream may be extended replacing " + fusedSteps,
+                                   new FuseStreamOperationsFix(fusedSteps));
+          }
+        }
+      }
+    };
+  }
+
+  @Nullable
+  private static CollectTerminal extractTerminal(PsiMethodCallExpression streamChain) {
+    if(streamChain.getMethodExpression().getQualifierExpression() == null) return null;
+    PsiMethodCallExpression collector =
+      tryCast(PsiUtil.skipParenthesizedExprDown(ArrayUtil.getFirstElement(streamChain.getArgumentList().getExpressions())),
+              PsiMethodCallExpression.class);
+    PsiLocalVariable var = tryCast(streamChain.getParent(), PsiLocalVariable.class);
+    CollectTerminal terminal;
+    PsiElement nextElement;
+    if (var == null) {
+      terminal = new StreamCollectChainNoVar(streamChain, collector);
+      nextElement = RefactoringUtil.getParentStatement(streamChain, false);
+    }
+    else {
+      PsiDeclarationStatement declaration = tryCast(var.getParent(), PsiDeclarationStatement.class);
+      if (declaration == null || declaration.getDeclaredElements().length != 1) return null;
+      terminal = new StreamCollectChain(var, streamChain, collector);
+      nextElement = PsiTreeUtil.skipWhitespacesAndCommentsForward(declaration);
+    }
+    CollectTerminal newTerminal = CollectMigration.includePostStatements(terminal, nextElement);
+    if (newTerminal == terminal) return null;
+    return newTerminal;
+  }
+
+  private static class FuseStreamOperationsFix implements LocalQuickFix {
+    private String myFusedSteps;
+
+    public FuseStreamOperationsFix(String fusedSteps) {
+      myFusedSteps = fusedSteps;
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getName() {
+      return "Fuse " + myFusedSteps + " into the Stream API chain";
+    }
+
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Fuse more statements to the Stream API chain";
+    }
+
+    @Override
+    public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+      PsiMethodCallExpression chain = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class);
+      if (chain == null) return;
+      PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
+      CollectTerminal terminal = extractTerminal(chain);
+      if (terminal == null) return;
+      String stream = terminal.generateIntermediate() + terminal.generateTerminal();
+      PsiElement toReplace = terminal.getElementToReplace();
+      PsiElement result;
+      if (toReplace != null) {
+        result = toReplace.replace(factory.createExpressionFromText(stream, toReplace));
+      }
+      else {
+        PsiVariable variable = Objects.requireNonNull(terminal.getTargetVariable());
+        PsiExpression initializer = Objects.requireNonNull(variable.getInitializer());
+        result = initializer.replace(factory.createExpressionFromText(stream, initializer));
+      }
+      terminal.cleanUp();
+      LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result);
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations/afterJustArrayList.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.toList());
+  }
+}

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

@@ -0,0 +1,10 @@
+// "Fuse HashSet and ArrayList into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  Collection<String> test(String[] args) {
+      return Arrays.stream(args).filter(String::isEmpty).distinct().collect(Collectors.toList());
+  }
+}

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

@@ -0,0 +1,10 @@
+// "Fuse ArrayList, 'sort' and 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public void testSetListSort(String[] args) {
+      System.out.println(Arrays.stream(args).distinct().sorted().toArray());
+  }
+}

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

@@ -0,0 +1,11 @@
+// "Fuse 'sort' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public List<String> testToArray(String[] args) {
+    List<String> list = Arrays.stream(args).sorted().collect(Collectors.toList());
+      return list;
+  }
+}

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

@@ -0,0 +1,10 @@
+// "Fuse 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public String[] testToArray(String[] args) {
+      return Arrays.stream(args).toArray(String[]::new);
+  }
+}

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

@@ -0,0 +1,10 @@
+// "Fuse 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public String[] testToArray(String[] args) {
+      return Arrays.stream(args).distinct().sorted().toArray(String[]::new);
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations/beforeJustArrayList.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/testData/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations/beforeListSetList.java

@@ -0,0 +1,12 @@
+// "Fuse HashSet and ArrayList into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  Collection<String> test(String[] args) {
+    List<String> list = Arrays.stream(args).filter(String::isEmpty).coll<caret>ect(Collectors.toList());
+    HashSet<String> strings = new HashSet<>(list);
+    return new ArrayList<>(strings);
+  }
+}

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

@@ -0,0 +1,13 @@
+// "Fuse ArrayList, 'sort' and 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public void testSetListSort(String[] args) {
+    Set<String> set = Arrays.stream(args).co<caret>llect(Collectors.toSet());
+    List<String> list = new ArrayList<>(set);
+    list.sort(null);
+    System.out.println(list.toArray());
+  }
+}

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

@@ -0,0 +1,12 @@
+// "Fuse 'sort' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public List<String> testToArray(String[] args) {
+    List<String> list = Arrays.stream(args).c<caret>ollect(Collectors.toList());
+    list.sort(null);
+    return list;
+  }
+}

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

@@ -0,0 +1,11 @@
+// "Fuse 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public String[] testToArray(String[] args) {
+    List<String> list = Arrays.stream(args).co<caret>llect(Collectors.toList());
+    return list.toArray(new String[list.size()]);
+  }
+}

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

@@ -0,0 +1,11 @@
+// "Fuse 'toArray' into the Stream API chain" "true"
+import java.util.*;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class Test {
+  public String[] testToArray(String[] args) {
+    Set<String> set = Arrays.stream(args).co<caret>llect(Collectors.toCollection(TreeSet::new));
+    return set.toArray(new String[set.size()]);
+  }
+}

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

@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+package com.intellij.java.codeInsight.daemon.quickFix;
+
+import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
+import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.codeInspection.streamMigration.FuseStreamOperationsInspection;
+import org.jetbrains.annotations.NotNull;
+
+
+public class FuseStreamOperationsInspectionTest extends LightQuickFixParameterizedTestCase {
+  @NotNull
+  @Override
+  protected LocalInspectionTool[] configureLocalInspectionTools() {
+    return new LocalInspectionTool[]{new FuseStreamOperationsInspection()};
+  }
+
+  public void test() { doAllTests(); }
+
+  @Override
+  protected String getBasePath() {
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/fuseStreamOperations";
+  }
+}

resources-en/src/inspectionDescriptions/FuseStreamOperations.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+Detects when some transformations are performed on Stream API result which could be incorporated into the Stream API call chain directly.
+<!-- tooltip end -->
+<p><small>New in 2017.3</small></p>
+</body>
+</html>

resources/src/META-INF/IdeaPlugin.xml

@@ -806,6 +806,9 @@
     <localInspection groupPath="Java,Java language level migration aids" language="JAVA" shortName="Convert2streamapi" displayName="Loop can be collapsed with Stream API"
                      groupKey="group.names.language.level.specific.issues.and.migration.aids8" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="INFORMATION"
                      implementationClass="com.intellij.codeInspection.streamMigration.StreamApiMigrationInspection"/>
+    <localInspection groupPath="Java" language="JAVA" shortName="FuseStreamOperations" displayName="Subsequent steps can be fused into Stream API chain"
+                     groupKey="group.names.code.style.issues" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
+                     implementationClass="com.intellij.codeInspection.streamMigration.FuseStreamOperationsInspection"/>
     <localInspection groupPath="Java,Java language level migration aids" language="JAVA" shortName="SimplifyForEach" displayName="forEach call can be simplified"
                      groupKey="group.names.language.level.specific.issues.and.migration.aids8" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="INFORMATION"
                      implementationClass="com.intellij.codeInspection.streamMigration.SimplifyForEachInspection"/>