[java] Convert PositionLiteralsFirstIn*Comparisons to Java rules

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

@@ -0,0 +1,119 @@
+/*
+ * 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.lang.ast.Node;
+import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
+import net.sourceforge.pmd.lang.java.ast.ASTArguments;
+import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
+import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
+import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
+import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+
+class AbstractPositionLiteralsFirstInComparisons extends AbstractJavaRule {
+
+    private final String equalsImage;
+
+    AbstractPositionLiteralsFirstInComparisons(String equalsImage) {
+        addRuleChainVisit(ASTPrimaryExpression.class);
+        this.equalsImage = equalsImage;
+    }
+
+    @Override
+    public Object visit(ASTPrimaryExpression node, Object data) {
+        ASTPrimaryPrefix primaryPrefix = node.getFirstChildOfType(ASTPrimaryPrefix.class);
+        ASTPrimarySuffix primarySuffix = node.getFirstChildOfType(ASTPrimarySuffix.class);
+        if (primaryPrefix != null && primarySuffix != null) {
+            ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class);
+            if (name == null || !name.getImage().endsWith(equalsImage)) {
+                return data;
+            }
+            if (!isSingleStringLiteralArgument(primarySuffix)) {
+                return data;
+            }
+            if (isWithinNullComparison(node)) {
+                return data;
+            }
+            addViolation(data, node);
+        }
+        return node;
+    }
+
+    private boolean isWithinNullComparison(ASTPrimaryExpression node) {
+        for (ASTExpression parentExpr : node.getParentsOfType(ASTExpression.class)) {
+            if (isComparisonWithNull(parentExpr, "==", ASTConditionalOrExpression.class)
+                    || isComparisonWithNull(parentExpr, "!=", ASTConditionalAndExpression.class)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /*
+     * Expression/ConditionalAndExpression//EqualityExpression(@Image='!=']//NullLiteral
+     * Expression/ConditionalOrExpression//EqualityExpression(@Image='==']//NullLiteral
+     */
+    private boolean isComparisonWithNull(ASTExpression parentExpr, String equalOperator, Class<? extends JavaNode> condition) {
+        Node condExpr = null;
+        ASTEqualityExpression eqExpr = null;
+        if (parentExpr != null) {
+            condExpr = parentExpr.getFirstChildOfType(condition);
+        }
+        if (condExpr != null) {
+            eqExpr = condExpr.getFirstDescendantOfType(ASTEqualityExpression.class);
+        }
+        if (eqExpr != null) {
+            return eqExpr.hasImageEqualTo(equalOperator) && eqExpr.hasDescendantOfType(ASTNullLiteral.class);
+        }
+        return false;
+    }
+
+    /*
+     * This corresponds to the following XPath expression:
+     * (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()])
+     *       and
+     * ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
+     */
+    private boolean isSingleStringLiteralArgument(ASTPrimarySuffix primarySuffix) {
+        if (!primarySuffix.isArguments() || primarySuffix.getArgumentCount() != 1) {
+            return false;
+        }
+        Node node = primarySuffix;
+        node = node.getFirstChildOfType(ASTArguments.class);
+        if (node != null) {
+            node = node.getFirstChildOfType(ASTArgumentList.class);
+            if (node.getNumChildren() != 1) {
+                return false;
+            }
+        }
+        if (node != null) {
+            node = node.getFirstChildOfType(ASTExpression.class);
+        }
+        if (node != null) {
+            node = node.getFirstChildOfType(ASTPrimaryExpression.class);
+        }
+        if (node != null) {
+            node = node.getFirstChildOfType(ASTPrimaryPrefix.class);
+        }
+        if (node != null) {
+            node = node.getFirstChildOfType(ASTLiteral.class);
+        }
+        if (node != null) {
+            ASTLiteral literal = (ASTLiteral) node;
+            if (literal.isStringLiteral()) {
+                return true;
+            }
+        }
+        return false;
+    }
+}

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

@@ -0,0 +1,13 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+public class PositionLiteralsFirstInCaseInsensitiveComparisonsRule extends AbstractPositionLiteralsFirstInComparisons {
+
+    public PositionLiteralsFirstInCaseInsensitiveComparisonsRule() {
+        super(".equalsIgnoreCase");
+    }
+
+}

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

@@ -0,0 +1,13 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+public class PositionLiteralsFirstInComparisonsRule extends AbstractPositionLiteralsFirstInComparisons {
+
+    public PositionLiteralsFirstInComparisonsRule() {
+        super(".equals");
+    }
+
+}

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

@@ -1029,36 +1029,13 @@ String name,
           language="java"
           since="5.1"
           message="Position literals first in String comparisons for EqualsIgnoreCase"
-          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          class="net.sourceforge.pmd.lang.java.rule.bestpractices.PositionLiteralsFirstInCaseInsensitiveComparisonsRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#positionliteralsfirstincaseinsensitivecomparisons">
         <description>
 Position literals first in comparisons, if the second argument is null then NullPointerExceptions
 can be avoided, they will just return false.
         </description>
         <priority>3</priority>
-        <properties>
-            <property name="version" value="2.0"/>
-            <property name="xpath">
-                <value>
-<![CDATA[
-//PrimaryExpression[
-        PrimaryPrefix[Name
-                [
-    (ends-with(@Image, '.equalsIgnoreCase'))
-                ]
-        ]
-        [
-                   (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
-    and
-    ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
-        ]
-]
-[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
-[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
-]]>
-                </value>
-            </property>
-        </properties>
         <example>
 <![CDATA[
 class Foo {
@@ -1074,32 +1051,13 @@ class Foo {
           language="java"
           since="3.3"
           message="Position literals first in String comparisons"
-          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          class="net.sourceforge.pmd.lang.java.rule.bestpractices.PositionLiteralsFirstInComparisonsRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#positionliteralsfirstincomparisons">
         <description>
 Position literals first in comparisons, if the second argument is null then NullPointerExceptions
 can be avoided, they will just return false.
         </description>
         <priority>3</priority>
-        <properties>
-            <property name="version" value="2.0"/>
-            <property name="xpath">
-                <value>
-<![CDATA[
-//PrimaryExpression[
-    PrimaryPrefix[Name[(ends-with(@Image, '.equals'))]]
-        [
-            (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()])
-            and
-            ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
-        ]
-]
-[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
-[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
-]]>
-                </value>
-            </property>
-        </properties>
         <example>
 <![CDATA[
 class Foo {

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

@@ -40,6 +40,15 @@ public class Foo {
   if((str == null) || (str.equals(""))) {
    str = "snafu";
   }
+  if(str == null || str.equals("")) {
+   str = "snafu";
+  }
+  if((str != null) && (str.equals(""))) {
+   str = "snafu";
+  }
+  if(str != null && str.equals("")) {
+   str = "snafu";
+  }
  }
 }
      ]]></code>