diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 7444e23886..5b0fa18cb9 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -7,6 +7,7 @@ New rules: J2ee ruleset: DoNotCallSystemExit, StaticEJBFieldShouldBeFinal,DoNotUseThreads JSP basic ruleset: ExternalizeJavascript +Fixed bug 631681 - fixed false positive in UnusedPrivateField when field is accessed by outer class Fixed bug 1472195 - fixed false positives in PositionLiteralsFirstInComparisons when the string is used as a parameter Fixed bug 1522517 - fixed false positive in UselessOverridingMethod for clone method Fixed bug 1744065 - fixed false positive in BooleanInstantiation when a custom Boolean is used diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/unusedcode/xml/UnusedPrivateField.xml b/pmd/regress/test/net/sourceforge/pmd/rules/unusedcode/xml/UnusedPrivateField.xml index e41c7038b2..43d209b8ec 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/unusedcode/xml/UnusedPrivateField.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/unusedcode/xml/UnusedPrivateField.xml @@ -324,6 +324,42 @@ SuppressWarnings("unused") unused private field public class Foo { @SuppressWarnings("unused") private String foo; +} + ]]> + + + + 0 + + + + + 0 + diff --git a/pmd/src/net/sourceforge/pmd/rules/UnusedPrivateFieldRule.java b/pmd/src/net/sourceforge/pmd/rules/UnusedPrivateFieldRule.java index 507c1ea751..b945e738ca 100644 --- a/pmd/src/net/sourceforge/pmd/rules/UnusedPrivateFieldRule.java +++ b/pmd/src/net/sourceforge/pmd/rules/UnusedPrivateFieldRule.java @@ -3,14 +3,20 @@ */ package net.sourceforge.pmd.rules; -import net.sourceforge.pmd.AbstractRule; -import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.symboltable.NameOccurrence; -import net.sourceforge.pmd.symboltable.VariableNameDeclaration; - +import java.util.ArrayList; import java.util.List; import java.util.Map; +import net.sourceforge.pmd.AbstractRule; +import net.sourceforge.pmd.ast.ASTClassOrInterfaceBody; +import net.sourceforge.pmd.ast.ASTClassOrInterfaceBodyDeclaration; +import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.ast.ASTName; +import net.sourceforge.pmd.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.symboltable.NameOccurrence; +import net.sourceforge.pmd.symboltable.VariableNameDeclaration; + public class UnusedPrivateFieldRule extends AbstractRule { public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { @@ -21,18 +27,62 @@ public class UnusedPrivateFieldRule extends AbstractRule { continue; } if (!actuallyUsed(entry.getValue())) { - addViolation(data, decl.getNode(), decl.getImage()); + if (!usedInOuterClass(node, decl)) { + addViolation(data, decl.getNode(), decl.getImage()); + } } } return super.visit(node, data); } + /** + * Find out whether the variable is used in an outer class + */ + private boolean usedInOuterClass(ASTClassOrInterfaceDeclaration node, + VariableNameDeclaration decl) { + List outerClasses = node.getParentsOfType(ASTClassOrInterfaceDeclaration.class); + for (ASTClassOrInterfaceDeclaration outerClass : outerClasses) { + ASTClassOrInterfaceBody classOrInterfaceBody = outerClass.getFirstChildOfType(ASTClassOrInterfaceBody.class); + + List classOrInterfaceBodyDeclarations = new ArrayList(); + classOrInterfaceBody.findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class, classOrInterfaceBodyDeclarations, false); + + for (ASTClassOrInterfaceBodyDeclaration classOrInterfaceBodyDeclaration : classOrInterfaceBodyDeclarations) { + for (int i = 0; i < classOrInterfaceBodyDeclaration.jjtGetNumChildren(); i++) { + if (classOrInterfaceBodyDeclaration.jjtGetChild(i) instanceof ASTClassOrInterfaceDeclaration) { + continue; //Skip other inner classes + } + + List primarySuffixes = classOrInterfaceBodyDeclaration.findChildrenOfType(ASTPrimarySuffix.class); + for (ASTPrimarySuffix primarySuffix : primarySuffixes) { + if (decl.getImage().equals(primarySuffix.getImage())) { + return true; //No violation + } + } + + List primaryPrefixes = classOrInterfaceBodyDeclaration.findChildrenOfType(ASTPrimaryPrefix.class); + for (ASTPrimaryPrefix primaryPrefix : primaryPrefixes) { + ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class); + + if (name != null && name.getImage().endsWith(decl.getImage())) { + return true; //No violation + } + } + } + } + + } + + return false; + } + private boolean actuallyUsed(List usages) { for (NameOccurrence nameOccurrence: usages) { if (!nameOccurrence.isOnLeftHandSide()) { return true; } } + return false; }