From dc453cf103787e06a70385cb4eab1917f6286446 Mon Sep 17 00:00:00 2001 From: Tom Copeland Date: Mon, 25 Jul 2005 13:19:33 +0000 Subject: [PATCH] Fixed bug 1242544 - SimplifyConditional no longer flags null checks that precede an instanceof involving an array dereference. git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@3728 51baf565-9d33-0410-a72c-fc3788e3496d --- pmd/etc/changelog.txt | 3 ++- .../pmd/ast/ASTPrimarySuffixTest.java | 2 +- .../pmd/rules/SimplifyConditionalTest.java | 8 ++++++++ pmd/rulesets/design.xml | 2 +- .../sourceforge/pmd/ast/ASTPrimarySuffix.java | 17 +++++++++++++---- .../pmd/jaxen/AttributeAxisIterator.java | 3 +-- .../pmd/rules/IdempotentOperations.java | 4 ++-- pmd/xdocs/credits.xml | 2 +- 8 files changed, 29 insertions(+), 12 deletions(-) diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 9b2cbee2d2..8b722413dc 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -14,7 +14,8 @@ Fixed bug 1228589 - DoubleCheckedLocking and ExceptionSignatureDeclaration no lo Fixed bug 1235299 - NullAssignment no longer flags null equality comparisons in ternary expressions. Fixed bug 1235300 - NullAssignment no longer flags assignments to final fields. Fixed bug 1240201 - The UnnecessaryParentheses message is no longer restricted to return statements. -Fixed bug 1242290 - The JDK 1.5 parser no longer chokes on nested enumerations with a constructor. +Fixed bug 1242290 - The JDK 1.5 parser no longer chokes on nested enumerations with a constructor. +Fixed bug 1242544 - SimplifyConditional no longer flags null checks that precede an instanceof involving an array dereference. Applied patch 1228834 - XPath rules can now use properties to customize rules. Thanks to Wouter Zelle for another great piece of work! Fixed a bug in RuleSetFactory that missed some override cases; thx to Wouter Zelle for the report and a fix. Fixed a bug in the grammar that didn't allow constructors to have type parameters, which couldn't parse some JDK 1.5 constructs. diff --git a/pmd/regress/test/net/sourceforge/pmd/ast/ASTPrimarySuffixTest.java b/pmd/regress/test/net/sourceforge/pmd/ast/ASTPrimarySuffixTest.java index fcd902b4b1..49817260b3 100644 --- a/pmd/regress/test/net/sourceforge/pmd/ast/ASTPrimarySuffixTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/ast/ASTPrimarySuffixTest.java @@ -10,7 +10,7 @@ public class ASTPrimarySuffixTest extends ParserTst { public void testArrayDereference() throws Throwable { Set ops = getNodes(ASTPrimarySuffix.class, TEST1); - assertTrue(((ASTPrimarySuffix)(ops.iterator().next())).isArrayDeference()); + assertTrue(((ASTPrimarySuffix)(ops.iterator().next())).isArrayDereference()); } public void testArguments() throws Throwable { diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/SimplifyConditionalTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/SimplifyConditionalTest.java index 2e2d334353..299abd0913 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/SimplifyConditionalTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/rules/SimplifyConditionalTest.java @@ -19,6 +19,7 @@ public class SimplifyConditionalTest extends SimpleAggregatorTst{ new TestDescriptor(TEST2, "ok", 0, rule), new TestDescriptor(TEST3, "transpose x and null, still bad", 1, rule), new TestDescriptor(TEST4, "conditional or and !(instanceof)", 1, rule), + new TestDescriptor(TEST5, "indexing into array is ok", 0, rule), }); } @@ -50,4 +51,11 @@ public class SimplifyConditionalTest extends SimpleAggregatorTst{ " }" + PMD.EOL + "}"; + private static final String TEST5 = + "public class Foo {" + PMD.EOL + + " void bar(Object x) {" + PMD.EOL + + " if (x != null && x[0] instanceof String) {}" + PMD.EOL + + " }" + PMD.EOL + + "}"; + } diff --git a/pmd/rulesets/design.xml b/pmd/rulesets/design.xml index d4511f9233..9158115863 100644 --- a/pmd/rulesets/design.xml +++ b/pmd/rulesets/design.xml @@ -1027,7 +1027,7 @@ ConditionalAndExpression [EqualityExpression[@Image='!=']//NullLiteral and InstanceOfExpression - [PrimaryExpression + [PrimaryExpression[count(PrimarySuffix[@ArrayDereference='true'])=0] //Name/@Image = ancestor::ConditionalAndExpression /EqualityExpression//PrimaryPrefix/Name/@Image]]] ]]> diff --git a/pmd/src/net/sourceforge/pmd/ast/ASTPrimarySuffix.java b/pmd/src/net/sourceforge/pmd/ast/ASTPrimarySuffix.java index 591cabf366..2ec6a2c012 100644 --- a/pmd/src/net/sourceforge/pmd/ast/ASTPrimarySuffix.java +++ b/pmd/src/net/sourceforge/pmd/ast/ASTPrimarySuffix.java @@ -12,14 +12,14 @@ public class ASTPrimarySuffix extends SimpleNode { } private boolean isArguments; - private boolean isArrayDeference; + private boolean isArrayDereference; public void setIsArrayDereference() { - isArrayDeference = true; + isArrayDereference = true; } - public boolean isArrayDeference() { - return isArrayDeference; + public boolean isArrayDereference() { + return isArrayDereference; } public void setIsArguments() { @@ -30,6 +30,15 @@ public class ASTPrimarySuffix extends SimpleNode { return this.isArguments; } + + public void dump(String prefix) { + String out = ""; + if (isArrayDereference()) { + out += "["; + } + System.out.println(toString(prefix) + ":" + out); + dumpChildren(prefix); + } /** * Accept the visitor. * */ diff --git a/pmd/src/net/sourceforge/pmd/jaxen/AttributeAxisIterator.java b/pmd/src/net/sourceforge/pmd/jaxen/AttributeAxisIterator.java index f9063a9474..30ba18de36 100644 --- a/pmd/src/net/sourceforge/pmd/jaxen/AttributeAxisIterator.java +++ b/pmd/src/net/sourceforge/pmd/jaxen/AttributeAxisIterator.java @@ -70,8 +70,7 @@ public class AttributeAxisIterator implements Iterator { protected Attribute getAttribute(Node node, Method method) throws IllegalAccessException, InvocationTargetException { - String name = method.getName(); - name = truncateMethodName(name); + String name = truncateMethodName(method.getName()); Object value = method.invoke(node, EMPTY_OBJ_ARRAY); if (value != null) { if (value instanceof String) { diff --git a/pmd/src/net/sourceforge/pmd/rules/IdempotentOperations.java b/pmd/src/net/sourceforge/pmd/rules/IdempotentOperations.java index fc20d43934..ccdf4353b8 100644 --- a/pmd/src/net/sourceforge/pmd/rules/IdempotentOperations.java +++ b/pmd/src/net/sourceforge/pmd/rules/IdempotentOperations.java @@ -44,14 +44,14 @@ public class IdempotentOperations extends AbstractRule { if (lhs.jjtGetParent().jjtGetParent().jjtGetNumChildren() > 1) { Node n = lhs.jjtGetParent().jjtGetParent().jjtGetChild(1); - if (n instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) n).isArrayDeference()) { + if (n instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) n).isArrayDereference()) { return super.visit(node, data); } } if (rhs.jjtGetParent().jjtGetParent().jjtGetNumChildren() > 1) { Node n = rhs.jjtGetParent().jjtGetParent().jjtGetChild(1); - if (n instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) n).isArguments() || ((ASTPrimarySuffix) n).isArrayDeference()) { + if (n instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) n).isArguments() || ((ASTPrimarySuffix) n).isArrayDereference()) { return super.visit(node, data); } } diff --git a/pmd/xdocs/credits.xml b/pmd/xdocs/credits.xml index dfb93d73e1..ebdb1ddfa6 100644 --- a/pmd/xdocs/credits.xml +++ b/pmd/xdocs/credits.xml @@ -43,6 +43,7 @@