From c85154adfcc678a5fc8e3e166dc7330b8c93438a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= <clement.fournier76@gmail.com>
Date: Tue, 8 Feb 2022 19:41:01 +0100
Subject: [PATCH] Fix #278 - make ConfusingTernary treat != null as a positive
 condition

update release notes
---
 docs/pages/release_notes.md                   |  3 ++
 .../rule/codestyle/ConfusingTernaryRule.java  | 29 ++++++++++++++++---
 .../rule/codestyle/xml/ConfusingTernary.xml   | 27 +++++++++++++++++
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index b8f8783555..e684296b5f 100644
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release.
 
 ### Fixed Issues
 
+*   java-codestyle
+    *   [#278](https://github.com/pmd/pmd/issues/278): \[java] ConfusingTernary should treat `!= null` as positive condition
+
 ### API Changes
 
 ### External Contributions
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java
index b0ef3262e1..e4c63b6df2 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java
@@ -6,13 +6,14 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
 
 import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
 
-import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
 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.ASTIfStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
+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.ASTUnaryExpressionNotPlusMinus;
@@ -55,6 +56,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
  * </pre>
  */
 public class ConfusingTernaryRule extends AbstractJavaRule {
+
     private static PropertyDescriptor<Boolean> ignoreElseIfProperty = booleanProperty("ignoreElseIf").desc("Ignore conditions with an else-if case").defaultValue(false).build();
 
     public ConfusingTernaryRule() {
@@ -100,14 +102,33 @@ public class ConfusingTernaryRule extends AbstractJavaRule {
         return isUnaryNot(node) || isNotEquals(node) || isConditionalWithAllMatches(node);
     }
 
-    private static boolean isUnaryNot(Node node) {
+    private static boolean isUnaryNot(JavaNode node) {
         // look for "!x"
         return node instanceof ASTUnaryExpressionNotPlusMinus && "!".equals(node.getImage());
     }
 
-    private static boolean isNotEquals(Node node) {
+    private static boolean isNotEquals(JavaNode node) {
         // look for "x != y"
-        return node instanceof ASTEqualityExpression && "!=".equals(node.getImage());
+        return node instanceof ASTEqualityExpression && "!=".equals(node.getImage())
+            && !isNullLiteral(node.getChild(0))
+            && !isNullLiteral(node.getChild(1));
+    }
+
+    private static boolean isNullLiteral(JavaNode node) {
+        node = unwrapParentheses(node);
+        if (node instanceof ASTExpression && node.getNumChildren() == 1) {
+            node = node.getChild(0);
+        }
+        if (node instanceof ASTPrimaryExpression && node.getNumChildren() == 1) {
+            node = node.getChild(0);
+            if (node instanceof ASTPrimaryPrefix && node.getNumChildren() == 1) {
+                node = node.getChild(0);
+                if (node instanceof ASTLiteral && node.getNumChildren() == 1) {
+                    return node.getChild(0) instanceof ASTNullLiteral;
+                }
+            }
+        }
+        return false;
     }
 
     private static boolean isConditionalWithAllMatches(JavaNode node) {
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/ConfusingTernary.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/ConfusingTernary.xml
index 33f2e8be45..0ddd0a2a5c 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/ConfusingTernary.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/ConfusingTernary.xml
@@ -28,6 +28,33 @@ public class Foo {
         ]]></code>
     </test-code>
 
+    <test-code>
+        <description>null check, ok (if) - #278</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    String bar2(Object a) {
+        if (a != null)
+            return a.toString();
+        else
+            return null;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>null check, ok (ternary) - #278</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    String bar(Object a) {
+        return a != null ? a.toString() : null;
+    }
+}
+        ]]></code>
+    </test-code>
+
     <test-code>
         <description>!= inside if, bad</description>
         <expected-problems>2</expected-problems>