Feature/avoid hashmapint (#1773)

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

@@ -0,0 +1,64 @@
+/*
+ * (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.common.collect.Maps;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
+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.NewClassTree;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        name = "AvoidNewHashMapInt",
+        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"
+            + " 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_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;
+        }
+
+        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();
+    }
+}

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

@@ -0,0 +1,40 @@
+/*
+ * (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.errorprone.BugCheckerRefactoringTestHelper;
+import org.junit.jupiter.api.Test;
+
+public final class AvoidNewHashMapIntTest {
+
+    @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);}}")
+                .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);}}")
+                .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+    }
+}

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

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Feature/avoid hashmapint
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1773

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

@@ -29,6 +29,7 @@ topics:
     - [Avoid Generics clutter where possible](#avoid-generics-clutter-where-possible)
     - [Keep Boolean expressions simple](#keep-boolean-expressions-simple)
     - [Durations: measured by longs or complex types, not by ints](#durations-measured-by-longs-or-complex-types-not-by-ints)
+    - [Avoid new HashMap(int), use Maps.newHashMapWithExpectedSize(int)](#avoid-new-HashMap(int))
   - [APIs and Interfaces](#apis-and-interfaces)
     - [Indicate failure consistently](#indicate-failure-consistently)
     - [Return empty arrays and collections, not null](#return-empty-arrays-and-collections-not-null)
@@ -648,6 +649,18 @@ must thus be represented by `long` types, or by appropriate
 non-primitive types such as Java8/Joda-Time
 [Duration](https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html).
 
+### Avoid new HashMap(int)
+
+Avoid new HashMap(int), use Maps#newHashMapWithExpectedSize(int) instead.
+
+The behavior of `new HashMap(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.
+
 ## APIs and Interfaces
 
 ### Indicate failure consistently

gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java

@@ -28,6 +28,7 @@ public class BaselineErrorProneExtension {
      */
     private static final ImmutableList<String> DEFAULT_PATCH_CHECKS = ImmutableList.of(
             // Baseline checks
+            "AvoidNewHashMapInt",
             "CatchBlockLogException",
             // TODO(ckozak): re-enable pending scala check
             // "CatchSpecificity",