Partially fixed bug 998122 - CloseConnectionRule now checks for imports of java.sql before reporting a rule violation.
git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@2836 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
@ -1,11 +1,12 @@
|
||||
?????, 2004 - 2.0:
|
||||
Moved development environment to Maven 1.0.
|
||||
Moved development environment to Ant 1.6.2. This is nice because using the new JUnit task attribute "forkmode='perBatch'" cuts test runtime from 90 seconds to 7 seconds. Sweet.
|
||||
MethodWithSameNameAsEnclosingClass now reports a more helpful line number.
|
||||
Applied patch in RFE 992576 - Enhancements to VariableNamingConventionsRule
|
||||
Fixed bug in SimplifyBooleanExpressions - now it catches more cases.
|
||||
Fixed bugs in AvoidDuplicateLiterals - now it ignores small duplicate literals, its message is more helpful, and it catches more cases.
|
||||
Fixed bug 997893 - false positive in UnusedPrivateField
|
||||
MethodWithSameNameAsEnclosingClass now reports a more helpful line number.
|
||||
Fixed bug 997893 - UnusedPrivateField now detects assignments to members of private fields as a usage.
|
||||
Partially fixed bug 998122 - CloseConnectionRule now checks for imports of java.sql before reporting a rule violation.
|
||||
|
||||
July 14, 2004 - 1.9:
|
||||
New rules: CloneMethodMustImplementCloneable, CloneThrowsCloneNotSupportedException, EqualsNull, ConfusingTernary
|
||||
|
@ -14,6 +14,7 @@ public class CloseConnectionRuleTest extends SimpleAggregatorTst {
|
||||
runTests(new TestDescriptor[] {
|
||||
new TestDescriptor(TEST1, "connection is closed, ok", 0, new CloseConnectionRule()),
|
||||
new TestDescriptor(TEST2, "connection not closed, should have failed", 1, new CloseConnectionRule()),
|
||||
new TestDescriptor(TEST3, "java.sql.* not imported, ignore", 0, new CloseConnectionRule()),
|
||||
});
|
||||
}
|
||||
|
||||
@ -30,6 +31,7 @@ public class CloseConnectionRuleTest extends SimpleAggregatorTst {
|
||||
"}";
|
||||
|
||||
private static final String TEST2 =
|
||||
"import java.sql.*;" + PMD.EOL +
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" void bar() {" + PMD.EOL +
|
||||
" Connection c = pool.getConnection();" + PMD.EOL +
|
||||
@ -39,4 +41,13 @@ public class CloseConnectionRuleTest extends SimpleAggregatorTst {
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
private static final String TEST3 =
|
||||
"import some.pckg.Connection;" + PMD.EOL +
|
||||
"public class Foo {" + PMD.EOL +
|
||||
" void bar() {" + PMD.EOL +
|
||||
" Connection c = pool.getConnection();" + PMD.EOL +
|
||||
" try {} catch (Exception e) {}" + PMD.EOL +
|
||||
" }" + PMD.EOL +
|
||||
"}";
|
||||
|
||||
}
|
||||
|
@ -1,6 +1,6 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
*/
|
||||
package net.sourceforge.pmd.rules;
|
||||
|
||||
import net.sourceforge.pmd.AbstractRule;
|
||||
@ -13,6 +13,8 @@ import net.sourceforge.pmd.ast.ASTTryStatement;
|
||||
import net.sourceforge.pmd.ast.ASTType;
|
||||
import net.sourceforge.pmd.ast.ASTVariableDeclaratorId;
|
||||
import net.sourceforge.pmd.ast.Node;
|
||||
import net.sourceforge.pmd.ast.ASTImportDeclaration;
|
||||
import net.sourceforge.pmd.ast.ASTCompilationUnit;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Iterator;
|
||||
@ -33,69 +35,89 @@ import java.util.Vector;
|
||||
* </pre>
|
||||
*/
|
||||
public class CloseConnectionRule extends AbstractRule {
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
List vars = node.findChildrenOfType(ASTLocalVariableDeclaration.class);
|
||||
List ids = new Vector();
|
||||
|
||||
// find all variable references to Connection objects
|
||||
for (Iterator it = vars.iterator(); it.hasNext();) {
|
||||
ASTLocalVariableDeclaration var = (ASTLocalVariableDeclaration) it.next();
|
||||
ASTType type = (ASTType) var.jjtGetChild(0);
|
||||
private boolean importJavaSQLPackageFound;
|
||||
|
||||
if (type.jjtGetChild(0) instanceof ASTName && ((ASTName) type.jjtGetChild(0)).getImage().equals("Connection")) {
|
||||
ASTVariableDeclaratorId id = (ASTVariableDeclaratorId) var.jjtGetChild(1).jjtGetChild(0);
|
||||
ids.add(id);
|
||||
public Object visit(ASTCompilationUnit node, Object data) {
|
||||
importJavaSQLPackageFound = false;
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
public Object visit(ASTImportDeclaration node, Object data) {
|
||||
if (node.getImportedNameNode().getImage().startsWith("java.sql")) {
|
||||
importJavaSQLPackageFound = true;
|
||||
}
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
// if there are connections, ensure each is closed.
|
||||
for (int i = 0; i < ids.size(); i++) {
|
||||
ASTVariableDeclaratorId x = (ASTVariableDeclaratorId) ids.get(i);
|
||||
ensureClosed((ASTLocalVariableDeclaration) x.jjtGetParent()
|
||||
.jjtGetParent(), x, data);
|
||||
}
|
||||
return data;
|
||||
}
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
// Quick exit if there's not an import java.sql.whatever;
|
||||
if (!importJavaSQLPackageFound) {
|
||||
return data;
|
||||
}
|
||||
|
||||
private void ensureClosed(ASTLocalVariableDeclaration var,
|
||||
ASTVariableDeclaratorId id, Object data) {
|
||||
// What are the chances of a Connection being instantiated in a
|
||||
// for-loop init block? Anyway, I'm lazy!
|
||||
String target = id.getImage() + ".close";
|
||||
Node n = var;
|
||||
List vars = node.findChildrenOfType(ASTLocalVariableDeclaration.class);
|
||||
List ids = new Vector();
|
||||
|
||||
while (!((n = n.jjtGetParent()) instanceof ASTBlock))
|
||||
;
|
||||
// find all variable references to Connection objects
|
||||
for (Iterator it = vars.iterator(); it.hasNext();) {
|
||||
ASTLocalVariableDeclaration var = (ASTLocalVariableDeclaration) it.next();
|
||||
ASTType type = (ASTType) var.jjtGetChild(0);
|
||||
|
||||
ASTBlock top = (ASTBlock) n;
|
||||
|
||||
List tryblocks = new Vector();
|
||||
top.findChildrenOfType(ASTTryStatement.class, tryblocks, true);
|
||||
|
||||
boolean closed = false;
|
||||
|
||||
// look for try blocks below the line the variable was
|
||||
// introduced and make sure there is a .close call in a finally
|
||||
// block.
|
||||
for (Iterator it = tryblocks.iterator(); it.hasNext();) {
|
||||
ASTTryStatement t = (ASTTryStatement) it.next();
|
||||
|
||||
if ((t.getBeginLine() > id.getBeginLine()) && (t.hasFinally())) {
|
||||
ASTBlock f = t.getFinallyBlock();
|
||||
List names = new ArrayList();
|
||||
f.findChildrenOfType(ASTName.class, names, true);
|
||||
for (Iterator it2 = names.iterator(); it2.hasNext();) {
|
||||
if (((ASTName) it2.next()).getImage().equals(target)) {
|
||||
closed = true;
|
||||
if (type.jjtGetChild(0) instanceof ASTName && ((ASTName) type.jjtGetChild(0)).getImage().equals("Connection")) {
|
||||
ASTVariableDeclaratorId id = (ASTVariableDeclaratorId) var.jjtGetChild(1).jjtGetChild(0);
|
||||
ids.add(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// if all is not well, complain
|
||||
if (!closed) {
|
||||
RuleContext ctx = (RuleContext) data;
|
||||
ctx.getReport().addRuleViolation(createRuleViolation(ctx, id.getBeginLine(), getMessage()));
|
||||
}
|
||||
}
|
||||
// if there are connections, ensure each is closed.
|
||||
for (int i = 0; i < ids.size(); i++) {
|
||||
ASTVariableDeclaratorId x = (ASTVariableDeclaratorId) ids.get(i);
|
||||
ensureClosed((ASTLocalVariableDeclaration) x.jjtGetParent()
|
||||
.jjtGetParent(), x, data);
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
private void ensureClosed(ASTLocalVariableDeclaration var,
|
||||
ASTVariableDeclaratorId id, Object data) {
|
||||
// What are the chances of a Connection being instantiated in a
|
||||
// for-loop init block? Anyway, I'm lazy!
|
||||
String target = id.getImage() + ".close";
|
||||
Node n = var;
|
||||
|
||||
while (!((n = n.jjtGetParent()) instanceof ASTBlock))
|
||||
;
|
||||
|
||||
ASTBlock top = (ASTBlock) n;
|
||||
|
||||
List tryblocks = new Vector();
|
||||
top.findChildrenOfType(ASTTryStatement.class, tryblocks, true);
|
||||
|
||||
boolean closed = false;
|
||||
|
||||
// look for try blocks below the line the variable was
|
||||
// introduced and make sure there is a .close call in a finally
|
||||
// block.
|
||||
for (Iterator it = tryblocks.iterator(); it.hasNext();) {
|
||||
ASTTryStatement t = (ASTTryStatement) it.next();
|
||||
|
||||
if ((t.getBeginLine() > id.getBeginLine()) && (t.hasFinally())) {
|
||||
ASTBlock f = t.getFinallyBlock();
|
||||
List names = new ArrayList();
|
||||
f.findChildrenOfType(ASTName.class, names, true);
|
||||
for (Iterator it2 = names.iterator(); it2.hasNext();) {
|
||||
if (((ASTName) it2.next()).getImage().equals(target)) {
|
||||
closed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// if all is not well, complain
|
||||
if (!closed) {
|
||||
RuleContext ctx = (RuleContext) data;
|
||||
ctx.getReport().addRuleViolation(createRuleViolation(ctx, id.getBeginLine(), getMessage()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -13,6 +13,7 @@
|
||||
<li>David Dixon-Peugh - much of the early work on the grammar, initial Emacs plugin</li>
|
||||
<li>David Craine - JBuilder plugin</li>
|
||||
<li>Philippe Herlin - Eclipse plugin, fixed bugs in RuleSetFactory</li>
|
||||
<li>Nascif Abousalh Neto - Emacs plugin</li>
|
||||
<li>Tom Copeland - lead developer, JDeveloper plugin, initial Gel plugin, initial jEdit plugin, IDEAJ integration</li>
|
||||
<li>Jiger Patel - jEdit plugin</li>
|
||||
<li>Gunnlaugur Thor Briem - NetBeans plugin, Maven build script fixes, bug report on JavaCC parser's use of java.lang.Error</li>
|
||||
@ -25,7 +26,6 @@
|
||||
<li>Don Leckie - The PMD GUI</li>
|
||||
<li>Tom Burke - Eclipse plugin</li>
|
||||
<li>Rich Kilmer - logo design</li>
|
||||
<li>Nascif Abousalh Neto - Emacs plugin</li>
|
||||
<li>Paul Kendall - various bugfixes and features</li>
|
||||
<li>Alex Chaffee - various bugfixes and features</li>
|
||||
<li>Siegfried Goeschl - original Maven plugin, various bugfixes and features</li>
|
||||
|
Reference in New Issue
Block a user