Disallow usage of .collapseKeys() in EntryStream(s) (#2291)

Added `DangerousCollapseKeysUsage` error prone check to disallow usage of `collapseKeys()` API of `EntryStream`.

README.md

@@ -213,6 +213,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
 - `ConsistentOverrides`: Ensure values are bound to the correct variables when overriding methods
 - `FilterOutputStreamSlowMultibyteWrite`: Subclasses of FilterOutputStream should provide a more efficient implementation of `write(byte[], int, int)` to avoid slow writes.
 - `BugCheckerAutoService`: Concrete BugChecker implementations should be annotated `@AutoService(BugChecker.class)` for auto registration with error-prone.
+- `DangerousCollapseKeysUsage`: Disallow usage of `EntryStream#collapseKeys()`.
 
 ### Programmatic Application
 

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/DangerousCollapseKeysUsage.java

@@ -0,0 +1,60 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * 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.palantir.baseline.errorprone;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.method.MethodMatchers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
+        linkType = BugPattern.LinkType.CUSTOM,
+        severity = SeverityLevel.ERROR,
+        summary = "Disallow usage of .collapseKeys() in EntryStream(s).")
+public final class DangerousCollapseKeysUsage extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
+    private static final long serialVersionUID = 1L;
+    private static final String ERROR_MESSAGE = "The collapseKeys API of EntryStream must be avoided. The "
+            + "API is frequently used as a grouping operation but its not suitable for that use case. The contract "
+            + "requires duplicate keys to be adjacent to each other in the stream, which is rarely the case in "
+            + "production code paths. When this constraint is violated, it leads to a duplicate key error at runtime.\n"
+            + "A work around for the issue is to sort the keys prior to running the collapse operation. Since the "
+            + "sort operation is surprising, a comment is often added to explain. Overall the usage of collapseKeys() "
+            + "leads to code that is error prone or surprising.\nHence in place of collapseKeys() we recommend using "
+            + "grouping operations which may require creation of intermediate maps but should avoid surprising code.";
+
+    private static final Matcher<ExpressionTree> COLLAPSE_KEYS_CALL = MethodMatchers.instanceMethod()
+            .onExactClass("one.util.streamex.EntryStream")
+            .named("collapseKeys");
+
+    @Override
+    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+        if (!COLLAPSE_KEYS_CALL.matches(tree, state)) {
+            return Description.NO_MATCH;
+        }
+
+        // Fail on any 'parallel(...)' implementation, regardless of how many parameters it takes
+        return buildDescription(tree).setMessage(ERROR_MESSAGE).build();
+    }
+}

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/DangerousCollapseKeysUsageTest.java

@@ -0,0 +1,85 @@
+/*
+ * (c) Copyright 2022 Palantir Technologies Inc. All rights reserved.
+ *
+ * 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.palantir.baseline.errorprone;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DangerousCollapseKeysUsageTest {
+    private CompilationTestHelper compilationHelper;
+
+    @BeforeEach
+    public void before() {
+        compilationHelper = CompilationTestHelper.newInstance(DangerousCollapseKeysUsage.class, getClass());
+    }
+
+    @Test
+    public void should_error_when_collapse_keys_with_collector_is_used() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.List;",
+                        "import java.util.Map;",
+                        "import java.util.Set;",
+                        "import java.util.stream.Collectors;",
+                        "import one.util.streamex.EntryStream;",
+                        "import org.apache.commons.lang3.tuple.Pair;",
+                        "class Test {",
+                        "   public static final void main(String[] args) {",
+                        "       List<Pair<Integer, Integer>> nodesAndPriorities = List.of(",
+                        "                Pair.of(1, 5),",
+                        "                Pair.of(1, 3),",
+                        "                Pair.of(4, 2),",
+                        "                Pair.of(1, 9));",
+                        "        Map<Integer, Set<Integer>> nodesByPriority = EntryStream.of(nodesAndPriorities)",
+                        "                .mapValues(Pair::getRight)",
+                        "               // BUG: Diagnostic contains: collapseKeys API of EntryStream must be avoided",
+                        "                .collapseKeys(Collectors.toSet())",
+                        "                .toMap();",
+                        "   }",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void should_error_when_collapse_keys_is_used_even_without_collector() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.List;",
+                        "import java.util.Map;",
+                        "import java.util.stream.Collectors;",
+                        "import one.util.streamex.EntryStream;",
+                        "import org.apache.commons.lang3.tuple.Pair;",
+                        "class Test {",
+                        "   public static final void main(String[] args) {",
+                        "       List<Pair<Integer, Integer>> nodesAndPriorities = List.of(",
+                        "                Pair.of(1, 5),",
+                        "                Pair.of(1, 3),",
+                        "                Pair.of(4, 2),",
+                        "                Pair.of(1, 9));",
+                        "        Map<Integer, List<Integer>> nodesByPriority = EntryStream.of(nodesAndPriorities)",
+                        "                .mapValues(Pair::getRight)",
+                        "               // BUG: Diagnostic contains: collapseKeys API of EntryStream must be avoided",
+                        "                .collapseKeys()",
+                        "                .toMap();",
+                        "   }",
+                        "}")
+                .doTest();
+    }
+}

changelog/@unreleased/pr-2291.v2.yml

@@ -0,0 +1,6 @@
+type: feature
+feature:
+  description: Added `DangerousCollapseKeysUsage` error prone check to disallow usage
+    of `collapseKeys()` API of `EntryStream`.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2291