added UnusedPrivateMethodRule

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

pmd/etc/build.xml

@@ -36,9 +36,9 @@
 
     <target name="pmd">
         <taskdef name="pmd" classname="net.sourceforge.pmd.ant.PMDTask"/>
-        <pmd reportFile="c:\pmd.html" rulesetfiles="rulesets/design.xml,rulesets/unusedcode.xml,rulesets/basic.xml,rulesets/imports.xml" format="html">
-            <fileset dir="c:\data\pmd\pmd\src">
-            <!--<fileset dir="c:\j2sdk1.4.0\src\com\sun\corba">-->
+        <pmd reportFile="c:\jdk.html" rulesetfiles="rulesets/new_for_0_7.xml" format="html">
+            <!--<fileset dir="c:\data\pmd\pmd\src">-->
+            <fileset dir="c:\j2sdk1.4.0\src\">
             <!--<fileset dir="c:\j2sdk1.4.0\src\">-->
                 <include name="**/*.java"/>
             </fileset>

pmd/etc/changelog.txt

@@ -1,5 +1,6 @@
 ???? 2002 - 0.7:
-Fixed bug 583482 - EmptyCatchBlock and EmptyFinallyBlock report the try statement line number.
+Added new rule: UnusedPrivateMethodRule
+Fixed bug 583482 - EmptyCatchBlock and EmptyFinallyBlock no longer report an incorrect line number.
 
 July 18 2002 - 0.6:
 Added new rules: ExcessiveClassLength, ExcessiveMethodLength

pmd/etc/go.bat

@@ -2,4 +2,4 @@
 set MAIN=net.sourceforge.pmd.PMD
 set TEST_FILE=c:\\data\\pmd\\pmd\\test-data\\%1%.java
 
-java %MAIN% %TEST_FILE% xml rulesets\imports.xml
+java %MAIN% %TEST_FILE% xml rulesets\new_for_0_7.xml

pmd/regress/test/net/sourceforge/pmd/rules/UnusedPrivateMethodRuleTest.java

@@ -0,0 +1,52 @@
+/*
+ * User: tom
+ * Date: Jul 19, 2002
+ * Time: 12:25:27 PM
+ */
+package test.net.sourceforge.pmd.rules;
+
+import net.sourceforge.pmd.rules.UnusedPrivateMethodRule;
+import net.sourceforge.pmd.Report;
+
+public class UnusedPrivateMethodRuleTest  extends RuleTst {
+    private UnusedPrivateMethodRule rule;
+
+    public UnusedPrivateMethodRuleTest(String name) {
+        super(name);
+    }
+
+    public void setUp() {
+        rule = new UnusedPrivateMethodRule();
+        rule.setMessage("Avoid this stuff -> ''{0}''");
+    }
+
+    public void test1() throws Throwable {
+        Report report = process("UnusedPrivateMethod1.java", rule);
+        assertTrue(report.isEmpty());
+    }
+
+    public void test2() throws Throwable {
+        Report report = process("UnusedPrivateMethod2.java", rule);
+        assertEquals(1, report.size());
+    }
+
+    public void test3() throws Throwable {
+        Report report = process("UnusedPrivateMethod3.java", rule);
+        assertTrue(report.isEmpty());
+    }
+
+    public void test4() throws Throwable {
+        Report report = process("UnusedPrivateMethod4.java", rule);
+        assertEquals(1, report.size());
+    }
+
+    public void test5() throws Throwable {
+        Report report = process("UnusedPrivateMethod5.java", rule);
+        assertTrue(report.isEmpty());
+    }
+    public void test6() throws Throwable {
+        Report report = process("UnusedPrivateMethod6.java", rule);
+        assertTrue(report.isEmpty());
+    }
+
+}

pmd/rulesets/future.xml

@@ -0,0 +1,33 @@
+<?xml version="1.0"?>
+
+<ruleset name="future">
+  <description>
+These are rules for the future, when we're doing cross-class type checking and such
+  </description>
+
+
+ <rule name="UnnecessaryCast"
+        message="Avoid unnecessary casts"
+        class="net.sourceforge.pmd.rules.UnnecessaryCastRule">
+    <description>
+A variable is cast to itself, one of its supertypes or one of its interfaces.  Usually
+indicates that the programmer is not clear on the class structure they
+are working with.
+    </description>
+
+    <example>
+<![CDATA[
+public Collection doSomething() {
+  List list = new ArrayList();
+
+  return (Collection) list; // Unnecessary Cast
+}
+]]>
+    </example>
+  </rule>
+
+
+</ruleset>
+
+
+

pmd/rulesets/new_for_0_7.xml

@@ -5,22 +5,16 @@
 These are new ones for 0.7
   </description>
 
-
- <rule name="UnnecessaryCast"
-        message="Avoid unnecessary casts"
-        class="net.sourceforge.pmd.rules.UnnecessaryCastRule">
+    <rule name="UnusedPrivateMethod"
+        message="Avoid unused private methods such as ''{0}''"
+        class="net.sourceforge.pmd.rules.UnusedPrivateMethodRule">
     <description>
-A variable is casted to one of its supertypes or interfaces.  Usually
-indicates that the programmer is not clear on the class structure they
-are working with.
+Unused Private Method detects when a private method is declared but is unused.
     </description>
-
     <example>
 <![CDATA[
-public Collection doSomething() { 
-  List list = new ArrayList();
-
-  return (Collection) list; // Unnecessary Cast
+public class Something {
+ private void foo() {} // unused
 }
 ]]>
     </example>

pmd/rulesets/unusedcode.xml

@@ -47,6 +47,8 @@ public class Something {
     </example>
   </rule>
 
+
+
 </ruleset>
 
 

pmd/src/net/sourceforge/pmd/PMD.java

@@ -76,7 +76,6 @@ public class PMD {
         RuleContext ctx = new RuleContext();
         RuleSetFactory ruleSetFactory = new RuleSetFactory();
         RuleSet rules = ruleSetFactory.createRuleSet(pmd.getClass().getClassLoader().getResourceAsStream(ruleSetFilename));
-
         ctx.setReport(new Report());
 
         Renderer rend = null;

pmd/src/net/sourceforge/pmd/ast/ASTArguments.java

@@ -12,6 +12,12 @@ public class ASTArguments extends SimpleNode {
   }
 
 
+    public int getArgumentCount() {
+        if (this.jjtGetNumChildren() == 0) {
+            return 0;
+        }
+        return this.jjtGetChild(0).jjtGetNumChildren();
+    }
   /** Accept the visitor. **/
   public Object jjtAccept(JavaParserVisitor visitor, Object data) {
     return visitor.visit(this, data);

pmd/src/net/sourceforge/pmd/ast/ASTMethodDeclarator.java

@@ -11,6 +11,10 @@ public class ASTMethodDeclarator extends SimpleNode {
     super(p, id);
   }
 
+    public int getParameterCount() {
+        return this.jjtGetChild(0).jjtGetNumChildren();
+    }
+
 
   /** Accept the visitor. **/
   public Object jjtAccept(JavaParserVisitor visitor, Object data) {

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

@@ -12,7 +12,7 @@ import java.text.MessageFormat;
 import net.sourceforge.pmd.ast.*;
 import net.sourceforge.pmd.*;
 
-public class UnusedPrivateInstanceVariableRule extends AbstractRule implements Rule {
+public class UnusedPrivateInstanceVariableRule extends AbstractRule {
 
     private Stack nameSpaces = new Stack();
 

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

@@ -0,0 +1,181 @@
+/*
+ * User: tom
+ * Date: Jul 19, 2002
+ * Time: 12:05:30 PM
+ */
+package net.sourceforge.pmd.rules;
+
+import net.sourceforge.pmd.*;
+import net.sourceforge.pmd.ast.*;
+
+import java.util.Stack;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.HashSet;
+import java.text.MessageFormat;
+
+public class UnusedPrivateMethodRule extends AbstractRule {
+
+    private Set privateMethodNodes = new HashSet();
+
+    // TODO - What I need is a Visitor that does a breadth first search
+    private boolean trollingForDeclarations;
+    private int depth;
+
+    /**
+     * Skip interfaces because they have no implementation
+     */
+    public Object visit(ASTInterfaceDeclaration node, Object data) {
+        return data;
+    }
+
+    /**
+     * Reset state when we leave an ASTCompilationUnit
+     */
+    public Object visit(ASTCompilationUnit node, Object data) {
+        depth = 0;
+        super.visit(node, data);
+        privateMethodNodes.clear();
+        depth = 0;
+        trollingForDeclarations = false;
+        return data;
+    }
+
+    public Object visit(ASTClassBody node, Object data) {
+        depth++;
+
+        // first troll for declarations, but only in the top level class
+        if (depth == 1) {
+            trollingForDeclarations = true;
+            super.visit(node, null);
+            trollingForDeclarations = false;
+        } else {
+            trollingForDeclarations = false;
+        }
+
+        // troll for usages, regardless of depth
+        super.visit(node, null);
+
+        // if we're back at the top level class, harvest
+        if (depth == 1) {
+            RuleContext ctx = (RuleContext)data;
+            harvestUnused(ctx);
+        }
+
+        depth--;
+        return data;
+    }
+
+    //ASTMethodDeclarator
+    // FormalParameters
+    //  FormalParameter
+    //  FormalParameter
+    public Object visit(ASTMethodDeclarator node, Object data) {
+        if (!trollingForDeclarations) {
+            return super.visit(node, data);
+        }
+        AccessNode parent = (AccessNode)node.jjtGetParent();
+        if (!parent.isPrivate() || parent.isStatic()) {
+            return super.visit(node, data);
+        }
+        // exclude these serializable things
+        if (node.getImage().equals("readObject") || node.getImage().equals("writeObject")|| node.getImage().equals("readResolve") || node.getImage().equals("writeResolve")) {
+            return super.visit(node, data);
+        }
+        privateMethodNodes.add(node);
+        return super.visit(node, data);
+    }
+
+    //PrimarySuffix
+    // Arguments
+    //  ArgumentList
+    //   Expression
+    //   Expression
+    public Object visit(ASTPrimarySuffix node, Object data) {
+        if (!trollingForDeclarations && (node.jjtGetParent() instanceof ASTPrimaryExpression) && (node.getImage() != null)) {
+            if (node.jjtGetNumChildren() > 0) {
+                ASTArguments args = (ASTArguments)node.jjtGetChild(0);
+                removeIfUsed(node.getImage(), args.getArgumentCount());
+                return super.visit(node, data);
+            }
+            // to handle this.foo()
+            //PrimaryExpression
+            // PrimaryPrefix
+            // PrimarySuffix <-- this node has "foo"
+            // PrimarySuffix <-- this node has null
+            //  Arguments
+            ASTPrimaryExpression parent = (ASTPrimaryExpression)node.jjtGetParent();
+            int pointer = 0;
+            while (true) {
+                if (parent.jjtGetChild(pointer).equals(node)) {
+                    break;
+                }
+                pointer++;
+            }
+            // now move to the next PrimarySuffix and get the number of arguments
+            pointer++;
+            // this.foo = foo;
+            // yields this:
+            // PrimaryExpression
+            //  PrimaryPrefix
+            //  PrimarySuffix
+            // so we check for that
+            if (parent.jjtGetNumChildren() <= pointer) {
+                return super.visit(node, data);
+            }
+            if (!(parent.jjtGetChild(pointer) instanceof ASTPrimarySuffix)) {
+                return super.visit(node, data);
+            }
+            ASTPrimarySuffix actualMethodNode = (ASTPrimarySuffix)parent.jjtGetChild(pointer);
+            // when does this happen?
+            if (actualMethodNode.jjtGetNumChildren() == 0 || !(actualMethodNode.jjtGetChild(0) instanceof ASTArguments)) {
+                return super.visit(node, data);
+            }
+            ASTArguments args = (ASTArguments)actualMethodNode.jjtGetChild(0);
+            removeIfUsed(node.getImage(), args.getArgumentCount());
+            // what about Outer.this.foo()?
+        }
+        return super.visit(node, data);
+    }
+
+    //PrimaryExpression
+    // PrimaryPrefix
+    //  Name
+    // PrimarySuffix
+    //  Arguments
+    public Object visit(ASTName node, Object data) {
+        if (!trollingForDeclarations && (node.jjtGetParent() instanceof ASTPrimaryPrefix)) {
+            ASTPrimaryExpression primaryExpression = (ASTPrimaryExpression)node.jjtGetParent().jjtGetParent();
+            if (primaryExpression.jjtGetNumChildren() > 1) {
+                ASTPrimarySuffix primarySuffix = (ASTPrimarySuffix)primaryExpression.jjtGetChild(1);
+                if (primarySuffix.jjtGetNumChildren() > 0 && (primarySuffix.jjtGetChild(0) instanceof ASTArguments)) {
+                    ASTArguments arguments = (ASTArguments)primarySuffix.jjtGetChild(0);
+                    removeIfUsed(node.getImage(), arguments.getArgumentCount());
+                }
+            }
+        }
+        return super.visit(node, data);
+    }
+
+    private void removeIfUsed(String nodeImage, int args) {
+        String img = (nodeImage.indexOf('.') == -1) ? nodeImage : nodeImage.substring(nodeImage.indexOf('.') +1, nodeImage.length());
+        for (Iterator i = privateMethodNodes.iterator(); i.hasNext();) {
+            ASTMethodDeclarator methodNode = (ASTMethodDeclarator)i.next();
+            // is the name the same?
+            if (methodNode.getImage().equals(img)) {
+                // is the number of params the same?
+                if (methodNode.getParameterCount() == args) {
+                    // should check param types here, this misses some unused methods
+                    i.remove();
+                }
+            }
+        }
+    }
+
+    private void harvestUnused(RuleContext ctx) {
+        for (Iterator i = privateMethodNodes.iterator(); i.hasNext();) {
+            SimpleNode node = (SimpleNode)i.next();
+            ctx.getReport().addRuleViolation(createRuleViolation(ctx, node.getBeginLine(), MessageFormat.format(getMessage(), new Object[] {node.getImage()})));
+        }
+    }
+}

pmd/test-data/UnusedPrivateMethod1.java

@@ -0,0 +1,6 @@
+public class UnusedPrivateMethod1 {
+ public void bar() {
+  foo();
+ }
+ private void foo() {}
+}

pmd/test-data/UnusedPrivateMethod2.java

@@ -0,0 +1,3 @@
+public class UnusedPrivateMethod2 {
+ private void foo() {}
+}

pmd/test-data/UnusedPrivateMethod3.java

@@ -0,0 +1,11 @@
+public class UnusedPrivateMethod3 {
+ public void bar() {
+  new Runnable() {
+   public void run() {
+    foo();
+   }
+  };
+ }
+
+ private void foo() {}
+}

pmd/test-data/UnusedPrivateMethod4.java

@@ -0,0 +1,7 @@
+public class UnusedPrivateMethod4 {
+ private void foo() {}
+ private void foo(String baz) {}
+ public void bar() {
+  foo();
+ }
+}

pmd/test-data/UnusedPrivateMethod5.java

@@ -0,0 +1,7 @@
+public class UnusedPrivateMethod5 {
+ private void foo(String[] args) {}
+ public static void main(String[] args) {
+  UnusedPrivateMethod5 u = new UnusedPrivateMethod5();
+  u.foo(args); 
+ }
+}

pmd/test-data/UnusedPrivateMethod6.java

@@ -0,0 +1,6 @@
+public class UnusedPrivateMethod6 {
+ public void bar() {
+  this.foo();
+ }
+ private void foo() {}
+}