From 171fdb877fa5cfc3225a2b8bbc830bf0ce34041f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 9 May 2015 18:25:12 +0200 Subject: [PATCH] #1345 UseCollectionIsEmpty throws NullPointerException --- .../rule/AbstractInefficientZeroCheck.java | 29 ++++++++++++++++--- .../rule/design/xml/UseCollectionIsEmpty.xml | 23 +++++++++++++++ src/site/markdown/overview/changelog.md | 1 + 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractInefficientZeroCheck.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractInefficientZeroCheck.java index 819139ac85..157a12dc90 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractInefficientZeroCheck.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractInefficientZeroCheck.java @@ -104,14 +104,32 @@ public abstract class AbstractInefficientZeroCheck extends AbstractJavaRule { private boolean isCompare(Node equality) { if (isLiteralLeftHand(equality)) { return checkComparison(inverse.get(equality.getImage()), equality, 0); - } else { + } else if (isLiteralRightHand(equality)) { return checkComparison(equality.getImage(), equality, 1); } + return false; } private boolean isLiteralLeftHand(Node equality) { - return equality.jjtGetChild(0).jjtGetChild(0).jjtGetNumChildren() > 0 - && equality.jjtGetChild(0).jjtGetChild(0).jjtGetChild(0) instanceof ASTLiteral; + return isLiteral(equality, 0); + } + + private boolean isLiteralRightHand(Node equality) { + return isLiteral(equality, 1); + } + + private boolean isLiteral(Node equality, int child) { + Node target = equality.jjtGetChild(child); + target = getFirstChildOrThis(target); + target = getFirstChildOrThis(target); + return target instanceof ASTLiteral; + } + + private Node getFirstChildOrThis(Node node) { + if (node.jjtGetNumChildren() > 0) { + return node.jjtGetChild(0); + } + return node; } /** @@ -126,7 +144,10 @@ public abstract class AbstractInefficientZeroCheck extends AbstractJavaRule { * @see #getComparisonTargets() */ private boolean checkComparison(String operator, Node equality, int i) { - Node target = equality.jjtGetChild(i).jjtGetChild(0).jjtGetChild(0); + Node target = equality + .jjtGetChild(i) + .jjtGetChild(0) + .jjtGetChild(0); return target instanceof ASTLiteral && getComparisonTargets().get(operator).contains(target.getImage()); } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseCollectionIsEmpty.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseCollectionIsEmpty.xml index 774870f61d..44af72204d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseCollectionIsEmpty.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseCollectionIsEmpty.xml @@ -291,6 +291,29 @@ public class PMDIsEmptyFalsePositive { // do something } } +} + ]]> + + + #1345 UseCollectionIsEmpty throws NullPointerException + 0 + list) { + if (list.size() < this.getSize()) { + throw new IllegalArgumentException(); + } + } } ]]> diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 63f13373e4..94599d2433 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -14,5 +14,6 @@ * [#1335](https://sourceforge.net/p/pmd/bugs/1335/): GuardLogStatementJavaUtil should not apply to SLF4J Logger * [#1342](https://sourceforge.net/p/pmd/bugs/1342/): UseConcurrentHashMap false positive (with documentation example) +* [#1345](https://sourceforge.net/p/pmd/bugs/1345/): UseCollectionIsEmpty throws NullPointerException **API Changes:**