hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

#1375 CloseResource not detected properly

Browse Source
This commit is contained in:
Andreas Dangel
2015-06-20 18:15:02 +02:00
parent 39eb581134
commit 1b35a59997
3 changed files with 51 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java
View File

@ -58,7 +58,7 @@ public class CloseResourceRule extends AbstractJavaRule {
private Set<String> closeTargets = new HashSet<String>();
private static final StringMultiProperty CLOSE_TARGETS_DESCRIPTOR = new StringMultiProperty("closeTargets",
"Methods which may close this resource", new String[] {}, 1.0f, ',');
"Methods which may close this resource", new String[] {"close"}, 1.0f, ',');
private static final StringMultiProperty TYPES_DESCRIPTOR = new StringMultiProperty("types", "Affected types",
new String[] { "java.sql.Connection", "java.sql.Statement", "java.sql.ResultSet" }, 2.0f, ',');
@ -121,14 +121,8 @@ public class CloseResourceRule extends AbstractJavaRule {
if (clazz.getType() != null && types.contains(clazz.getType().getName()) || clazz.getType() == null
&& simpleTypes.contains(toSimpleType(clazz.getImage())) || types.contains(clazz.getImage())) {
// if the variables are initialized with null, then they
// are ignored.
// At some point later in the code, there is an
// assignment - however, this is currently ignored
if (!hasNullInitializer(var)) {
ASTVariableDeclaratorId id = var.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
ids.add(id);
}
ASTVariableDeclaratorId id = var.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
ids.add(id);
}
}
}
@ -158,7 +152,6 @@ public class CloseResourceRule extends AbstractJavaRule {
// What are the chances of a Connection being instantiated in a
// for-loop init block? Anyway, I'm lazy!
String variableToClose = id.getImage();
String target = variableToClose + ".close";
Node n = var;
while (!(n instanceof ASTBlock) && !(n instanceof ASTConstructorDeclaration)) {
@ -182,7 +175,9 @@ public class CloseResourceRule extends AbstractJavaRule {
// variable declaration and
// the beginning of the try block.
ASTBlockStatement tryBlock = t.getFirstParentOfType(ASTBlockStatement.class);
if (parentBlock.jjtGetParent() == tryBlock.jjtGetParent()) {
if (!hasNullInitializer(var) // no need to check for critical statements, if
// the variable has been initialized with null
&& parentBlock.jjtGetParent() == tryBlock.jjtGetParent()) {
List<ASTBlockStatement> blocks = parentBlock.jjtGetParent().findChildrenOfType(ASTBlockStatement.class);
int parentBlockIndex = blocks.indexOf(parentBlock);
@ -208,11 +203,7 @@ public class CloseResourceRule extends AbstractJavaRule {
List<ASTName> names = f.findDescendantsOfType(ASTName.class);
for (ASTName oName : names) {
String name = oName.getImage();
if (name.equals(target) && nullCheckIfCondition(f, oName, variableToClose)) {
closed = true;
break;
}
if (name.contains(".")) {
if (name != null && name.contains(".")) {
String[] parts = name.split("\\.");
if (parts.length == 2) {
String methodName = parts[1];

45
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml
View File

@ -154,12 +154,13 @@ New use case
<rule-property name="closeTargets">commit,close</rule-property>
<rule-property name="types">DAOTransaction,Connection,Statement,ResulSet</rule-property>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
import java.sql.*;
public class Foo {
public void bar() throws SQLException
{
DAOTransaction trx = trxManager.open();;
DAOTransaction trx = trxManager.open();
Connection cnx = pool.newConn();
ResultSet rs = null;
Statement stmt = null;
@ -170,8 +171,8 @@ public class Foo {
}
finally
{
rs.close(); // Correct
//stmt.close(); // Error !!! you have to close the Statement
rs.close(); // Correct
cnx.commit(); // Correct ( cnx.close() would be equivalent)
trx.commit(); // Correct
}
@ -530,6 +531,46 @@ public class Bar {
}
}
}
}
]]></code>
</test-code>
<test-code>
<description>#1375 CloseResource not detected properly - ok</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
ResultSet rs;
Statement stmt = getConnection().createStatement();
try {
rs = stmt.getResultSet();
rs.getString(0);
rs.close();
} catch (Exception e) {
} finally {
stmt.close();
}
}
}
]]></code>
</test-code>
<test-code>
<description>#1375 CloseResource not detected properly - false negative</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
ResultSet rs = null;
Statement stmt = getConnection().createStatement();
try {
rs = stmt.getResultSet();
rs.getString(0);
rs.close();
} catch (Exception e) {
} finally {
stmt.close();
}
}
}
]]></code>
</test-code>

1
src/site/markdown/overview/changelog.md
View File

@ -17,5 +17,6 @@
* [#1364](https://sourceforge.net/p/pmd/bugs/1364/): FieldDeclarationsShouldBeAtStartOfClass false positive using multiple annotations
* [#1365](https://sourceforge.net/p/pmd/bugs/1365/): Aggregated javadoc report is missing
* [#1366](https://sourceforge.net/p/pmd/bugs/1366/): UselessParentheses false positive on multiple equality operators
* [#1375](https://sourceforge.net/p/pmd/bugs/1375/): CloseResource not detected properly
**API Changes:**