diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 35ac39ffcc..003c000c17 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -16,6 +16,7 @@ Fixed bug 1312723 - Added isSyntacticallyPublic() behavior to ASTFieldDeclaratio Fixed bug 1313216 - Designer was not displaying 'final' attribute for ASTLocalVariableDeclaration nodes. Fixed bug 1314086 - Added logging-jakarta-commons as a short name for rulesets/logging-jakarta-commons.xml to SimpleRuleSetNameMapper. Fixed bug 1351498 - Improved UnnecessaryCaseChange warning message. +Fixed bug 1351706 - CompareObjectsWithEquals now catches more cases. Implemented RFE 1311309 - Suppressed RuleViolation counts are now included in the reports. Implemented RFE 1220371 - Rule violation suppression via annotations. Per the JLS, @SuppressWarnings can be placed before the following nodes: TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE. Implemented RFE 1275547 - OverrideBothEqualsAndHashcode now skips Comparator implementations. diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/design/CompareObjectsWithEqualsTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/design/CompareObjectsWithEqualsTest.java index dd0efa34e1..916c5cd50d 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/design/CompareObjectsWithEqualsTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/rules/design/CompareObjectsWithEqualsTest.java @@ -22,6 +22,7 @@ public class CompareObjectsWithEqualsTest extends SimpleAggregatorTst{ new TestDescriptor(TEST4, "missed hit - qualified names. that's ok, we can't resolve the types yet, so better to skip this for now", 0, rule), new TestDescriptor(TEST5, "more qualified name skippage", 0, rule), new TestDescriptor(TEST6, "locals", 1, rule), + new TestDescriptor(TEST7, "2 locals declared on one line", 1, rule), }); } @@ -69,4 +70,14 @@ public class CompareObjectsWithEqualsTest extends SimpleAggregatorTst{ " }" + PMD.EOL + "}"; + private static final String TEST7 = + "public class Foo {" + PMD.EOL + + " void bar() {" + PMD.EOL + + " String a,b;" + PMD.EOL + + " a = \"foo\";" + PMD.EOL + + " b = \"bar\";" + PMD.EOL + + " if (a == b) {}" + PMD.EOL + + " }" + PMD.EOL + + "}"; + } diff --git a/pmd/src/net/sourceforge/pmd/rules/design/CompareObjectsWithEquals.java b/pmd/src/net/sourceforge/pmd/rules/design/CompareObjectsWithEquals.java index 4deb8e3ca1..1214bacf13 100644 --- a/pmd/src/net/sourceforge/pmd/rules/design/CompareObjectsWithEquals.java +++ b/pmd/src/net/sourceforge/pmd/rules/design/CompareObjectsWithEquals.java @@ -6,14 +6,8 @@ import net.sourceforge.pmd.ast.ASTInitializer; import net.sourceforge.pmd.ast.ASTName; import net.sourceforge.pmd.ast.Node; import net.sourceforge.pmd.ast.SimpleNode; -import net.sourceforge.pmd.symboltable.NameOccurrence; -import net.sourceforge.pmd.symboltable.Scope; import net.sourceforge.pmd.symboltable.VariableNameDeclaration; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - public class CompareObjectsWithEquals extends AbstractRule { private boolean hasName(Node n) { @@ -37,28 +31,17 @@ public class CompareObjectsWithEquals extends AbstractRule { return data; } - check((Scope)node.getScope(), node, data); - check(node.getScope().getEnclosingMethodScope(), node, data); - return data; - } + ASTName n0 = (ASTName)node.jjtGetChild(0).jjtGetChild(0).jjtGetChild(0); + ASTName n1 = (ASTName)node.jjtGetChild(1).jjtGetChild(0).jjtGetChild(0); - private void check(Scope scope, SimpleNode node, Object ctx) { - Map vars = scope.getVariableDeclarations(); - for (Iterator i = vars.keySet().iterator(); i.hasNext();) { - VariableNameDeclaration key = (VariableNameDeclaration)i.next(); - if (key.isPrimitiveType() || key.isArray()) { - continue; - } - List usages = (List)vars.get(key); - if (usages.isEmpty()) { - continue; - } - for (Iterator j = usages.iterator(); j.hasNext();) { - if (((NameOccurrence)j.next()).getLocation().jjtGetParent().jjtGetParent().jjtGetParent() == node) { - addViolation(ctx, node); - return; - } + if (n0.getNameDeclaration() instanceof VariableNameDeclaration && n1.getNameDeclaration() instanceof VariableNameDeclaration) { + VariableNameDeclaration nd0 = (VariableNameDeclaration)n0.getNameDeclaration(); + VariableNameDeclaration nd1 = (VariableNameDeclaration)n1.getNameDeclaration(); + if (nd0.isReferenceType() && nd1.isReferenceType()) { + addViolation(data, node); } } + + return data; } } diff --git a/pmd/xdocs/credits.xml b/pmd/xdocs/credits.xml index b7ef317527..c7b2a2354d 100644 --- a/pmd/xdocs/credits.xml +++ b/pmd/xdocs/credits.xml @@ -45,6 +45,7 @@