Applied patch 1306999 - Renamed CloseConnection to CloseResource and added support for checking Statement and ResultSet objects.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@3886 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Tom Copeland
2005-10-04 00:44:51 +00:00
parent 2faac17b35
commit e6144a65a6
9 changed files with 61 additions and 24 deletions

View File

@ -10,7 +10,8 @@ Fixed bug 1296544 - TooManyFields no longer checks the wrong property value.
Fixed bug 1304739 - StringInstantiation no longer crashes on certain String constructor usages.
Fixed bug 1306180 - AvoidConcatenatingNonLiteralsInStringBuffer no longer reports false positives on certain StringBuffer usages.
Fixed bug 1309235 - TooManyFields no longer includes static finals towards its count.
Implemented RFE 1311309 - Suppressed RuleViolation counts are now include in the text report.
Implemented RFE 1311309 - Suppressed RuleViolation counts are now included in the text report.
Applied patch 1306999 - Renamed CloseConnection to CloseResource and added support for checking Statement and ResultSet objects.
September 15, 2005 - 3.3:
New rules: PositionLiteralsFirstInComparisons (in the design ruleset), UnnecessaryLocalBeforeReturn (design ruleset), ProperLogger (logging-jakarta-commons ruleset), UselessOverridingMethod (basic ruleset), PackageCase (naming ruleset), NoPackage (naming ruleset), UnnecessaryCaseChange (strings ruleset)

View File

@ -38,8 +38,7 @@ public class AbstractRuleTest extends TestCase {
private static class MyRule extends AbstractRule{
public MyRule() {
setName("MyRule");
setMessage("my rule");
setDescription("my rule desc");
setMessage("my rule msg");
setPriority(3);
addProperty("foo", "value");
}
@ -71,7 +70,7 @@ public class AbstractRuleTest extends TestCase {
assertEquals("Line number mismatch!", 5, rv.getNode().getBeginLine());
assertEquals("Filename mismatch!", "filename", rv.getFilename());
assertEquals("Rule object mismatch!", r, rv.getRule());
assertEquals("Rule description mismatch!", "my rule desc", rv.getDescription());
assertEquals("Rule msg mismatch!", "my rule msg", rv.getDescription());
assertEquals("RuleSet name mismatch!", "foo", rv.getRule().getRuleSetName());
}

View File

@ -17,7 +17,7 @@ public class TextPadRendererTest extends RuleTst {
if (c.getImage().equals("Foo")) addViolation(ctx,c);
return ctx;
}
public String getMessage() { return "blah"; }
public String getMessage() { return "msg"; }
public String getName() { return "Foo"; }
public String getRuleSetName() { return "RuleSet"; }
public String getDescription() { return "desc"; }
@ -37,7 +37,7 @@ public class TextPadRendererTest extends RuleTst {
Report rep = new Report();
runTestFromString(TEST1, new FooRule(), rep);
String actual = (new TextPadRenderer()).render(rep);
String expected = PMD.EOL + "n/a(1, Foo): desc" ;
String expected = PMD.EOL + "n/a(1, Foo): msg" ;
assertEquals(expected, actual);
}

View File

@ -9,19 +9,21 @@ import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class CloseConnectionTest extends SimpleAggregatorTst {
public class CloseResourceTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("design", "CloseConnection");
rule = findRule("design", "CloseResource");
}
public void testAll() {
runTests(new TestDescriptor[] {
new TestDescriptor(TEST1, "connection is closed, ok", 0, rule),
new TestDescriptor(TEST2, "connection not closed, should have failed", 1, rule),
new TestDescriptor(TEST3, "java.sql.* not imported, ignore", 0, rule),
new TestDescriptor(TEST3, "ResultSet not closed, should have failed", 1, rule),
new TestDescriptor(TEST4, "Statement not closed, should have failed", 1, rule),
new TestDescriptor(TEST5, "java.sql.* not imported, ignore", 0, rule),
});
}
@ -49,6 +51,26 @@ public class CloseConnectionTest extends SimpleAggregatorTst {
"}";
private static final String TEST3 =
"import java.sql.*;" + PMD.EOL +
"public class Foo {" + PMD.EOL +
" void bar() {" + PMD.EOL +
" ResultSet c = pool.getRS();" + PMD.EOL +
" try {" + PMD.EOL +
" } catch (Exception e) {}" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST4 =
"import java.sql.*;" + PMD.EOL +
"public class Foo {" + PMD.EOL +
" void bar() {" + PMD.EOL +
" Statement c = pool.getStmt();" + PMD.EOL +
" try {" + PMD.EOL +
" } catch (Exception e) {}" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST5 =
"import some.pckg.Connection;" + PMD.EOL +
"public class Foo {" + PMD.EOL +
" void bar() {" + PMD.EOL +

View File

@ -11,12 +11,12 @@ public class UselessAssignmentRuleTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("rulesets/scratchpad.xml", "UselessAssignment");
//rule = findRule("rulesets/scratchpad.xml", "UselessAssignment");
}
public void testAll() {
runTests(new TestDescriptor[] {
new TestDescriptor(TEST1, "local variable useless assignment", 1, rule),
// new TestDescriptor(TEST1, "local variable useless assignment", 1, rule),
});
}

View File

@ -284,12 +284,15 @@ public class Foo {
</rule>
<rule name="CloseConnection"
message="Ensure that Connection objects are always closed after use"
class="net.sourceforge.pmd.rules.CloseConnection">
<rule name="CloseResource"
message="Ensure that resources like this {0} object are closed after use"
class="net.sourceforge.pmd.rules.CloseResource">
<description>
Ensure that Connection objects are always closed after use
Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use
</description>
<properties>
<property name="types" value="Connection,Statement,ResultSet"/>
</properties>
<priority>3</priority>
<example>
<![CDATA[

View File

@ -47,14 +47,14 @@ public class RuleViolation {
private SimpleNode node;
public RuleViolation(Rule rule, RuleContext ctx, SimpleNode node) {
this(rule, ctx, node, rule.getDescription());
this(rule, ctx, node, rule.getMessage());
}
public RuleViolation(Rule rule, RuleContext ctx, SimpleNode node, String specificDescription) {
public RuleViolation(Rule rule, RuleContext ctx, SimpleNode node, String specificMsg) {
this.rule = rule;
this.node = node;
this.filename = ctx.getSourceCodeFilename();
this.description = specificDescription;
this.description = specificMsg;
}
/**

View File

@ -21,6 +21,9 @@ import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Vector;
import java.util.HashSet;
import java.util.Set;
import java.util.StringTokenizer;
/**
@ -35,12 +38,19 @@ import java.util.Vector;
* }
* </pre>
*/
public class CloseConnection extends AbstractRule {
public class CloseResource extends AbstractRule {
private Set types = new HashSet();
public Object visit(ASTCompilationUnit node, Object data) {
if (!importsJavaSqlPackage(node)) {
return data;
}
if (types.isEmpty()) {
for (StringTokenizer st = new StringTokenizer(getStringProperty("types"), ","); st.hasMoreTokens();) {
types.add(st.nextToken());
}
}
return super.visit(node, data);
}
@ -57,7 +67,7 @@ public class CloseConnection extends AbstractRule {
ASTReferenceType ref = (ASTReferenceType)type.jjtGetChild(0);
if (ref.jjtGetChild(0) instanceof ASTClassOrInterfaceType) {
ASTClassOrInterfaceType clazz = (ASTClassOrInterfaceType)ref.jjtGetChild(0);
if (clazz.getImage().equals("Connection")) {
if (types.contains(clazz.getImage())) {
ASTVariableDeclaratorId id = (ASTVariableDeclaratorId) var.jjtGetChild(1).jjtGetChild(0);
ids.add(id);
}
@ -68,8 +78,7 @@ public class CloseConnection extends AbstractRule {
// 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);
ensureClosed((ASTLocalVariableDeclaration) x.jjtGetParent().jjtGetParent(), x, data);
}
return data;
}
@ -110,7 +119,10 @@ public class CloseConnection extends AbstractRule {
// if all is not well, complain
if (!closed) {
addViolation(data, id);
ASTType type = (ASTType) var.jjtGetChild(0);
ASTReferenceType ref = (ASTReferenceType)type.jjtGetChild(0);
ASTClassOrInterfaceType clazz = (ASTClassOrInterfaceType)ref.jjtGetChild(0);
addViolation(data, id, clazz.getImage());
}
}

View File

@ -45,7 +45,7 @@
</subsection>
<subsection name="Contributors">
<ul>
<li>Allan Caplan - Suggested including suppressed rule violations in the report</li>
<li>Allan Caplan - Patch 1306999 to enhance CloseResource to check for more types, suggested including suppressed rule violations in the report</li>
<li>Lori Olson - JBuilder plugin suggestions and prerelease tests, found copy/paste bug in rule descriptions</li>
<li>Rodrigo Ruiz - bug report 1309235 for TooManyFields</li>
<li>Andriy Rozeluk - bug report 1306180 for AvoidConcatenatingNonLiteralsInStringBuffer, reported bug 1293157 for UnusedPrivateMethod, suggested UnnecessaryCaseChange, bug report for SimplifyConditional, suggested UnnecessaryLocalBeforeReturn, suggestions for improving BooleanInstantiation, UnnecessaryReturn, AvoidDuplicateLiterals RFEs and bug reports, various other RFEs and thoughtful discussions as well</li>