From 05e77241858d105a63caab52ddc562915bc8c600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 23 May 2024 19:28:04 +0200 Subject: [PATCH 1/4] [java] Add new rule UseEnumCollections --- .../bestpractices/UseEnumCollectionsRule.java | 50 ++++++++++++++ .../resources/category/java/bestpractices.xml | 33 +++++++++ .../bestpractices/UseEnumCollectionsTest.java | 11 +++ .../bestpractices/xml/UseEnumCollections.xml | 68 +++++++++++++++++++ 4 files changed, 162 insertions(+) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsTest.java create mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java new file mode 100644 index 0000000000..a90b971544 --- /dev/null +++ b/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 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; + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 8db08d0343..3a45bbaacd 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1723,6 +1723,39 @@ public class Foo { + + + + Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys + are of an enum type. This rule reports constructor expressions for hash sets or maps whose key + type is an enum type. + + 3 + + newSet() { + return new HashSet<>(); // Could be EnumSet.noneOf(Example.class) + } + + public static Map newMap() { + return new HashMap<>(); // Could be new EnumMap<>(Example.class) + } + } + ]]> + + + + + + + Use enumset + 1 + 8 + + This collection could be an EnumSet + + set = new HashSet<>(); + return set.contains(E.A); + } + } + ]]> + + + + + + Use enummap + 1 + 7 + + This collection could be an EnumMap + + bar() { + return new HashMap<>(); + } + } + ]]> + + + + Neg, LinkedHashSet or LinkedHashMap + 0 + bar() { + Set set = new LinkedHashSet<>(); + return new LinkedHashMap<>(); + } + } + ]]> + + + From 209fc134621862b7043dd05ac72265cd21a838d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 5 Jun 2024 11:17:57 +0200 Subject: [PATCH 2/4] support TreeSet/Map --- .../bestpractices/UseEnumCollectionsRule.java | 9 +++++++-- .../resources/category/java/bestpractices.xml | 6 +++--- .../bestpractices/xml/UseEnumCollections.xml | 17 +++++++++++++---- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java index a90b971544..b4acae55e6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java @@ -7,6 +7,8 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.TreeMap; +import java.util.TreeSet; import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; @@ -33,8 +35,11 @@ public class UseEnumCollectionsRule extends AbstractJavaRulechainRule { JTypeMirror builtType = call.getTypeMirror(); if (!builtType.isRaw()) { - boolean isMap = TypeTestUtil.isExactlyA(HashMap.class, builtType); - if (isMap || TypeTestUtil.isExactlyA(HashSet.class, builtType)) { + boolean isMap = TypeTestUtil.isExactlyA(HashMap.class, builtType) + || TypeTestUtil.isExactlyA(TreeMap.class, builtType); + if (isMap + || TypeTestUtil.isExactlyA(HashSet.class, builtType) + || TypeTestUtil.isExactlyA(TreeSet.class, builtType)) { List typeArgs = ((JClassType) builtType).getTypeArgs(); JTypeDeclSymbol keySymbol = typeArgs.get(0).getSymbol(); diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 3a45bbaacd..b965db740f 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1731,9 +1731,9 @@ public class Foo { class="net.sourceforge.pmd.lang.java.rule.bestpractices.UseEnumCollectionsRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useenumcollections"> - Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys - are of an enum type. This rule reports constructor expressions for hash sets or maps whose key - type is an enum type. + Wherever possible, use `EnumSet` or `EnumMap` instead of more generic set and map implementations when the keys + are of an enum type. The specialized collections are more space- and time-efficient. + This rule reports constructor expressions for hash and tree sets or maps whose key type is an enum type. 3 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml index c98b1d7124..76f474e3b5 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml @@ -6,10 +6,11 @@ Use enumset - 1 - 8 + 2 + 8,12 This collection could be an EnumSet + This collection could be an EnumSet set = new HashSet<>(); return set.contains(E.A); } + public static boolean bar2() { + Set set = new TreeSet<>(); + return set.contains(E.A); + } } ]]> @@ -30,10 +35,11 @@ Use enummap - 1 - 7 + 2 + 7,10 This collection could be an EnumMap + This collection could be an EnumMap bar() { return new HashMap<>(); } + public static Map bar() { + return new TreeMap<>(); + } } ]]> From f37d432b9e07bb907468909943904e7a6804b6c7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 27 Jun 2024 11:29:00 +0200 Subject: [PATCH 3/4] Revert "support TreeSet/Map" This reverts commit 209fc134621862b7043dd05ac72265cd21a838d2. --- .../bestpractices/UseEnumCollectionsRule.java | 9 ++------- .../resources/category/java/bestpractices.xml | 6 +++--- .../bestpractices/xml/UseEnumCollections.xml | 17 ++++------------- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java index b4acae55e6..a90b971544 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseEnumCollectionsRule.java @@ -7,8 +7,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.TreeMap; -import java.util.TreeSet; import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; @@ -35,11 +33,8 @@ public class UseEnumCollectionsRule extends AbstractJavaRulechainRule { JTypeMirror builtType = call.getTypeMirror(); if (!builtType.isRaw()) { - boolean isMap = TypeTestUtil.isExactlyA(HashMap.class, builtType) - || TypeTestUtil.isExactlyA(TreeMap.class, builtType); - if (isMap - || TypeTestUtil.isExactlyA(HashSet.class, builtType) - || TypeTestUtil.isExactlyA(TreeSet.class, builtType)) { + boolean isMap = TypeTestUtil.isExactlyA(HashMap.class, builtType); + if (isMap || TypeTestUtil.isExactlyA(HashSet.class, builtType)) { List typeArgs = ((JClassType) builtType).getTypeArgs(); JTypeDeclSymbol keySymbol = typeArgs.get(0).getSymbol(); diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index b965db740f..3a45bbaacd 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1731,9 +1731,9 @@ public class Foo { class="net.sourceforge.pmd.lang.java.rule.bestpractices.UseEnumCollectionsRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useenumcollections"> - Wherever possible, use `EnumSet` or `EnumMap` instead of more generic set and map implementations when the keys - are of an enum type. The specialized collections are more space- and time-efficient. - This rule reports constructor expressions for hash and tree sets or maps whose key type is an enum type. + Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys + are of an enum type. This rule reports constructor expressions for hash sets or maps whose key + type is an enum type. 3 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml index 76f474e3b5..c98b1d7124 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseEnumCollections.xml @@ -6,11 +6,10 @@ Use enumset - 2 - 8,12 + 1 + 8 This collection could be an EnumSet - This collection could be an EnumSet set = new HashSet<>(); return set.contains(E.A); } - public static boolean bar2() { - Set set = new TreeSet<>(); - return set.contains(E.A); - } } ]]> @@ -35,11 +30,10 @@ Use enummap - 2 - 7,10 + 1 + 7 This collection could be an EnumMap - This collection could be an EnumMap bar() { return new HashMap<>(); } - public static Map bar() { - return new TreeMap<>(); - } } ]]> From bf8e11d9b454c82c775cac88d14392c00768a61f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 27 Jun 2024 11:40:18 +0200 Subject: [PATCH 4/4] [doc] Update release notes (#577, #5038) --- docs/pages/release_notes.md | 6 ++++++ pmd-java/src/main/resources/category/java/bestpractices.xml | 3 ++- pmd-java/src/main/resources/rulesets/java/quickstart.xml | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index e8d7cdbf90..b7ee338344 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,7 +14,13 @@ 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 +* java + * [#577](https://github.com/pmd/pmd/issues/577): \[java] New Rule: Check that Map is an EnumMap if K is an enum value ### 🚨 API Changes diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 3a45bbaacd..a8a3169254 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1732,7 +1732,8 @@ public class Foo { externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useenumcollections"> Wherever possible, use `EnumSet` or `EnumMap` instead of `HashSet` and `HashMap` when the keys - are of an enum type. This rule reports constructor expressions for hash sets or maps whose key + 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. 3 diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index da02719225..4afb964163 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -52,6 +52,7 @@ +