Fixed bug 631681 - fixed false positive in UnusedPrivateField when field is accessed by outer class

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@5512 51baf565-9d33-0410-a72c-fc3788e3496d

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

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;
+}
+     ]]></code>
+    </test-code>
+    <test-code>
+        <description><![CDATA[
+631681, private field is accessed by outer class
+     ]]></description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class XPathFunctionContext {
+
+    private class Singleton {
+        private String foo = "";
+    }
+
+    public String getFoo() {
+        return (new Singleton()).foo;
+    }
+}
+     ]]></code>
+    </test-code>
+    <test-code>
+        <description><![CDATA[
+631681, private field in singleton is accessed by outer class
+     ]]></description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class XPathFunctionContext {
+
+    private static class Singleton {
+        private static XPathFunctionContext instance = new XPathFunctionContext();
+    }
+
+    public static XPathFunctionContext getInstance() {
+        return Singleton.instance;
+    }
 }
      ]]></code>
     </test-code>

pmd/src/net/sourceforge/pmd/rules/UnusedPrivateFieldRule.java

@@ -3,14 +3,20 @@
  */
 package net.sourceforge.pmd.rules;
 
-import net.sourceforge.pmd.AbstractRule;
+import java.util.ArrayList;
-import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
-import net.sourceforge.pmd.symboltable.NameOccurrence;
-import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
-
 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<ASTClassOrInterfaceDeclaration> outerClasses = node.getParentsOfType(ASTClassOrInterfaceDeclaration.class);
+		for (ASTClassOrInterfaceDeclaration outerClass : outerClasses) {
+			ASTClassOrInterfaceBody classOrInterfaceBody = outerClass.getFirstChildOfType(ASTClassOrInterfaceBody.class);
+			
+			List<ASTClassOrInterfaceBodyDeclaration> classOrInterfaceBodyDeclarations = new ArrayList<ASTClassOrInterfaceBodyDeclaration>();
+			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<ASTPrimarySuffix> primarySuffixes = classOrInterfaceBodyDeclaration.findChildrenOfType(ASTPrimarySuffix.class);
+					for (ASTPrimarySuffix primarySuffix : primarySuffixes) {
+						if (decl.getImage().equals(primarySuffix.getImage())) {
+							return true; //No violation
+						}
+					}
+					
+					List<ASTPrimaryPrefix> 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<NameOccurrence> usages) {
         for (NameOccurrence nameOccurrence: usages) {
             if (!nameOccurrence.isOnLeftHandSide()) {
                 return true;
             }
         }
+        
         return false;
     }