From 5541a0397c913ec3f842fbb85732e5d8a9f0af32 Mon Sep 17 00:00:00 2001 From: Tom Copeland Date: Thu, 25 Oct 2007 13:31:36 +0000 Subject: [PATCH] Minor refactoring git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@5583 51baf565-9d33-0410-a72c-fc3788e3496d --- .../pmd/rules/design/NullAssignmentRule.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java b/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java index 8bedba630a..55c1f67056 100644 --- a/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java +++ b/pmd/src/net/sourceforge/pmd/rules/design/NullAssignmentRule.java @@ -12,11 +12,13 @@ import net.sourceforge.pmd.ast.ASTNullLiteral; import net.sourceforge.pmd.ast.ASTStatementExpression; import net.sourceforge.pmd.symboltable.VariableNameDeclaration; -// Would this be simplified by using DFA somehow? - +// TODO - should check that this is not the first assignment. e.g., this is OK: +// Object x; +// x = null; public class NullAssignmentRule extends AbstractRule { public Object visit(ASTNullLiteral node, Object data) { + if (node.getNthParent(5) instanceof ASTStatementExpression) { ASTStatementExpression n = (ASTStatementExpression) node.getNthParent(5); @@ -28,9 +30,13 @@ public class NullAssignmentRule extends AbstractRule { addViolation(data, node); } } else if (node.getNthParent(4) instanceof ASTConditionalExpression) { - checkTernary((ASTConditionalExpression) node.getNthParent(4), data, node); + if (isBadTernary((ASTConditionalExpression)node.getNthParent(4))) { + addViolation(data, node); + } } else if (node.getNthParent(5) instanceof ASTConditionalExpression) { - checkTernary((ASTConditionalExpression) node.getNthParent(5), data, node); + if (isBadTernary((ASTConditionalExpression)node.getNthParent(5))) { + addViolation(data, node); + } } return data; @@ -43,9 +49,7 @@ public class NullAssignmentRule extends AbstractRule { && ((VariableNameDeclaration) name.getNameDeclaration()).getAccessNodeParent().isFinal(); } - private void checkTernary(ASTConditionalExpression n, Object data, ASTNullLiteral node) { - if (n.isTernary() && !(n.jjtGetChild(0) instanceof ASTEqualityExpression)) { - addViolation(data, node); - } + private boolean isBadTernary(ASTConditionalExpression n) { + return n.isTernary() && !(n.jjtGetChild(0) instanceof ASTEqualityExpression); } }