diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java index 35a0de31ae..32df8ecaea 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CloseResourceRule.java @@ -58,7 +58,7 @@ public class CloseResourceRule extends AbstractJavaRule { private Set closeTargets = new HashSet(); 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 blocks = parentBlock.jjtGetParent().findChildrenOfType(ASTBlockStatement.class); int parentBlockIndex = blocks.indexOf(parentBlock); @@ -208,11 +203,7 @@ public class CloseResourceRule extends AbstractJavaRule { List 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]; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml index 46d1dbe9ae..c174bbca19 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/CloseResource.xml @@ -154,12 +154,13 @@ New use case commit,close DAOTransaction,Connection,Statement,ResulSet 1 + 8 + + + #1375 CloseResource not detected properly - ok + 1 + + + + #1375 CloseResource not detected properly - false negative + 1 + diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 959de654dc..942615f290 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -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:**