diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 2df014843f..fb1f6b2194 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,6 +14,11 @@ This is a {{ site.pmd.release_type }} release. ### 🚀 New and noteworthy +### ✨ New rules + +- The new Java rule {%rule java/bestpractices/UnnecessaryVarargsArrayCreation %} reports explicit array creation + when a varargs is expected. This is more heavy to read and could be simplified. + ### 🌟 Rule Changes * {%rule java/bestpractices/JUnitTestsShouldIncludeAssert %} and {% rule java/bestpractices/JUnitTestContainsTooManyAsserts %} @@ -38,6 +43,7 @@ This is a {{ site.pmd.release_type }} release. * [#4947](https://github.com/pmd/pmd/issues/4947): \[java] Broken TextBlock parser * java-bestpractices * [#1084](https://github.com/pmd/pmd/issues/1084): \[java] Allow JUnitTestsShouldIncludeAssert to configure verification methods + * [#3216](https://github.com/pmd/pmd/issues/3216): \[java] New rule: UnnecessaryVarargsArrayCreation * [#4435](https://github.com/pmd/pmd/issues/4435): \[java] \[7.0-rc1] UnusedAssignment for used field * [#4569](https://github.com/pmd/pmd/issues/4569): \[java] ForLoopCanBeForeach reports on loop `for (int i = 0; i < list.size(); i += 2)` * [#4618](https://github.com/pmd/pmd/issues/4618): \[java] UnusedAssignment false positive with conditional assignments of fields diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnnecessaryVarargsArrayCreationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnnecessaryVarargsArrayCreationRule.java new file mode 100644 index 0000000000..9ecb6e5374 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnnecessaryVarargsArrayCreationRule.java @@ -0,0 +1,61 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import java.util.List; + +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTArrayAllocation; +import net.sourceforge.pmd.lang.java.ast.InvocationNode; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.types.JArrayType; +import net.sourceforge.pmd.lang.java.types.JTypeMirror; +import net.sourceforge.pmd.lang.java.types.OverloadSelectionResult; +import net.sourceforge.pmd.lang.java.types.TypePrettyPrint; + +public class UnnecessaryVarargsArrayCreationRule extends AbstractJavaRulechainRule { + + // we visit array allocations because they are less frequent than + // method calls + public UnnecessaryVarargsArrayCreationRule() { + super(ASTArrayAllocation.class); + } + + @Override + public Object visit(ASTArrayAllocation array, Object data) { + + JavaNode parent = array.getParent(); + if (parent instanceof ASTArgumentList && array.getIndexInParent() == parent.getNumChildren() - 1) { + // node is the last param in an arguments list + InvocationNode call = (InvocationNode) parent.getParent(); + OverloadSelectionResult info = call.getOverloadSelectionInfo(); + if (info.isFailed() || info.isVarargsCall() + || !info.getMethodType().isVarargs()) { + return null; + } + + List formals = info.getMethodType().getFormalParameters(); + JTypeMirror lastFormal = formals.get(formals.size() - 1); + JTypeMirror expectedComponent = ((JArrayType) lastFormal).getComponentType(); + + if (array.getTypeMirror().isSubtypeOf(expectedComponent) + && !array.getTypeMirror().equals(lastFormal)) { + // confusing + asCtx(data) + .addViolationWithMessage( + array, + "Unclear if a varargs or non-varargs call is intended. Cast to {0} or {0}[], or pass varargs parameters separately to clarify intent.", + TypePrettyPrint.prettyPrintWithSimpleNames(expectedComponent) + ); + } else if (array.getArrayInitializer() != null) { + // just regular unnecessary + asCtx(data).addViolation(array); + } + } + + 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 31fc4fea2e..19beaf3d79 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1435,6 +1435,61 @@ class Foo{ ]]> + + + Reports explicit array creation when a varargs is expected. + For instance: + ```java + Arrays.asList(new String[] { "foo", "bar", }); + ``` + can be replaced by: + ```java + Arrays.asList("foo", "bar"); + ``` + + This rule also reports such array creations when they are confusing, because the array is a subtype of the component type of the expected array type. + For instance if you have + ```java + void varargs(Object... parm); + ``` + and call it like so + ```java + varargs(new String[]{"a"}); + ``` + it is not clear whether you intended the method to receive the value `new Object[]{ new String[] {"a"} }` or just `new String[] {"a"}` (the latter happens). This confusion occurs because `String[]` is both a subtype of `Object[]` and of `Object`. To clarify your intent in this case, use a cast or pass individual elements like so: + ```java + // varargs call + // parm will be `new Object[] { "a" }` + varargs("a"); + + // non-varargs call + // parm will be `new String[] { "a" }` + varargs((Object[]) new String[]{"a"}); + + // varargs call + // parm will be `new Object[] { new String[] { "a" } }` + varargs((Object) new String[]{"a"}); + ``` + + 3 + + + + + + + Unnecessary in asList + 2 + 6,7 + + + + + Necessary array creation + 0 + + + + + + Confusing argument + 2 + + Unnecessary explicit varargs array creation + Unclear if a varargs or non-varargs call is intended. Cast to Object or Object[], or pass varargs parameters separately to clarify intent. + + + + + + + Array creation without elements + 0 + + + +