Fixed bug 1808110 - PreserveStackTrace
(backport from trunk) git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6006 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Romain Pelisse
parent
2f4cdbd6c3
commit
adfd316184
@ -3,6 +3,8 @@
|
||||
'41' and '42' shortcuts for rulesets added
|
||||
Fixed bug 1928009 - Error using migration ruleset in PMD 4.2
|
||||
Fixed bug 1932242 - EmptyMethodInAbstractClassShouldBeAbstract false +
|
||||
Fixed bug 1808110 - PreserveStackTrace
|
||||
|
||||
AvoidDuplicateLiteralRule now has 'skipAnnotations' boolean property
|
||||
ruleset.dtd and ruleset_xml_schema.xsd added to jar file in rulesets directory
|
||||
Update RuleSetWriter to handle non-Apache TRAX implementations, add an option to not use XML Namespaces
|
||||
|
||||
@ -162,8 +162,8 @@ public class Foo {
|
||||
public void foo(String a) {
|
||||
try {
|
||||
int i = Integer.parseInt(a);
|
||||
} catch(Exception e){
|
||||
Exception e1 = new Exception(e);
|
||||
} catch(Exception e10){
|
||||
Exception e1 = new Exception(e10);
|
||||
throw e1;
|
||||
}
|
||||
}
|
||||
@ -243,4 +243,63 @@ public class B {
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
15, IllegalStateException does not have any constructor which takes original exception.
|
||||
]]></description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
try {
|
||||
;
|
||||
} catch (Exception excep) {
|
||||
throw new IllegalStateException();
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
16, False -, No violations reported by PreserveStackTrace Rule
|
||||
]]></description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
try {
|
||||
;
|
||||
} catch (Exception notUsed) {
|
||||
RuntimeException ex = new RuntimeException();
|
||||
throw ex;
|
||||
}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description><![CDATA[
|
||||
17, PreserveStackTrace Rule should exclude this as initCause is used.
|
||||
]]></description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public void bar() {
|
||||
try {
|
||||
;
|
||||
} catch (Exception e) {
|
||||
IllegalStateException ex = new IllegalStateException();
|
||||
ex.initCause(e);
|
||||
throw ex;
|
||||
}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
</test-data>
|
||||
@ -1,6 +1,13 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
package net.sourceforge.pmd.rules.design;
|
||||
|
||||
import net.sourceforge.pmd.AbstractRule;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import net.sourceforge.pmd.AbstractJavaRule;
|
||||
import net.sourceforge.pmd.RuleContext;
|
||||
import net.sourceforge.pmd.ast.ASTArgumentList;
|
||||
import net.sourceforge.pmd.ast.ASTCastExpression;
|
||||
@ -10,61 +17,138 @@ import net.sourceforge.pmd.ast.ASTName;
|
||||
import net.sourceforge.pmd.ast.ASTPrimaryExpression;
|
||||
import net.sourceforge.pmd.ast.ASTPrimaryPrefix;
|
||||
import net.sourceforge.pmd.ast.ASTThrowStatement;
|
||||
import net.sourceforge.pmd.ast.Node;
|
||||
import net.sourceforge.pmd.ast.SimpleNode;
|
||||
import net.sourceforge.pmd.symboltable.NameOccurrence;
|
||||
import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import org.jaxen.JaxenException;
|
||||
|
||||
public class PreserveStackTrace extends AbstractRule {
|
||||
/**
|
||||
*
|
||||
* @author Unknown,
|
||||
* @author Romain PELISSE, belaran@gmail.com, fix for bug 1808110
|
||||
*
|
||||
*/
|
||||
public class PreserveStackTrace extends AbstractJavaRule {
|
||||
|
||||
private List<ASTName> nameNodes = new ArrayList<ASTName>();
|
||||
|
||||
public Object visit(ASTCatchStatement node, Object data) {
|
||||
String target = (((SimpleNode) node.jjtGetChild(0).jjtGetChild(1)).getImage());
|
||||
List<ASTThrowStatement> lstThrowStatements = node.findChildrenOfType(ASTThrowStatement.class);
|
||||
// FUTURE: This is dectection is name based, it should probably used Type Resolution, to become type "based"
|
||||
private static final String FIND_THROWABLE_INSTANCE = "//VariableDeclaratorId[" +
|
||||
"(../descendant::VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/AllocationExpression/ClassOrInterfaceType" +
|
||||
"[" +
|
||||
"contains(@Image,'Exception')" + // Assuming the Exception class does contains 'Exception' in its name
|
||||
"and" +
|
||||
"(not (../Arguments/ArgumentList))" +
|
||||
"]" +
|
||||
")]";
|
||||
|
||||
private static final String ILLEGAL_STATE_EXCEPTION = "IllegalStateException";
|
||||
private static final String FILL_IN_STACKTRACE = ".fillInStackTrace";
|
||||
|
||||
public Object visit(ASTCatchStatement catchStmt, Object data) {
|
||||
String target = ((SimpleNode)catchStmt.jjtGetChild(0).jjtGetChild(1)).getImage();
|
||||
// Gather every variable used to store exception instance created without any argument, inside the catch
|
||||
gatherVariableWithExceptionRef(catchStmt,data);
|
||||
// Inspect all the throw stmt inside the catch stmt
|
||||
List<ASTThrowStatement> lstThrowStatements = catchStmt.findChildrenOfType(ASTThrowStatement.class);
|
||||
for (ASTThrowStatement throwStatement : lstThrowStatements) {
|
||||
SimpleNode sn = (SimpleNode) throwStatement.jjtGetChild(0).jjtGetChild(0);
|
||||
if (sn.getClass().equals(ASTCastExpression.class)) {
|
||||
ASTPrimaryExpression expr = (ASTPrimaryExpression) sn.jjtGetChild(1);
|
||||
Node n = throwStatement.jjtGetChild(0).jjtGetChild(0);
|
||||
if (n.getClass().equals(ASTCastExpression.class)) {
|
||||
ASTPrimaryExpression expr = (ASTPrimaryExpression) n.jjtGetChild(1);
|
||||
if (expr.jjtGetNumChildren() > 1 && expr.jjtGetChild(1).getClass().equals(ASTPrimaryPrefix.class)) {
|
||||
RuleContext ctx = (RuleContext) data;
|
||||
addViolation(ctx, throwStatement);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
ASTArgumentList args = throwStatement.getFirstChildOfType(ASTArgumentList.class);
|
||||
// If the thrown exception is IllegalStateException, no way to preserve the exception (the constructor has no args)
|
||||
if ( ! isThrownExceptionOfType(throwStatement,ILLEGAL_STATE_EXCEPTION) ) {
|
||||
// Retrieve all argument for the throw exception (to see if the original exception is preserved)
|
||||
ASTArgumentList args = throwStatement.getFirstChildOfType(ASTArgumentList.class);
|
||||
|
||||
if (args != null) {
|
||||
ck(data, target, throwStatement, args);
|
||||
} else {
|
||||
SimpleNode child = (SimpleNode) throwStatement.jjtGetChild(0);
|
||||
while (child != null && child.jjtGetNumChildren() > 0
|
||||
&& !child.getClass().equals(ASTName.class)) {
|
||||
child = (SimpleNode) child.jjtGetChild(0);
|
||||
}
|
||||
if (child != null){
|
||||
if( child.getClass().equals(ASTName.class) && (!target.equals(child.getImage()) && !child.hasImageEqualTo(target + ".fillInStackTrace"))) {
|
||||
Map<VariableNameDeclaration, List<NameOccurrence>> vars = ((ASTName) child).getScope().getVariableDeclarations();
|
||||
for (VariableNameDeclaration decl: vars.keySet()) {
|
||||
args = ((SimpleNode) decl.getNode().jjtGetParent())
|
||||
.getFirstChildOfType(ASTArgumentList.class);
|
||||
if (args != null) {
|
||||
ck(data, target, throwStatement, args);
|
||||
}
|
||||
if (args != null) {
|
||||
ck(data, target, throwStatement, args);
|
||||
}
|
||||
else {
|
||||
Node child = throwStatement.jjtGetChild(0);
|
||||
while (child != null && child.jjtGetNumChildren() > 0
|
||||
&& !child.getClass().equals(ASTName.class)) {
|
||||
child = child.jjtGetChild(0);
|
||||
}
|
||||
if (child != null){
|
||||
if( child.getClass().equals(ASTName.class) && (!target.equals(((SimpleNode)child).getImage()) && !((SimpleNode)child).hasImageEqualTo(target + FILL_IN_STACKTRACE))) {
|
||||
Map<VariableNameDeclaration, List<NameOccurrence>> vars = ((ASTName) child).getScope().getVariableDeclarations();
|
||||
for (VariableNameDeclaration decl: vars.keySet()) {
|
||||
args = ((SimpleNode)decl.getNode().jjtGetParent())
|
||||
.getFirstChildOfType(ASTArgumentList.class);
|
||||
if (args != null) {
|
||||
ck(data, target, throwStatement, args);
|
||||
}
|
||||
}
|
||||
} else if(child.getClass().equals(ASTClassOrInterfaceType.class)){
|
||||
addViolation(data, throwStatement);
|
||||
}
|
||||
} else if(child.getClass().equals(ASTClassOrInterfaceType.class)){
|
||||
addViolation(data, throwStatement);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
return super.visit(node, data);
|
||||
return super.visit(catchStmt, data);
|
||||
}
|
||||
|
||||
private void ck(Object data, String target, ASTThrowStatement throwStatement,
|
||||
/*
|
||||
* Search Catch stmt nodes for variable used to store unproperly created throwable or exception
|
||||
*/
|
||||
private void gatherVariableWithExceptionRef(ASTCatchStatement catchStmt, Object data) {
|
||||
try {
|
||||
List<Node> nodes = catchStmt.findChildNodesWithXPath(FIND_THROWABLE_INSTANCE);
|
||||
for ( Node node : nodes ) {
|
||||
List <Node> violations = catchStmt.findChildNodesWithXPath("//Expression/PrimaryExpression/PrimaryPrefix/Name[@Image = '" + ((SimpleNode)node).getImage() + "']");
|
||||
if ( violations != null && violations.size() > 0 ) {
|
||||
// If, after this allocation, the 'initCause' method is called, and the ex passed
|
||||
// this is not a violation
|
||||
if ( ! useInitCause((Node)violations.get(0),catchStmt) ) //FIXME: iterate, better than get(0) ?
|
||||
super.addViolation(data,(SimpleNode) node);
|
||||
}
|
||||
}
|
||||
} catch (JaxenException e) {
|
||||
// XPath is valid, this should never happens...
|
||||
e.printStackTrace();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private boolean useInitCause(Node node, ASTCatchStatement catchStmt) throws JaxenException {
|
||||
// In case of NPE...
|
||||
if ( node != null && ((SimpleNode)node).getImage() != null )
|
||||
{
|
||||
List <Node> nodes = catchStmt.findChildNodesWithXPath("descendant::StatementExpression/PrimaryExpression/PrimaryPrefix/Name[@Image = '" + ((SimpleNode)node).getImage() + ".initCause']");
|
||||
if ( nodes != null && nodes.size() > 0 )
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean isThrownExceptionOfType(ASTThrowStatement throwStatement,String type) {
|
||||
boolean status = false;
|
||||
try {
|
||||
List<Node> results = throwStatement.findChildNodesWithXPath("Expression/PrimaryExpression/PrimaryPrefix/AllocationExpression/ClassOrInterfaceType[@Image = '" + type + "']");
|
||||
// If we have a match, return true
|
||||
if ( results != null && results.size() == 1 ) {
|
||||
status = true;
|
||||
}
|
||||
} catch (JaxenException e) {
|
||||
// XPath is valid, this should never happens !
|
||||
e.printStackTrace();
|
||||
}
|
||||
return status;
|
||||
}
|
||||
|
||||
private void ck(Object data, String target, ASTThrowStatement throwStatement,
|
||||
ASTArgumentList args) {
|
||||
boolean match = false;
|
||||
nameNodes.clear();
|
||||
@ -75,7 +159,7 @@ public class PreserveStackTrace extends AbstractRule {
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!match) {
|
||||
if ( ! match) {
|
||||
RuleContext ctx = (RuleContext) data;
|
||||
addViolation(ctx, throwStatement);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user