From 32fe36028dee24b6662f236d7fd62cf809861a00 Mon Sep 17 00:00:00 2001
From: Romain Pelisse <rpelisse@users.sourceforge.net>
Date: Fri, 8 Jan 2010 17:16:35 +0000
Subject: [PATCH] fixed bug False+ on CloseResource - ID: 2920057 by applying
 Nicolas Dordet patch.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@7060 51baf565-9d33-0410-a72c-fc3788e3496d
---
 pmd/etc/changelog.txt                         |  1 +
 .../java/rule/design/xml/CloseResource.xml    | 74 ++++++++++++++++++-
 .../java/rule/design/CloseResourceRule.java   | 27 ++++++-
 pmd/xdocs/credits.xml                         |  3 +-
 4 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt
index 8fe01b68b5..208babc5d2 100644
--- a/pmd/etc/changelog.txt
+++ b/pmd/etc/changelog.txt
@@ -356,6 +356,7 @@ Creating a unnecessary Code Ruleset and moved the following rules from Basic rul
         * UselessParentheses
 	Basic rulesets still includes a reference to those rules.	
 
+Fixed bug 2920057 - Fixed False + on CloseResource
 Fixed bug 1808110 - Fixed performance issues on PreserveStackTrace
 Fixed bug 2832322 - cpd.xml file tag path attribute should be entity-encoded
 Fixed bug 2826119 - False +: DoubleCheckedLocking warning with volatile field
diff --git a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
index a97cb7e740..2b98c7291c 100644
--- a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
+++ b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
@@ -262,6 +262,78 @@ public class Foo {
 }
      ]]></code>
     </test-code>
-    
+        <test-code reinitializeRule="true">
+        <description><![CDATA[
+invoke an external method that close the resource
+bug 2920057
+     ]]></description>
+        <rule-property name="closeTargets">closeStatement,closeStatement,closeResultSet,closeConnexion
+        </rule-property>
+        <rule-property name="types">PreparedStatement,Statement,ResultSet,Connection</rule-property>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.sql.*;
+
+public class StructureFactory {
+
+    public void rechercherListe() {
+
+        Connection _connexion = null;
+        ResultSet _rs = null;
+        PreparedStatement _st = null;
+        try
+        {
+           //
+        }
+
+
+        finally
+        {
+            getProviderInstance().closeConnexion(_connexion);
+            getProviderInstance().closeResultSet(_rs);
+            getProviderInstance().closeStatement(_st);
+            
+        }
+    }
+}
+     ]]></code>
+    </test-code>
+    <test-code reinitializeRule="true">
+        <description><![CDATA[
+invoke an external method that closes the resource, but one is not the right method and an another is not the right variable
+see bug 2920057
+     ]]></description>
+        <rule-property name="closeTargets">closeStatement,closeStatement,closeResultSet,closeConnexion
+        </rule-property>
+        <rule-property name="types">PreparedStatement,Statement,ResultSet,Connection</rule-property>
+        <expected-problems>2</expected-problems>
+        <code><![CDATA[
+import java.sql.*;
+
+public class StructureFactory {
+
+    public void searchList() {
+
+        Connection _connexion = null;
+        ResultSet _rs = null;
+        PreparedStatement _st = null;
+        Structure _structure = null;
+        try
+        {
+           //
+        }
+
+
+        finally
+        {
+            getProviderInstance().closeConnexion(_connexion);
+            getProviderInstance().closeYourEyes(_rs); //not the right method
+            getProviderInstance().closeStatement(_badOne); // not the right variable
+            
+        }
+    }
+}
+     ]]></code>
+    </test-code>
     
 </test-data>
diff --git a/pmd/src/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java b/pmd/src/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
index 1203760b2e..640c253322 100644
--- a/pmd/src/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
+++ b/pmd/src/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
@@ -10,6 +10,7 @@ import java.util.List;
 import java.util.Set;
 
 import net.sourceforge.pmd.lang.ast.Node;
+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;
@@ -167,6 +168,28 @@ public class CloseResourceRule extends AbstractJavaRule {
                                     }
                                 }
                             }
+                         // look for primary suffix containing the close Targets elements.
+                            // If the .close is executed in another class accessed by a method
+                            // this form : getProviderInstance().closeConnexion(connexion)
+                            // For this use case, we assume the variable is correctly closed
+                            // in the other class since there is no way to really check it.
+                            if (!closed)
+                            {
+                                List<ASTPrimarySuffix> suffixes = new ArrayList<ASTPrimarySuffix>();
+                                expr.findDescendantsOfType(ASTPrimarySuffix.class, suffixes, true);
+                                for (ASTPrimarySuffix oSuffix : suffixes) {
+                                    String suff = oSuffix.getImage();
+                                    if (closeTargets.contains(suff)) 
+                                    {
+                                        closed = variableIsPassedToMethod(expr, variableToClose);
+                                        if(closed)
+                                        { 
+                                            break;                    
+                                        }                                                        
+                                    }
+
+                                }
+                            }
                         }
                     }
                 }
@@ -205,7 +228,9 @@ public class CloseResourceRule extends AbstractJavaRule {
         expr.findDescendantsOfType(ASTName.class, methodParams, true);
         for (ASTName pName : methodParams) {
             String paramName = pName.getImage();
-            if (paramName.equals(variable)) {
+            // also check if we've got the a parameter (i.e if it's an argument !) 
+            ASTArgumentList parentParam = pName.getFirstParentOfType(ASTArgumentList.class);
+            if (paramName.equals(variable) && parentParam != null) {
                 return true;
             }
         }
diff --git a/pmd/xdocs/credits.xml b/pmd/xdocs/credits.xml
index 8b2888eb64..404e380997 100644
--- a/pmd/xdocs/credits.xml
+++ b/pmd/xdocs/credits.xml
@@ -57,7 +57,8 @@
         </subsection>
         <subsection name="Contributors">
         <ul>
-            <li>Sergey Pariev - Fixed an ugly ArrayIndexOutOfBoundsException in CPD for Ruby</li>
+            <li>Nicolas Dordet - Fixed an issue on CloseResource</li>
+	    <li>Sergey Pariev - Fixed an ugly ArrayIndexOutOfBoundsException in CPD for Ruby</li>
             <li>Chris Heister - Reported and noted proper fix for bug in IDEAJ renderer operations</li>
             <li>Ralf Wagner - Reported bug in UselessOperationOnImmutable, reported and noted proper fix for broken XSLT</li>
             <li>Caroline Rioux - Reported bug in ImmutableField</li>