[java] Add new rule UseEnumCollections (#5038)

Merge pull request #5038 from oowekyala:issue577-new-rule-use-enum-collections

docs/pages/release_notes.md

@@ -14,6 +14,10 @@ This is a {{ site.pmd.release_type }} release.
 
 ### 🚀 New and noteworthy
 
+#### ✨ New Rules
+* The new Java rule {%rule java/bestpractices/UseEnumCollections %} reports usages for `HashSet` and `HashMap`
+  when the keys are of an enum type. The specialized enum collections are more space- and time-efficient.
+
 ### 🐛 Fixed Issues
 * core
   * [#4992](https://github.com/pmd/pmd/pull/4992): \[core] CPD: Include processing errors in XML report
@@ -26,6 +30,7 @@ This is a {{ site.pmd.release_type }} release.
 * java
   * [#5050](https://github.com/pmd/pmd/issues/5050): \[java] Problems with pattern variables in switch branches
 * java-bestpractices
+  * [#577](https://github.com/pmd/pmd/issues/577): \[java] New Rule: Check that Map<K,V> is an EnumMap if K is an enum value
   * [#5047](https://github.com/pmd/pmd/issues/5047): \[java] UnusedPrivateMethod FP for Generics & Overloads
 * plsql
   * [#1934](https://github.com/pmd/pmd/issues/1934): \[plsql] ParseException with MERGE statement in anonymous block

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java

@@ -0,0 +1,50 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+
+import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
+import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol;
+import net.sourceforge.pmd.lang.java.types.JClassType;
+import net.sourceforge.pmd.lang.java.types.JTypeMirror;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+
+/**
+ * Detect cases where EnumSet and EnumMap can be used.
+ *
+ * @author Clément Fournier
+ */
+public class UseEnumCollectionsRule extends AbstractJavaRulechainRule {
+
+    public UseEnumCollectionsRule() {
+        super(ASTConstructorCall.class);
+    }
+
+
+    @Override
+    public Object visit(ASTConstructorCall call, Object data) {
+        JTypeMirror builtType = call.getTypeMirror();
+
+        if (!builtType.isRaw()) {
+            boolean isMap = TypeTestUtil.isExactlyA(HashMap.class, builtType);
+            if (isMap || TypeTestUtil.isExactlyA(HashSet.class, builtType)) {
+
+                List<JTypeMirror> typeArgs = ((JClassType) builtType).getTypeArgs();
+                JTypeDeclSymbol keySymbol = typeArgs.get(0).getSymbol();
+
+                if (keySymbol instanceof JClassSymbol && ((JClassSymbol) keySymbol).isEnum()) {
+                    String enumCollectionReplacement = isMap ? "EnumMap" : "EnumSet";
+                    asCtx(data).addViolation(call.getTypeNode(), enumCollectionReplacement);
+                }
+            }
+        }
+        return null;
+    }
+}

pmd-java/src/main/resources/category/java/bestpractices.xml

@@ -1723,6 +1723,40 @@ public class Foo {
         </example>
     </rule>
 
+
+    <rule name="UseEnumCollections"
+          language="java"
+          since="7.3.0"
+          message="This collection could be an {0}"
+          class="net.sourceforge.pmd.lang.java.rule.bestpractices.UseEnumCollectionsRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useenumcollections">
+        <description>
+            Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys
+            are of an enum type. The specialized enum collections are more space- and time-efficient.
+            This rule reports constructor expressions for hash sets or maps whose key
+            type is an enum type.
+        </description>
+        <priority>3</priority>
+        <example>
+<![CDATA[
+            import java.util.EnumMap;
+            import java.util.HashSet;
+
+            enum Example {
+                A, B, C;
+
+                public static Set<Example> newSet() {
+                    return new HashSet<>(); // Could be EnumSet.noneOf(Example.class)
+                }
+
+                public static <V> Map<Example, V> newMap() {
+                    return new HashMap<>(); // Could be new EnumMap<>(Example.class)
+                }
+            }
+            ]]>
+        </example>
+    </rule>
+
     <rule name="UseStandardCharsets"
           language="java"
           since="6.34.0"

pmd-java/src/main/resources/rulesets/java/quickstart.xml

@@ -52,6 +52,7 @@
     <rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
     <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
     <rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
+    <!-- <rule ref="category/java/bestpractices.xml/UseEnumCollections"/> -->
     <rule ref="category/java/bestpractices.xml/UseStandardCharsets" />
     <!-- <rule ref="category/java/bestpractices.xml/UseTryWithResources" /> -->
     <!-- <rule ref="category/java/bestpractices.xml/UseVarargs" /> -->

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsTest.java

@@ -0,0 +1,11 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+import net.sourceforge.pmd.test.PmdRuleTst;
+
+class UseEnumCollectionsTest extends PmdRuleTst {
+    // no additional unit tests
+}

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml

@@ -0,0 +1,68 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+    xmlns="http://pmd.sourceforge.net/rule-tests"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+
+    <test-code>
+        <description>Use enumset</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>8</expected-linenumbers>
+        <expected-messages>
+            <message>This collection could be an EnumSet</message>
+        </expected-messages>
+        <code><![CDATA[
+            import java.util.*;
+
+            public class Foo {
+
+                enum E {A, B, C}
+
+                public static boolean bar() {
+                    Set<E> set = new HashSet<>();
+                    return set.contains(E.A);
+                }
+            }
+            ]]></code>
+    </test-code>
+
+
+
+    <test-code>
+        <description>Use enummap</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>7</expected-linenumbers>
+        <expected-messages>
+            <message>This collection could be an EnumMap</message>
+        </expected-messages>
+        <code><![CDATA[
+            import java.util.*;
+
+            public class Foo {
+                public enum E {A, B, C}
+
+                public static Map<E, Boolean> bar() {
+                    return new HashMap<>();
+                }
+            }
+            ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Neg, LinkedHashSet or LinkedHashMap</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+            import java.util.*;
+
+            public class Foo {
+                public enum E {A, B, C}
+
+                public static Map<E, Boolean> bar() {
+                    Set<E> set = new LinkedHashSet<>();
+                    return new LinkedHashMap<>();
+                }
+            }
+            ]]></code>
+    </test-code>
+
+</test-data>