Merge branch 'pr/3661' into 7.0.x

.ci/files/all-java.xml

@@ -184,7 +184,7 @@
     <!-- <rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingMethodName"/> -->
     <!-- <rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingTypeName"/> -->
     <rule ref="category/java/errorprone.xml/AvoidInstanceofChecksInCatchClause"/>
-    <!-- <rule ref="category/java/errorprone.xml/AvoidLiteralsInIfCondition"/> -->
+    <rule ref="category/java/errorprone.xml/AvoidLiteralsInIfCondition"/>
     <!-- <rule ref="category/java/errorprone.xml/AvoidLosingExceptionInformation"/> -->
     <rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators"/>
     <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/>

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java

@@ -4,6 +4,10 @@
 
 package net.sourceforge.pmd.lang.java.ast;
 
+import java.util.Set;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * A unary operator, either prefix or postfix. This is used by {@link ASTUnaryExpression UnaryExpression}
  * to abstract over the syntactic form of the operator.
@@ -96,4 +100,25 @@ public enum UnaryOp implements InternalInterfaces.OperatorLike {
         return this.code;
     }
 
+    /**
+     * Tests if the node is an {@link ASTUnaryExpression} with one of the given operators.
+     */
+    public static boolean isUnaryExprWithOperator(@Nullable JavaNode e, Set<UnaryOp> operators) {
+        if (e instanceof ASTUnaryExpression) {
+            ASTUnaryExpression unary = (ASTUnaryExpression) e;
+            return operators.contains(unary.getOperator());
+        }
+        return false;
+    }
+
+    /**
+     * Tests if the node is an {@link ASTUnaryExpression} with the given operator.
+     */
+    public static boolean isUnaryExprWithOperator(@Nullable JavaNode e, UnaryOp operator) {
+        if (e instanceof ASTUnaryExpression) {
+            ASTUnaryExpression unary = (ASTUnaryExpression) e;
+            return operator == unary.getOperator();
+        }
+        return false;
+    }
 }

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

@@ -451,7 +451,7 @@ try {  // Prefer this:
     <rule name="AvoidLiteralsInIfCondition"
           language="java"
           since="4.2.6"
-          message="Avoid using Literals in Conditional Statements"
+          message="Avoid using literals in if statements"
           class="net.sourceforge.pmd.lang.rule.XPathRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#avoidliteralsinifcondition">
         <description>
@@ -474,28 +474,28 @@ the property ignoreMagicNumbers is not taken into account, if there are multiple
             <property name="xpath">
                 <value>
 <![CDATA[
-(: simple case - no deep expressions :)
-//IfStatement[$ignoreExpressions = true()]/Expression/*/PrimaryExpression/PrimaryPrefix/Literal
-    [not(NullLiteral)]
-    [not(BooleanLiteral)]
+(: simple case - no deep expressions - this is always executed :)
+//IfStatement/*[1]/*[pmd-java:nodeIs('Literal')]
+    [not(pmd-java:nodeIs('NullLiteral'))]
+    [not(pmd-java:nodeIs('BooleanLiteral'))]
     [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), @Image))]
 |
 (: consider also deeper expressions :)
-//IfStatement[$ignoreExpressions = false()]/Expression//*[local-name() != 'UnaryExpression' or @Operator != '-']/PrimaryExpression/PrimaryPrefix/Literal
-    [not(NullLiteral)]
-    [not(BooleanLiteral)]
+//IfStatement[$ignoreExpressions = false()]/*[1]//*[not(self::UnaryExpression[@Operator = '-'])]/*[pmd-java:nodeIs('Literal')]
+    [not(pmd-java:nodeIs('NullLiteral'))]
+    [not(pmd-java:nodeIs('BooleanLiteral'))]
     [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), @Image))]
 |
 (: consider negative literals :)
-//IfStatement[$ignoreExpressions = false()]/Expression//UnaryExpression[@Operator = '-']/PrimaryExpression/PrimaryPrefix/Literal
-    [not(NullLiteral)]
-    [not(BooleanLiteral)]
+//IfStatement[$ignoreExpressions = false()]/*[1]//UnaryExpression[@Operator = '-']/*[pmd-java:nodeIs('Literal')]
+    [not(pmd-java:nodeIs('NullLiteral'))]
+    [not(pmd-java:nodeIs('BooleanLiteral'))]
     [empty(index-of(tokenize($ignoreMagicNumbers, '\s*,\s*'), concat('-', @Image)))]
 |
 (: consider multiple literals in expressions :)
-//IfStatement[$ignoreExpressions = false()]/Expression[count(*/PrimaryExpression/PrimaryPrefix/Literal
-    [not(NullLiteral)]
-    [not(BooleanLiteral)]) > 1]
+//IfStatement[$ignoreExpressions = false()]/*[1][count(*[pmd-java:nodeIs('Literal')]
+    [not(pmd-java:nodeIs('NullLiteral'))]
+    [not(pmd-java:nodeIs('BooleanLiteral'))]) > 1]
 ]]>
                 </value>
             </property>

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidLiteralsInIfConditionTest.java

@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
 
 import net.sourceforge.pmd.testframework.PmdRuleTst;
 
-@org.junit.Ignore("Rule has not been updated yet")
 public class AvoidLiteralsInIfConditionTest extends PmdRuleTst {
     // no additional unit tests
 }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidLiteralsInIfCondition.xml

@@ -7,6 +7,7 @@
     <test-code>
         <description>basic test</description>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>3</expected-linenumbers>
         <code><![CDATA[
 public class PrimitiveType {
     public void downCastPrimitiveType() {
@@ -49,10 +50,14 @@ public class MyClass {
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
-    public void bar() {
+    public void bar(String aString) {
         if( ( flags & Flag.IMPORTANT ) != 0 ) {}
         if (aString.indexOf(DOT) != -1) {}     // magic number -1, by default ignored
     }
+    private static final String DOT = ".";
+    public static class Flag {
+        public static final int IMPORTANT = 0x1;
+    }
 }
         ]]></code>
     </test-code>
@@ -63,7 +68,7 @@ public class Foo {
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
-    public void bar() {
+    public void bar(double aDouble) {
         if (aDouble > 0.0) {}                  // magic number 0.0
         if (aDouble >= Double.MIN_VALUE) {}    // preferred approach
     }
@@ -76,7 +81,7 @@ public class Foo {
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 public class Foo {
-    public void bar() {
+    public void bar(double aDouble) {
         if (true && aDouble > 0) {
         }
     }
@@ -90,11 +95,12 @@ public class Foo {
         <expected-problems>0</expected-problems>
         <code><![CDATA[
 class Foo {
-    void bar() {
+    void bar(double num) {
         if (num == 0.0) {
-            return MathExtItg.sgn0raw(num) == 1 ? IEEEclass.PositiveZero : IEEEclass.NegativeZero;
+            return Math.signum(num) == 1 ? IEEEclass.PositiveZero : IEEEclass.NegativeZero;
         }
     }
+    public static enum IEEEclass { PositiveZero, NegativeZero; }
 }
         ]]></code>
     </test-code>
@@ -124,7 +130,7 @@ public class Foo {
         <expected-linenumbers>4,7,7,8,9,10,11,11</expected-linenumbers>
         <code><![CDATA[
 public class AvoidLiteralsInIfConditionWithExpressions {
-    public void test() {
+    public void test(String currentToken, int bodyStart, String s) {
         if (1) {}    // ok, "1" is in ignoreMagicNumbers
         if (1+1) {}  // not ok! multiple literals in expression
         if (a+1) {}  // ok, single literal, whitelisted