From 5f2d71cddec8be00196d34138701ec350b531656 Mon Sep 17 00:00:00 2001
From: Xavier Le Vourch <xlv@users.sourceforge.net>
Date: Thu, 28 Aug 2008 01:56:07 +0000
Subject: [PATCH] Fixed bug 1556594 - Wonky detection of NullAssignment

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6418 51baf565-9d33-0410-a72c-fc3788e3496d
---
 pmd/etc/changelog.txt                             |  1 +
 .../rules/controversial/xml/NullAssignment.xml    | 15 ++++++++++++++-
 .../pmd/rules/design/NullAssignmentRule.java      |  5 ++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt
index 59bd496044..53aa90c17b 100644
--- a/pmd/etc/changelog.txt
+++ b/pmd/etc/changelog.txt
@@ -13,6 +13,7 @@ Fixed bug 2002722 - false + in UseStringBufferForStringAppends
 Fixed bug 2056318 - False positive for AvoidInstantiatingObjectsInLoops
 Fixed bug 1977438 - False positive for UselessStringValueOf
 Fixed bug 2050064 - False + SuspiciousOctalEscape with backslash literal
+Fixed bug 1556594 - Wonky detection of NullAssignment
 Optimizations and false positive fixes in PreserveStackTrace
 @SuppressWarnings("all") disables all warnings
 All comment types are now stored in ASTCompilationUnit, not just formal ones
diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/controversial/xml/NullAssignment.xml b/pmd/regress/test/net/sourceforge/pmd/rules/controversial/xml/NullAssignment.xml
index 5094ec04a9..7e6c02975b 100644
--- a/pmd/regress/test/net/sourceforge/pmd/rules/controversial/xml/NullAssignment.xml
+++ b/pmd/regress/test/net/sourceforge/pmd/rules/controversial/xml/NullAssignment.xml
@@ -113,4 +113,17 @@ public class Foo {
 }
      ]]></code>
     </test-code>
-</test-data>
\ No newline at end of file
+    <test-code>
+        <description><![CDATA[
+1556594 - Wonky detection of NullAssignment
+     ]]></description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ public void foo() {
+  assert isRoot() ? parentContext == null : parentContext != null;
+ }
+}
+     ]]></code>
+    </test-code>
+</test-data>
diff --git a/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java b/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java
index 55c1f67056..e9e0cd6514 100644
--- a/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java
+++ b/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java
@@ -7,6 +7,7 @@ import net.sourceforge.pmd.AbstractRule;
 import net.sourceforge.pmd.ast.ASTAssignmentOperator;
 import net.sourceforge.pmd.ast.ASTConditionalExpression;
 import net.sourceforge.pmd.ast.ASTEqualityExpression;
+import net.sourceforge.pmd.ast.ASTExpression;
 import net.sourceforge.pmd.ast.ASTName;
 import net.sourceforge.pmd.ast.ASTNullLiteral;
 import net.sourceforge.pmd.ast.ASTStatementExpression;
@@ -30,10 +31,12 @@ public class NullAssignmentRule extends AbstractRule {
                 addViolation(data, node);
             }
         } else if (node.getNthParent(4) instanceof ASTConditionalExpression) {
+            // "false" expression of ternary
             if (isBadTernary((ASTConditionalExpression)node.getNthParent(4))) {
                 addViolation(data, node);
             }
-        } else if (node.getNthParent(5) instanceof ASTConditionalExpression) {
+        } else if (node.getNthParent(5) instanceof ASTConditionalExpression && node.getNthParent(4) instanceof ASTExpression) {
+            // "true" expression of ternary
             if (isBadTernary((ASTConditionalExpression)node.getNthParent(5))) {
                 addViolation(data, node);
             }