Update CheckResultSet
This commit is contained in:
1 parent
a10252c071
commit
63bc84f724
4 files changed
+32
-66
No files matched your search
@@ -20,7 +20,7 @@
|
||||
<!-- <rule ref="category/java/bestpractices.xml/AvoidReassigningParameters"/> -->
|
||||
<rule ref="category/java/bestpractices.xml/AvoidStringBufferField"/>
|
||||
<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP"/>
|
||||
<!-- <rule ref="category/java/bestpractices.xml/CheckResultSet"/> -->
|
||||
<rule ref="category/java/bestpractices.xml/CheckResultSet"/>
|
||||
<rule ref="category/java/bestpractices.xml/ConstantsInInterface"/>
|
||||
<rule ref="category/java/bestpractices.xml/DefaultLabelNotLastInSwitchStmt"/>
|
||||
<rule ref="category/java/bestpractices.xml/DoubleBraceInitialization"/>
|
||||
|
||||
+22
-61
@@ -4,90 +4,51 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.java.rule.bestpractices;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import static net.sourceforge.pmd.util.CollectionUtil.setOf;
|
||||
|
||||
import java.sql.ResultSet;
|
||||
import java.util.Set;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTName;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
|
||||
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
|
||||
|
||||
/**
|
||||
* Rule that verifies, that the return values of next(), first(), last(), etc.
|
||||
* calls to a java.sql.ResultSet are actually verified.
|
||||
*
|
||||
*/
|
||||
public class CheckResultSetRule extends AbstractJavaRule {
|
||||
|
||||
private Map<String, Node> resultSetVariables = new HashMap<>();
|
||||
private static final Set<String> METHODS = setOf("next", "previous", "last", "first");
|
||||
|
||||
private static Set<String> methods = new HashSet<>();
|
||||
|
||||
static {
|
||||
methods.add(".next");
|
||||
methods.add(".previous");
|
||||
methods.add(".last");
|
||||
methods.add(".first");
|
||||
@Override
|
||||
public Object visit(ASTWhileStatement node, Object data) {
|
||||
return data;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
resultSetVariables.clear();
|
||||
return super.visit(node, data);
|
||||
public Object visit(ASTReturnStatement node, Object data) {
|
||||
return data;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTLocalVariableDeclaration node, Object data) {
|
||||
ASTClassOrInterfaceType type = null;
|
||||
if (!node.isTypeInferred()) {
|
||||
type = node.getFirstChildOfType(ASTType.class).getFirstDescendantOfType(ASTClassOrInterfaceType.class);
|
||||
}
|
||||
if (type != null && (type.getType() != null && "java.sql.ResultSet".equals(type.getType().getName())
|
||||
|| "ResultSet".equals(type.getImage()))) {
|
||||
ASTVariableDeclarator declarator = node.getFirstChildOfType(ASTVariableDeclarator.class);
|
||||
if (declarator != null) {
|
||||
ASTName name = declarator.getFirstDescendantOfType(ASTName.class);
|
||||
if (type.getType() != null || name != null && name.getImage().endsWith("executeQuery")) {
|
||||
ASTVariableDeclaratorId id = declarator.getFirstChildOfType(ASTVariableDeclaratorId.class);
|
||||
resultSetVariables.put(id.getImage(), node);
|
||||
}
|
||||
}
|
||||
public Object visit(ASTIfStatement node, Object data) {
|
||||
return data;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTMethodCall node, Object data) {
|
||||
if (isResultSetMethod(node)) {
|
||||
addViolation(data, node);
|
||||
}
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTName node, Object data) {
|
||||
String image = node.getImage();
|
||||
String var = getResultSetVariableName(image);
|
||||
if (var != null && resultSetVariables.containsKey(var)
|
||||
&& node.getFirstParentOfType(ASTIfStatement.class) == null
|
||||
&& node.getFirstParentOfType(ASTWhileStatement.class) == null
|
||||
&& node.getFirstParentOfType(ASTReturnStatement.class) == null) {
|
||||
|
||||
addViolation(data, resultSetVariables.get(var));
|
||||
}
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
private String getResultSetVariableName(String image) {
|
||||
if (image.contains(".")) {
|
||||
for (String method : methods) {
|
||||
if (image.endsWith(method)) {
|
||||
return image.substring(0, image.lastIndexOf(method));
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
private boolean isResultSetMethod(ASTMethodCall node) {
|
||||
return METHODS.contains(node.getMethodName())
|
||||
&& TypeTestUtil.isDeclaredInClass(ResultSet.class, node.getMethodType());
|
||||
}
|
||||
}
|
||||
-1
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
|
||||
|
||||
import net.sourceforge.pmd.testframework.PmdRuleTst;
|
||||
|
||||
@org.junit.Ignore("Rule has not been updated yet")
|
||||
public class CheckResultSetTest extends PmdRuleTst {
|
||||
// no additional unit tests
|
||||
}
|
||||
+9
-3
@@ -8,6 +8,7 @@
|
||||
<description>The result set is appropriately tested before using it, no violation.</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class ResultSetTesting
|
||||
{
|
||||
public String goodBehavior() throws SQLException
|
||||
@@ -34,6 +35,7 @@ public class ResultSetTesting
|
||||
<description>This most common violation case, not testing is done before a call to 'last()'.</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class ResultSetTesting
|
||||
{
|
||||
public void executeSql(Statement statement, String query) throws SQLException
|
||||
@@ -51,6 +53,7 @@ public class ResultSetTesting
|
||||
<description>This most common violation case, not testing is done before a call to 'first()'.</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class ResultSetTesting
|
||||
{
|
||||
public void executeSql(Statement statement, String query) throws SQLException
|
||||
@@ -68,6 +71,7 @@ public class ResultSetTesting
|
||||
<description>Using a 'while' instead of 'if' shouldn't result in a violation.</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class ResultSetTesting
|
||||
{
|
||||
public void executeSql(Statement statement, String query) throws SQLException
|
||||
@@ -90,6 +94,7 @@ public class ResultSetTesting
|
||||
<description>#942 CheckResultSet False Positive</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class Test {
|
||||
public int countReadOnlyForwardOnlyJDBC() throws SQLException, ClassNotFoundException {
|
||||
int _count = 0;
|
||||
@@ -148,6 +153,7 @@ public class Test {
|
||||
<description>#1135 CheckResultSet ignores results set declared outside of try/catch (good case)</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
try {
|
||||
@@ -172,7 +178,7 @@ public class Foo {
|
||||
<description>#1135 CheckResultSet ignores results set declared outside of try/catch</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.ResultSet;
|
||||
import java.sql.*;
|
||||
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
@@ -199,6 +205,7 @@ public class Foo {
|
||||
<description>#1135 CheckResultSet ignores results set declared outside of try/catch - prevent false positive</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.sql.*;
|
||||
import com.special.ResultSet;
|
||||
|
||||
public class Foo {
|
||||
@@ -217,8 +224,7 @@ public class Foo {
|
||||
<code><![CDATA[
|
||||
import java.sql.ResultSet;
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
ResultSet results = null;
|
||||
public void bar(ResultSet results) {
|
||||
String answer;
|
||||
List<String> stringList = new ArrayList<String>();
|
||||
|
||||
|
||||
Reference in new issue
Block a user