Handle [string literal].equals() correctly in EqualsAvoidNullRule

This fixes alibaba#471 Previously, the following code can't pass EqualsAvoidNullRule: public class Test { private static final String VERSION = System.getProperty("v"); public boolean isJava6(){ return "1.6".equals(VERSION); } } This PR fixes this issue by checking if the caller is a literal.
robinluob · Mar 29, 2019 · 1dc88f5 · 1dc88f5
1 parent 3cf2594
commit 1dc88f5
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/EqualsAvoidNullRule.java b/p3c-pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/oop/EqualsAvoidNullRule.java
@@ -26,6 +26,7 @@
 import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
 import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
 import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode;
@@ -61,6 +62,11 @@ public Object visit(ASTCompilationUnit node, Object data) {
                 return super.visit(node, data);
             }
             for (Node invocation : equalsInvocations) {
+                // https://github.com/alibaba/p3c/issues/471
+                if (callerIsLiteral(invocation)) {
+                    return super.visit(node, data);
+                }
+
                 // if arguments of equals is complicate expression, skip the check
                 List<? extends Node> simpleExpressions = invocation.findChildNodesWithXPath(INVOCATION_PREFIX_XPATH);
                 if (simpleExpressions == null || simpleExpressions.isEmpty()) {
@@ -98,6 +104,14 @@ public Object visit(ASTCompilationUnit node, Object data) {
         return super.visit(node, data);
     }
 
+    private boolean callerIsLiteral(Node equalsInvocation) {
+        if (equalsInvocation instanceof ASTPrimaryExpression) {
+            ASTPrimaryPrefix caller = equalsInvocation.getFirstChildOfType(ASTPrimaryPrefix.class);
+            return caller != null && caller.getFirstChildOfType(ASTLiteral.class) != null;
+        }
+        return false;
+    }
+
     private String getInvocationName(AbstractJavaNode javaNode) {
         Token token = (Token)javaNode.jjtGetFirstToken();
         StringBuilder sb = new StringBuilder(token.image).append(token.image);

diff --git a/...pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/oop/xml/EqualsAvoidNullRule.xml b/...pmd/src/test/resources/com/alibaba/p3c/pmd/lang/java/rule/oop/xml/EqualsAvoidNullRule.xml
@@ -129,4 +129,41 @@
 		<expected-problems>0</expected-problems>
 		<code-ref id="caller-is-constant" />
 	</test-code>
+
+	<!-- ====================================================================== -->
+
+	<code-fragment id="string-literal-equals-constant">
+		<![CDATA[
+	public class Test {
+		private static final String VERSION = System.getProperty("v");
+		public boolean isJava6(){
+			return "1.6".equals(VERSION);
+		}
+	}
+		]]>
+	</code-fragment>
+	<test-code>
+		<description>string literal equals constant</description>
+		<expected-problems>0</expected-problems>
+		<code-ref id="string-literal-equals-constant" />
+	</test-code>
+
+	<!-- ====================================================================== -->
+
+	<code-fragment id="string-literal-equals-literal">
+		<![CDATA[
+	public class Test {
+		public boolean isJava6(){
+			return "1.6".equals("1.6");
+		}
+	}
+		]]>
+	</code-fragment>
+	<test-code>
+		<description>string literal equals string literal</description>
+		<expected-problems>0</expected-problems>
+		<code-ref id="string-literal-equals-literal" />
+	</test-code>
+
+	<!-- ====================================================================== -->
 </test-data>