Update AvoidNewHashMapInt to include HashSet (#2292)

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

@@ -17,7 +17,6 @@
 package com.palantir.baseline.errorprone;
 
 import com.google.auto.service.AutoService;
-import com.google.common.collect.Maps;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -34,30 +33,42 @@
         link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
         linkType = BugPattern.LinkType.CUSTOM,
         severity = BugPattern.SeverityLevel.WARNING,
-        summary = "The HashMap(int) constructor is misleading: once the HashMap reaches 3/4 of the supplied size, it"
-            + " will double its internal storage array. Instead use Maps.newHashMapWithExpectedSize which behaves as"
-            + " expected.See"
+        summary = "The HashMap(int) and HashSet(int) constructors are misleading: once the HashMap/HashSet reaches 3/4"
+            + " of the supplied size, it resize. Instead use Maps.newHashMapWithExpectedSize or"
+            + " Sets.newHashSetWithExpectedSize which behaves as expected. See"
             + " https://github.com/palantir/gradle-baseline/blob/develop/docs/best-practices/java-coding-guidelines/readme.md#avoid-new-HashMap(int)"
             + " for more information.")
 public final class AvoidNewHashMapInt extends BugChecker implements BugChecker.NewClassTreeMatcher {
 
     private static final long serialVersionUID = 1L;
 
+    private static final Matcher<ExpressionTree> NEW_HASH_SET =
+            MethodMatchers.constructor().forClass("java.util.HashSet").withParameters("int");
     private static final Matcher<ExpressionTree> NEW_HASH_MAP =
             MethodMatchers.constructor().forClass("java.util.HashMap").withParameters("int");
 
     @Override
     public Description matchNewClass(NewClassTree tree, VisitorState state) {
-        if (!NEW_HASH_MAP.matches(tree, state)) {
-            return Description.NO_MATCH;
+        if (NEW_HASH_SET.matches(tree, state)) {
+            SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
+            String newType = SuggestedFixes.qualifyType(state, fixBuilder, "com.google.common.collect.Sets");
+            String arg = state.getSourceForNode(tree.getArguments().get(0));
+            String replacement = newType + ".newHashSetWithExpectedSize(" + arg + ")";
+            return buildDescription(tree)
+                    .addFix(fixBuilder.replace(tree, replacement).build())
+                    .build();
         }
 
-        SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
-        String newType = SuggestedFixes.qualifyType(state, fixBuilder, Maps.class.getName());
-        String arg = state.getSourceForNode(tree.getArguments().get(0));
-        String replacement = newType + ".newHashMapWithExpectedSize(" + arg + ")";
-        return buildDescription(tree)
-                .addFix(fixBuilder.replace(tree, replacement).build())
-                .build();
+        if (NEW_HASH_MAP.matches(tree, state)) {
+            SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
+            String newType = SuggestedFixes.qualifyType(state, fixBuilder, "com.google.common.collect.Maps");
+            String arg = state.getSourceForNode(tree.getArguments().get(0));
+            String replacement = newType + ".newHashMapWithExpectedSize(" + arg + ")";
+            return buildDescription(tree)
+                    .addFix(fixBuilder.replace(tree, replacement).build())
+                    .build();
+        }
+
+        return Description.NO_MATCH;
     }
 }

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

@@ -21,20 +21,37 @@
 
 public final class AvoidNewHashMapIntTest {
 
+    @Test
+    public void testRewriteHashSetConstructor() {
+        RefactoringValidator.of(AvoidNewHashMapInt.class, getClass())
+                .addInputLines(
+                        "Test.java",
+                        "import java.util.HashSet;",
+                        "import java.util.Set;",
+                        "class Test {{ Set<Integer> set = new HashSet<>(10); }}")
+                .addOutputLines(
+                        "Test.java",
+                        "import com.google.common.collect.Sets;",
+                        "import java.util.HashSet;", // HACK
+                        "import java.util.Set;",
+                        "class Test {{ Set<Integer> set = Sets.newHashSetWithExpectedSize(10); }}")
+                .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+    }
+
     @Test
     public void testRewriteHashMapConstructor() {
         RefactoringValidator.of(AvoidNewHashMapInt.class, getClass())
                 .addInputLines(
                         "Test.java",
-                        "import java.util.Map;",
                         "import java.util.HashMap;",
-                        "class Test {{ Map<Integer, Integer> map = new HashMap<>(10);}}")
+                        "import java.util.Map;",
+                        "class Test {{ Map<Integer, Integer> map = new HashMap<>(10); }}")
                 .addOutputLines(
                         "Test.java",
                         "import com.google.common.collect.Maps;",
                         "import java.util.HashMap;", // HACK
                         "import java.util.Map;",
-                        "class Test {{ Map<Integer, Integer> map = Maps.newHashMapWithExpectedSize(10);}}")
+                        "class Test {{ Map<Integer, Integer> map = Maps.newHashMapWithExpectedSize(10); }}")
                 .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
     }
 }

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

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Update `AvoidNewHashMapInt` to warn on uses of `new HashSet(int)`.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2292

docs/best-practices/java-coding-guidelines/readme.md

@@ -651,15 +651,15 @@ non-primitive types such as Java8/Joda-Time
 
 ### Avoid new HashMap(int)
 
-Avoid new HashMap(int), use Maps#newHashMapWithExpectedSize(int) instead.
+Avoid `new HashMap(int)` and `new HashSet(int)`, use `Maps#newHashMapWithExpectedSize(int)` and `Sets#newHashSetWithExpectedSize(int)` instead.
 
-The behavior of `new HashMap(int)` is misleading -- the parameter represents an
+The behavior of `new HashMap(int)`/`new HashSet(int)` is misleading -- the parameter represents an
 internal size rather than an ability to hold that number of elements.  If a HashMap
 with capacity K receives K elements, it will increase its capacity to 2*K along the way.
 This is because HashMap doubles its internal storage by 2 once it reaches 75% capacity.
 
-The Guava static method `Maps.newHashMapWithExpectedSize(int)` creates a HashMap which
-will not resize if provided the given number of elements.
+The Guava static methods `Maps.newHashMapWithExpectedSize(int)` and `Sets#newHashSetWithExpectedSize(int)` creates a
+HashMap which will not resize if provided the given number of elements.
 
 ## APIs and Interfaces