[java] New rule ConfusingArgumentToVarargsMethod (#4971)

Merge pull request #4971 from oowekyala:new-rule-UnnecessaryVarargsArrayCreation
pmd · Apr 25, 2024 · d0870f3 · d0870f3
2 parents a154f78 + 04206ec
commit d0870f3
Show file tree

Hide file tree

Showing 33 changed files with 320 additions and 130 deletions.
diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
@@ -24,6 +24,9 @@ These improvements apply to all supported languages, irrespective of supported f
 
 - 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.
+- The new Java rule {%rule java/errorprone/ConfusingArgumentToVarargsMethod %} reports some confusing situations
+  where a varargs method is called with an inexact argument type. These may end up in a mismatch between the expected
+  parameter type and the actual value.
 
 - The new Java rule {% rule "java/codestyle/LambdaCanBeMethodReference" %} reports lambda expressions that can be replaced
   with a method reference. Please read the documentation of the rule for more info. This rule is now part of the Quickstart

diff --git a/...apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CognitiveComplexityRule.java b/...apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CognitiveComplexityRule.java
@@ -75,7 +75,7 @@ public Object visit(ASTUserClass node, Object data) {
                     String.valueOf(classLevelThreshold),
                 };
 
-                asCtx(data).addViolation(node, messageParams);
+                asCtx(data).addViolation(node, (Object[]) messageParams);
             }
         }
         return data;

diff --git a/...pex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CyclomaticComplexityRule.java b/...pex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/CyclomaticComplexityRule.java
@@ -78,7 +78,7 @@ public Object visit(ASTUserClass node, Object data) {
                                           " total",
                                           classWmc + " (highest " + classHighest + ")", };
 
-                asCtx(data).addViolation(node, messageParams);
+                asCtx(data).addViolation(node, (Object[]) messageParams);
             }
         }
         return data;

diff --git a/...n/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java b/...n/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java
@@ -76,7 +76,7 @@ static void checkMemberAccess(RuleContext ruleContext, JavaNode refExpr, JAccess
             JavaNode node = sym.tryGetNode();
             assert node != null : "Node should be in the same compilation unit";
             if (reportedNodes.add(node)) {
-                ruleContext.addViolation(node, new String[] {stripPackageName(refExpr.getEnclosingType().getSymbol())});
+                ruleContext.addViolation(node, stripPackageName(refExpr.getEnclosingType().getSymbol()));
             }
         }
     }

diff --git a/...a/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/MissingOverrideRule.java b/...a/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/MissingOverrideRule.java
@@ -24,7 +24,7 @@ public MissingOverrideRule() {
     @Override
     public Object visit(ASTMethodDeclaration node, Object data) {
         if (node.isOverridden() && !node.isAnnotationPresent(Override.class)) {
-            asCtx(data).addViolation(node, new Object[] { PrettyPrintingUtil.displaySignature(node) });
+            asCtx(data).addViolation(node, PrettyPrintingUtil.displaySignature(node));
         }
         return data;
     }

diff --git a/...net/sourceforge/pmd/lang/java/rule/bestpractices/UnnecessaryVarargsArrayCreationRule.java b/...net/sourceforge/pmd/lang/java/rule/bestpractices/UnnecessaryVarargsArrayCreationRule.java
@@ -11,10 +11,8 @@
 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 {
 
@@ -26,6 +24,9 @@ public UnnecessaryVarargsArrayCreationRule() {
 
     @Override
     public Object visit(ASTArrayAllocation array, Object data) {
+        if (array.getArrayInitializer() == null) {
+            return null;
+        }
 
         JavaNode parent = array.getParent();
         if (parent instanceof ASTArgumentList && array.getIndexInParent() == parent.getNumChildren() - 1) {
@@ -39,19 +40,10 @@ public Object visit(ASTArrayAllocation array, Object data) {
 
             List<JTypeMirror> 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
+
+            if (array.getTypeMirror().equals(lastFormal)) {
+                // If type not equal, then it would not actually be equivalent to remove the array creation.
+                // That case may be caught by ConfusingArgumentToVarargsMethod
                 asCtx(data).addViolation(array);
             }
         }

diff --git a/...main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java b/...main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
@@ -52,7 +52,7 @@ private void check(ASTExecutableDeclaration node, Object data) {
         for (ASTFormalParameter formal : node.getFormalParameters()) {
             ASTVariableId varId = formal.getVarId();
             if (JavaAstUtils.isNeverUsed(varId) && !JavaRuleUtil.isExplicitUnusedVarName(varId.getName())) {
-                asCtx(data).addViolation(varId, new Object[] { node instanceof ASTMethodDeclaration ? "method" : "constructor", varId.getName(), });
+                asCtx(data).addViolation(varId, node instanceof ASTMethodDeclaration ? "method" : "constructor", varId.getName());
             }
         }
     }

diff --git a/.../main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java b/.../main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java
@@ -60,11 +60,9 @@ RegexPropertyBuilder defaultProp(String name, String displayName) {
     void checkMatches(T node, PropertyDescriptor<Pattern> regex, Object data) {
         String name = nameExtractor(node);
         if (!getProperty(regex).matcher(name).matches()) {
-            asCtx(data).addViolation(node, new Object[]{
-                kindDisplayName(node, regex),
-                name,
-                getProperty(regex).toString(),
-            });
+            asCtx(data).addViolation(node, kindDisplayName(node, regex),
+                                     name,
+                                     getProperty(regex).toString());
         }
     }
 

diff --git a/...rc/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldNamingConventionsRule.java b/...rc/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldNamingConventionsRule.java
@@ -90,11 +90,9 @@ public Object visit(ASTEnumConstant node, Object data) {
         // This inlines checkMatches because there's no variable declarator id
 
         if (!getProperty(enumConstantRegex).matcher(node.getImage()).matches()) {
-            asCtx(data).addViolation(node, new Object[]{
-                "enum constant",
-                node.getImage(),
-                getProperty(enumConstantRegex).toString(),
-            });
+            asCtx(data).addViolation(node, "enum constant",
+                                     node.getImage(),
+                                     getProperty(enumConstantRegex).toString());
         }
 
         return data;

diff --git a/...rc/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java b/...rc/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java
@@ -91,7 +91,7 @@ public Object visit(ASTTryStatement node, Object data) {
                 // By convention, lower catch blocks are collapsed into the highest one
                 // The first node of the equivalence class is thus the block that should be transformed
                 for (int i = 1; i < identicalStmts.size(); i++) {
-                    asCtx(data).addViolation(identicalStmts.get(i), new String[]{identicalBranchName, });
+                    asCtx(data).addViolation(identicalStmts.get(i), identicalBranchName);
                 }
             }
         }

diff --git a/...java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LinguisticNamingRule.java b/...java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LinguisticNamingRule.java
@@ -116,7 +116,7 @@ private void checkPrefixedTransformMethods(ASTMethodDeclaration node, Object dat
                 && prefixes.contains(splitMethodName[0].toLowerCase(Locale.ROOT))) {
             // "To" or any other configured prefix found
             asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The transform method ''{0}'' should not return void linguistically",
-                    new Object[] { nameOfMethod });
+                                                nameOfMethod);
         }
     }
 
@@ -125,7 +125,7 @@ private void checkTransformMethods(ASTMethodDeclaration node, Object data, Strin
             if (node.isVoid() && containsCamelCaseWord(nameOfMethod, StringUtils.capitalize(infix))) {
                 // "To" or any other configured infix in the middle somewhere
                 asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The transform method ''{0}'' should not return void linguistically",
-                        new Object[] { nameOfMethod });
+                                                    nameOfMethod);
                 // the first violation is sufficient - it is still the same method we are analyzing here
                 break;
             }
@@ -135,14 +135,14 @@ private void checkTransformMethods(ASTMethodDeclaration node, Object data, Strin
     private void checkGetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
         if (startsWithCamelCaseWord(nameOfMethod, "get") && node.isVoid()) {
             asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The getter ''{0}'' should not return void linguistically",
-                    new Object[] { nameOfMethod });
+                                                nameOfMethod);
         }
     }
 
     private void checkSetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
         if (startsWithCamelCaseWord(nameOfMethod, "set") && !node.isVoid()) {
             asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The setter ''{0}'' should not return any type except void linguistically",
-                    new Object[] { nameOfMethod });
+                                                nameOfMethod);
         }
     }
 
@@ -158,7 +158,7 @@ private void checkBooleanMethods(ASTMethodDeclaration node, Object data, String
             for (String prefix : getProperty(BOOLEAN_METHOD_PREFIXES_PROPERTY)) {
                 if (startsWithCamelCaseWord(nameOfMethod, prefix) && !isBooleanType(t)) {
                     asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The method ''{0}'' indicates linguistically it returns a boolean, but it returns ''{1}''",
-                            new Object[] {nameOfMethod, PrettyPrintingUtil.prettyPrintType(t) });
+                                                        nameOfMethod, PrettyPrintingUtil.prettyPrintType(t));
                 }
             }
         }
@@ -168,7 +168,7 @@ private void checkField(ASTType typeNode, ASTVariableDeclarator node, Object dat
         for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
             if (startsWithCamelCaseWord(node.getName(), prefix) && !isBooleanType(typeNode)) {
                 asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The field ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
-                        new Object[] { node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode) });
+                                                    node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode));
             }
         }
     }
@@ -177,7 +177,7 @@ private void checkVariable(ASTType typeNode, ASTVariableDeclarator node, Object
         for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
             if (startsWithCamelCaseWord(node.getName(), prefix) && !isBooleanType(typeNode)) {
                 asCtx(data).addViolationWithMessage(node, "Linguistics Antipattern - The variable ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
-                        new Object[] { node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode) });
+                                                    node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode));
             }
         }
     }

diff --git a/.../java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java b/.../java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java
@@ -88,7 +88,7 @@ && methodProbablyMeansSame(methodCall)) {
                     // we don't actually know where the method came from
                     String simpleName = formatMemberName(next, methodCall.getMethodType().getSymbol());
                     String unnecessary = produceQualifier(deepest, next, true);
-                    asCtx(data).addViolation(next, new Object[] {unnecessary, simpleName, ""});
+                    asCtx(data).addViolation(next, unnecessary, simpleName, "");
                     return null;
                 }
             } else if (getProperty(REPORT_FIELDS) && opa instanceof ASTFieldAccess) {
@@ -98,7 +98,7 @@ && methodProbablyMeansSame(methodCall)) {
                     String simpleName = formatMemberName(next, fieldAccess.getReferencedSym());
                     String reasonToString = unnecessaryReasonWrapper(reasonForFieldInScope);
                     String unnecessary = produceQualifier(deepest, next, true);
-                    asCtx(data).addViolation(next, new Object[] {unnecessary, simpleName, reasonToString});
+                    asCtx(data).addViolation(next, unnecessary, simpleName, reasonToString);
                     return null;
                 }
             }
@@ -108,7 +108,7 @@ && methodProbablyMeansSame(methodCall)) {
             String simpleName = next.getSimpleName();
             String reasonToString = unnecessaryReasonWrapper(bestReason);
             String unnecessary = produceQualifier(deepest, next, false);
-            asCtx(data).addViolation(next, new Object[] {unnecessary, simpleName, reasonToString});
+            asCtx(data).addViolation(next, unnecessary, simpleName, reasonToString);
         }
         return null;
     }

diff --git a/...ava/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java b/...ava/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java
@@ -220,7 +220,7 @@ private void visitImport(ASTImportDeclaration node, Object data, String thisPack
     }
 
     private void reportWithMessage(ASTImportDeclaration node, Object data, String message) {
-        asCtx(data).addViolationWithMessage(node, message, new String[] { PrettyPrintingUtil.prettyImport(node) });
+        asCtx(data).addViolationWithMessage(node, message, PrettyPrintingUtil.prettyImport(node));
     }
 
     @Override

diff --git a/...a/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryModifierRule.java b/...a/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryModifierRule.java
@@ -54,12 +54,10 @@ private void reportUnnecessaryModifiers(Object data, JavaNode node,
         if (unnecessaryModifiers.isEmpty()) {
             return;
         }
-        asCtx(data).addViolation(node, new String[]{
-                formatUnnecessaryModifiers(unnecessaryModifiers),
-                PrettyPrintingUtil.getPrintableNodeKind(node),
-                PrettyPrintingUtil.getNodeName(node),
-                explanation.isEmpty() ? "" : ": " + explanation,
-        });
+        asCtx(data).addViolation(node, formatUnnecessaryModifiers(unnecessaryModifiers),
+                                 PrettyPrintingUtil.getPrintableNodeKind(node),
+                                 PrettyPrintingUtil.getNodeName(node),
+                                 explanation.isEmpty() ? "" : ": " + explanation);
     }
 
 

diff --git a/...va/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UseDiamondOperatorRule.java b/...va/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UseDiamondOperatorRule.java
@@ -85,7 +85,7 @@ public Object visit(ASTConstructorCall ctorCall, Object data) {
             JavaNode reportNode = targs == null ? newTypeNode : targs;
             String message = targs == null ? RAW_TYPE_MESSAGE : REPLACE_TYPE_ARGS_MESSAGE;
             String replaceWith = produceSuggestedExprImage(ctorCall);
-            asCtx(data).addViolationWithMessage(reportNode, message, new String[] { replaceWith });
+            asCtx(data).addViolationWithMessage(reportNode, message, replaceWith);
         }
         return null;
     }

diff --git a/...java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CognitiveComplexityRule.java b/...java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CognitiveComplexityRule.java
@@ -56,10 +56,10 @@ private Object visitMethod(ASTExecutableDeclaration node, Object data) {
         int cognitive = MetricsUtil.computeMetric(COGNITIVE_COMPLEXITY, node);
         final int reportLevel = getReportLevel();
         if (cognitive >= reportLevel) {
-            asCtx(data).addViolation(node, new String[] { node instanceof ASTMethodDeclaration ? "method" : "constructor",
-                                                    PrettyPrintingUtil.displaySignature(node),
-                                                    String.valueOf(cognitive),
-                                                    String.valueOf(reportLevel) });
+            asCtx(data).addViolation(node, node instanceof ASTMethodDeclaration ? "method" : "constructor",
+                                     PrettyPrintingUtil.displaySignature(node),
+                                     String.valueOf(cognitive),
+                                     String.valueOf(reportLevel));
         }
 
         return data;

diff --git a/...ava/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CyclomaticComplexityRule.java b/...ava/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CyclomaticComplexityRule.java
@@ -89,7 +89,7 @@ public Object visitTypeDecl(ASTTypeDeclaration node, Object data) {
                                           " total",
                                           classWmc + " (highest " + classHighest + ")", };
 
-                asCtx(data).addViolation(node, messageParams);
+                asCtx(data).addViolation(node, (Object[]) messageParams);
             }
         }
         return data;
@@ -120,11 +120,7 @@ private void visitMethodLike(ASTExecutableDeclaration node, Object data) {
 
                 String kindname = node instanceof ASTConstructorDeclaration ? "constructor" : "method";
 
-
-                asCtx(data).addViolation(node, new String[] {kindname,
-                                                             opname,
-                                                             "",
-                                                             "" + cyclo, });
+                asCtx(data).addViolation(node, kindname, opname, "", "" + cyclo);
             }
         }
     }

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/DataClassRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/DataClassRule.java
@@ -53,9 +53,9 @@ private void visitTypeDecl(ASTTypeDeclaration node, RuleContext data) {
             int noam = MetricsUtil.computeMetric(NUMBER_OF_ACCESSORS, node);
             int wmc = MetricsUtil.computeMetric(WEIGHED_METHOD_COUNT, node);
 
-            asCtx(data).addViolation(node, new Object[] {node.getSimpleName(),
-                                                         StringUtil.percentageString(woc, 3),
-                                                         nopa, noam, wmc, });
+            asCtx(data).addViolation(node, node.getSimpleName(),
+                                     StringUtil.percentageString(woc, 3),
+                                     nopa, noam, wmc);
         }
     }
 

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/GodClassRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/GodClassRule.java
@@ -59,9 +59,9 @@ public Object visit(ASTClassDeclaration node, Object data) {
 
         if (wmc >= WMC_VERY_HIGH && atfd > FEW_ATFD_THRESHOLD && tcc < TCC_THRESHOLD) {
 
-            asCtx(data).addViolation(node, new Object[] {wmc,
-                                                         StringUtil.percentageString(tcc, 3),
-                                                         atfd, });
+            asCtx(data).addViolation(node, wmc,
+                                     StringUtil.percentageString(tcc, 3),
+                                     atfd);
         }
         return data;
     }

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java
@@ -132,11 +132,9 @@ public Object visit(ASTFieldAccess node, Object data) {
             asCtx(data).addViolationWithMessage(
                 node,
                 FIELD_ACCESS_ON_FOREIGN_VALUE,
-                new Object[] {
-                    node.getName(),
-                    PrettyPrintingUtil.prettyPrint(node.getQualifier()),
-                    foreignDegree(node.getQualifier()),
-                });
+                node.getName(),
+                PrettyPrintingUtil.prettyPrint(node.getQualifier()),
+                foreignDegree(node.getQualifier()));
         }
         return null;
     }
@@ -147,11 +145,9 @@ public Object visit(ASTMethodCall node, Object data) {
             asCtx(data).addViolationWithMessage(
                 node,
                 METHOD_CALL_ON_FOREIGN_VALUE,
-                new Object[] {
-                    node.getMethodName(),
-                    PrettyPrintingUtil.prettyPrint(node.getQualifier()),
-                    foreignDegree(node.getQualifier()),
-                });
+                node.getMethodName(),
+                PrettyPrintingUtil.prettyPrint(node.getQualifier()),
+                foreignDegree(node.getQualifier()));
         }
         return null;
     }