From c0a4006decf4c42207918e47e65498ef1922705d Mon Sep 17 00:00:00 2001
From: Tom Copeland <tomcopeland@users.sourceforge.net>
Date: Wed, 14 Dec 2005 22:20:47 +0000
Subject: [PATCH] More code in the framework, less code in the rule... good
 times

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4064 51baf565-9d33-0410-a72c-fc3788e3496d
---
 pmd/etc/bug_closer.txt                        |  3 +-
 pmd/rulesets/scratchpad.xml                   | 43 ++++++++-----------
 pmd/rulesets/unusedcode.xml                   |  2 -
 .../pmd/ast/ASTVariableDeclaratorId.java      | 11 +++++
 .../pmd/rules/UnusedLocalVariableRule.java    | 37 +++-------------
 .../ScopeAndDeclarationFinder.java            |  4 +-
 6 files changed, 40 insertions(+), 60 deletions(-)

diff --git a/pmd/etc/bug_closer.txt b/pmd/etc/bug_closer.txt
index 8ef13246c9..51bfd3aa4c 100644
--- a/pmd/etc/bug_closer.txt
+++ b/pmd/etc/bug_closer.txt
@@ -7,4 +7,5 @@ that contains this fix.
 Thanks,
 
 Tom
-Using PMD?  Get the book, "PMD Applied"!  http://pmdapplied.com/
\ No newline at end of file
+Using PMD?  Get the book, "PMD Applied"!  http://pmdapplied.com/
+
diff --git a/pmd/rulesets/scratchpad.xml b/pmd/rulesets/scratchpad.xml
index 9b3f4ac5bb..3d8a15df02 100644
--- a/pmd/rulesets/scratchpad.xml
+++ b/pmd/rulesets/scratchpad.xml
@@ -6,33 +6,26 @@
 These are new rules that are still in progress
   </description>
 
-    <rule name="NullAssignment"
-          message="Assigning an Object to null is a code smell.  Consider refactoring."
-          class="net.sourceforge.pmd.rules.design.NullAssignmentRule"
-          externalInfoUrl="http://pmd.sourceforge.net/rules/controversial.html#NullAssignment">
-      <description>
- Assigning a "null" to a variable (outside of its declaration) is usually
- bad form.  Some times, the assignment is an indication that the programmer doesn't
- completely understand what is going on in the code.  NOTE: This sort of assignment
- may in rare cases be useful to encourage garbage collection.  If that's what you're using
- it for, by all means, disregard this rule :-)
-      </description>
+    <rule name="UnusedLocalVariable"
+            message="Avoid unused local variables such as ''{0}''"
+            class="net.sourceforge.pmd.rules.UnusedLocalVariableRule"
+            externalInfoUrl="http://pmd.sourceforge.net/rules/unusedcode.html#UnusedLocalVariable">
+        <description>
+    Detects when a local variable is declared and/or assigned, but not used.
+        </description>
         <priority>3</priority>
-      <example>
- <![CDATA[
- public class Foo {
-   public void bar() {
-     Object x = null; // This is OK.
-     x = new Object();
-     // Big, complex piece of code here.
-     x = null; // This is BAD.
-     // Big, complex piece of code here.
-   }
- }
 
- ]]>
-      </example>
-    </rule>
+        <example>
+    <![CDATA[
+    public class Foo {
+     public void doSomething() {
+      int i = 5; // Unused
+     }
+    }
+    ]]>
+        </example>
+      </rule>
+
 
 
 <!--
diff --git a/pmd/rulesets/unusedcode.xml b/pmd/rulesets/unusedcode.xml
index 79ecb86653..afff0befd0 100644
--- a/pmd/rulesets/unusedcode.xml
+++ b/pmd/rulesets/unusedcode.xml
@@ -48,8 +48,6 @@ public class Foo {
     </example>
   </rule>
 
-
-
     <rule name="UnusedPrivateMethod"
         message="Avoid unused private methods such as ''{0}''"
         class="net.sourceforge.pmd.rules.UnusedPrivateMethodRule"
diff --git a/pmd/src/net/sourceforge/pmd/ast/ASTVariableDeclaratorId.java b/pmd/src/net/sourceforge/pmd/ast/ASTVariableDeclaratorId.java
index 3f7fce97cd..2194e92766 100644
--- a/pmd/src/net/sourceforge/pmd/ast/ASTVariableDeclaratorId.java
+++ b/pmd/src/net/sourceforge/pmd/ast/ASTVariableDeclaratorId.java
@@ -2,6 +2,9 @@
 
 package net.sourceforge.pmd.ast;
 
+import net.sourceforge.pmd.symboltable.NameDeclaration;
+import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
+
 public class ASTVariableDeclaratorId extends SimpleNode {
 
     public ASTVariableDeclaratorId(int id) {
@@ -20,6 +23,14 @@ public class ASTVariableDeclaratorId extends SimpleNode {
     }
 
     private int arrayDepth;
+    private VariableNameDeclaration nameDeclaration;
+
+    public VariableNameDeclaration getNameDeclaration() {
+        return nameDeclaration;
+    }
+    public void setNameDeclaration(VariableNameDeclaration decl) {
+        nameDeclaration = decl;
+    }
 
     public void bumpArrayDepth() {
         arrayDepth++;
diff --git a/pmd/src/net/sourceforge/pmd/rules/UnusedLocalVariableRule.java b/pmd/src/net/sourceforge/pmd/rules/UnusedLocalVariableRule.java
index aa2b875162..b2e822cdc5 100644
--- a/pmd/src/net/sourceforge/pmd/rules/UnusedLocalVariableRule.java
+++ b/pmd/src/net/sourceforge/pmd/rules/UnusedLocalVariableRule.java
@@ -4,49 +4,24 @@
 package net.sourceforge.pmd.rules;
 
 import net.sourceforge.pmd.AbstractRule;
-import net.sourceforge.pmd.ast.ASTCompilationUnit;
 import net.sourceforge.pmd.ast.ASTLocalVariableDeclaration;
 import net.sourceforge.pmd.ast.ASTVariableDeclaratorId;
 import net.sourceforge.pmd.symboltable.NameOccurrence;
-import net.sourceforge.pmd.symboltable.Scope;
 import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
 
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 public class UnusedLocalVariableRule extends AbstractRule {
 
-    private Set visited = new HashSet();
-
-    public Object visit(ASTCompilationUnit acu, Object data) {
-        visited.clear();
-        return super.visit(acu, data);
-    }
-
     public Object visit(ASTVariableDeclaratorId node, Object data) {
         if (node.jjtGetParent().jjtGetParent() instanceof ASTLocalVariableDeclaration) {
-            Scope scope = node.getScope();
-            if (visited.contains(scope)) {
-                return data;
-            } else {
-                visited.add(scope);
-            }
-            Map locals = scope.getVariableDeclarations();
-            for (Iterator i = locals.keySet().iterator(); i.hasNext();) {
-                VariableNameDeclaration decl = (VariableNameDeclaration) i.next();
-                // TODO this misses some cases
-                // need to add DFAish code to determine if an array
-                // is initialized locally or gotten from somewhere else
-                if (decl.isArray()) {
-                    continue;
-                }
-                List usages = (List) locals.get(decl);
-                if (!actuallyUsed(usages)) {
-                    addViolation(data, decl.getNode(), decl.getImage());
-                }
+            VariableNameDeclaration decl = (VariableNameDeclaration)node.getNameDeclaration();
+            // TODO this isArray() check misses some cases
+            // need to add DFAish code to determine if an array
+            // is initialized locally or gotten from somewhere else
+            if (!decl.isArray() && !actuallyUsed((List)node.getScope().getVariableDeclarations().get(decl))) {
+                addViolation(data, decl.getNode(), decl.getImage());
             }
         }
         return data;
diff --git a/pmd/src/net/sourceforge/pmd/symboltable/ScopeAndDeclarationFinder.java b/pmd/src/net/sourceforge/pmd/symboltable/ScopeAndDeclarationFinder.java
index 8ee1650422..e768a32d69 100644
--- a/pmd/src/net/sourceforge/pmd/symboltable/ScopeAndDeclarationFinder.java
+++ b/pmd/src/net/sourceforge/pmd/symboltable/ScopeAndDeclarationFinder.java
@@ -199,7 +199,9 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
     }
 
     public Object visit(ASTVariableDeclaratorId node, Object data) {
-        node.getScope().addDeclaration(new VariableNameDeclaration(node));
+        VariableNameDeclaration decl = new VariableNameDeclaration(node);
+        node.getScope().addDeclaration(decl);
+        node.setNameDeclaration(decl);
         return super.visit(node, data);
     }