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
This commit is contained in:
Romain Pelisse
2010-01-08 17:16:35 +00:00
parent c114e27594
commit 32fe36028d
4 changed files with 102 additions and 3 deletions

View File

@ -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

View File

@ -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>

View File

@ -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;
}
}

View File

@ -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>