From 7cbb385dbbb61395917be328dd49ac020c707ccf Mon Sep 17 00:00:00 2001
From: Andreas Dangel <adangel@users.sourceforge.net>
Date: Sat, 26 Jan 2013 15:25:05 +0100
Subject: [PATCH] pmd: fixed #1011 CloseResource Rule ignores Constructors

---
 pmd/etc/changelog.txt                         |  1 +
 .../java/rule/design/CloseResourceRule.java   | 19 ++++++--
 .../java/rule/design/xml/CloseResource.xml    | 44 +++++++++++++++++--
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt
index b989085a64..f316f53afb 100644
--- a/pmd/etc/changelog.txt
+++ b/pmd/etc/changelog.txt
@@ -3,6 +3,7 @@
 Fixed bug  878: False positive: UnusedFormalParameter for abstract methods
 Fixed bug  913: SignatureDeclareThrowsException is raised twice
 Fixed bug 1007: Parse Exception with annotation
+Fixed bug 1011: CloseResource Rule ignores Constructors
 Fixed bug 1012: False positive: Useless parentheses.
 Fixed bug 1020: Parsing Error
 Fixed bug 1026: PMD doesn't handle 'value =' in SuppressWarnings annotation
diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
index 640c253322..c6eb82bb1b 100644
--- a/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
+++ b/pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
@@ -14,6 +14,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
 import net.sourceforge.pmd.lang.java.ast.ASTBlock;
 import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
 import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
+import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTName;
@@ -71,8 +72,19 @@ public class CloseResourceRule extends AbstractJavaRule {
         return super.visit(node, data);
     }
 
+    @Override
+    public Object visit(ASTConstructorDeclaration node, Object data) {
+        checkForResources(node, data);
+        return data;
+    }
+
     @Override
     public Object visit(ASTMethodDeclaration node, Object data) {
+        checkForResources(node, data);
+        return data;
+    }
+
+    private void checkForResources(Node node, Object data) {
         List<ASTLocalVariableDeclaration> vars = node.findDescendantsOfType(ASTLocalVariableDeclaration.class);
         List<ASTVariableDeclaratorId> ids = new ArrayList<ASTVariableDeclaratorId>();
 
@@ -96,7 +108,6 @@ public class CloseResourceRule extends AbstractJavaRule {
         for (ASTVariableDeclaratorId x : ids) {
             ensureClosed((ASTLocalVariableDeclaration) x.jjtGetParent().jjtGetParent(), x, data);
         }
-        return data;
     }
 
     private void ensureClosed(ASTLocalVariableDeclaration var,
@@ -107,11 +118,11 @@ public class CloseResourceRule extends AbstractJavaRule {
         String target = variableToClose + ".close";
         Node n = var;
 
-        while (!(n instanceof ASTBlock)) {
+        while (!(n instanceof ASTBlock) && !(n instanceof ASTConstructorDeclaration)) {
             n = n.jjtGetParent();
         }
 
-        ASTBlock top = (ASTBlock) n;
+        Node top = n;
 
         List<ASTTryStatement> tryblocks = top.findDescendantsOfType(ASTTryStatement.class);
 
@@ -206,7 +217,7 @@ public class CloseResourceRule extends AbstractJavaRule {
             List<ASTReturnStatement> returns = new ArrayList<ASTReturnStatement>();
             top.findDescendantsOfType(ASTReturnStatement.class, returns, true);
             for (ASTReturnStatement returnStatement : returns) {
-                ASTName name = returnStatement.getFirstChildOfType(ASTName.class);
+                ASTName name = returnStatement.getFirstDescendantOfType(ASTName.class);
                 if ((name != null) && name.getImage().equals(variableToClose)) {
                     closed = true;
                     break;
diff --git a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
index 46aba2fc8a..6ed1a11866 100644
--- a/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
+++ b/pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
@@ -181,7 +181,7 @@ public class Foo {
     </test-code>
 
 
-    <test-code regressionTest="false">
+    <test-code>
         <description><![CDATA[
 [1964798] 3 bugs in CloseResourceRule : Case failing with complete name
      ]]></description>
@@ -247,7 +247,7 @@ public class Foo {
      ]]></code>
     </test-code>        
 
-	<test-code regressionTest="false">
+	<test-code>
         <description><![CDATA[
 [1964798] 3 bugs in CloseResourceRule : If connection is returned, we should not log a violation.
      ]]></description>
@@ -335,5 +335,43 @@ public class StructureFactory {
 }
      ]]></code>
     </test-code>
-    
+    <test-code> reinitializeRule="true"
+        <description>#1011 CloseResource Rule ignores Constructors</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Test {
+    public Test() {
+        Connection c = pool.getConnection();
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code reinitializeRule="true">
+        <description>#1011 CloseResource Rule ignores Constructors - closed in finally</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Test {
+    public Test() {
+        Connection c;
+        try {
+            c = pool.getConnection();
+        } finally {
+            c.close();
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code reinitializeRule="true">
+        <description>#1011 CloseResource Rule ignores Constructors - not a problem - instance variable</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Test {
+    Connection c;
+    public Test() {
+        c = pool.getConnection();
+    }
+}
+        ]]></code>
+    </test-code>
 </test-data>