This new rule checks for exceptions which are hidden from the re-throw. So, when an exception is caught and a new exception thrown, the old exception is not passed into the new exception. This sucks for trying to debug problems.


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4382 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Allan Caplan
2006-05-19 00:55:30 +00:00
parent 909ec95ffe
commit 071b18e4c5
5 changed files with 237 additions and 0 deletions

View File

@ -1,6 +1,7 @@
????, 2006 - 3.7:
New rules:
Basic-JSP ruleset: DuplicateJspImport
Design ruleset: PreserveStackTrace
Implemented RFE 1462019 - Add JSPs to Ant Task
Implemented RFE 1462020 - Add JSPs to Designer
Fixed bug 1461426 InsufficientStringBufferDeclaration does not consider paths

View File

@ -0,0 +1,144 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package test.net.sourceforge.pmd.rules.design;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleSetNotFoundException;
import test.net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import test.net.sourceforge.pmd.testframework.TestDescriptor;
public class PreserveStackTraceTest extends SimpleAggregatorTst {
private Rule rule;
public void setUp() throws RuleSetNotFoundException {
rule = findRule("design", "PreserveStackTrace");
}
public void test() throws Throwable {
rule.setMessage("{0}");
runTests(new TestDescriptor[]{
new TestDescriptor(TEST1_FAIL, "1, Exception thrown without preserving stack", 1, rule),
new TestDescriptor(TEST2_OK, "2, Exception thrown, stack preserved", 0, rule),
new TestDescriptor(TEST3_OK, "3, Exception thrown, stack preserved", 0, rule),
new TestDescriptor(TEST4_OK, "4, No exception thrown, OK", 0, rule),
new TestDescriptor(TEST5_OK, "5, No exception thrown, OK", 0, rule),
new TestDescriptor(TEST6_OK, "6, No exception thrown, OK", 0, rule),
new TestDescriptor(TEST7_OK, "7, No exception thrown, OK", 0, rule),
new TestDescriptor(TEST8_OK, "8, No exception thrown, OK", 0, rule),
new TestDescriptor(TEST9_OK, "9, Excetion is cast, OK", 0, rule),
});
}
private static final String TEST1_FAIL =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw new Exception(e.getMessage());" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST2_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw new Exception(e);" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST3_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw new Exception(e, e.getMessage());" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST4_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw e.fillInStackTrace();" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST5_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST6_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" e.printStackTrace();" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST7_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw new Exception(Bar.foo(e),e);" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST8_FAIL =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw new Exception(Bar.foo(e));" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST8_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw (Error)e;" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
private static final String TEST9_OK =
"public class Foo {" + PMD.EOL +
" public void foo(String a) {" + PMD.EOL +
" try {" + PMD.EOL +
" int i = Integer.parseInt(a);" + PMD.EOL +
" } catch(Exception e){" + PMD.EOL +
" throw (Error)e.fillInStackTrace();" + PMD.EOL +
" }" + PMD.EOL +
" }" + PMD.EOL +
"}";
}

View File

@ -1282,4 +1282,36 @@ public class Foo {
</example>
</rule>
<rule name="PreserveStackTrace"
message="Caught exception is rethrown, original stack trace may be lost"
class="net.sourceforge.pmd.rules.design.PreserveStackTrace">
<description>
Throwing a new exception from a catch block without passing the original exception into the
new Exception will cause the true stack trace to be lost, and can cause problems
debugging problems
</description>
<properties></properties>
<priority>3</priority>
<example>
<![CDATA[
public class Foo {
void good() {
try{
Integer.parseInt("a");
} catch(Exception e){
throw new Exception(e);
}
}
void bad() {
try{
Integer.parseInt("a");
} catch(Exception e){
throw new Exception(e.getMessage());
}
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -7,6 +7,7 @@ This ruleset contains links to rules that are new in PMD v3.7
</description>
<rule ref="rulesets/basic-jsp.xml/DuplicateJspImports"/>
<rule ref="rulesets/design.xml/PreserveStackTrace"/>
</ruleset>

View File

@ -0,0 +1,59 @@
package net.sourceforge.pmd.rules.design;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.ast.ASTArgumentList;
import net.sourceforge.pmd.ast.ASTCastExpression;
import net.sourceforge.pmd.ast.ASTCatchStatement;
import net.sourceforge.pmd.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.ast.ASTThrowStatement;
import net.sourceforge.pmd.ast.SimpleNode;
import java.util.Iterator;
import java.util.List;
import org.jaxen.JaxenException;
public class PreserveStackTrace extends AbstractRule {
public Object visit(ASTCatchStatement node, Object data) {
String target = (((SimpleNode) node.jjtGetChild(0).jjtGetChild(1)).getImage());
List lstThrowStatements = node.findChildrenOfType(ASTThrowStatement.class);
for (Iterator iter = lstThrowStatements.iterator(); iter.hasNext();) {
ASTThrowStatement throwStatement = (ASTThrowStatement) iter.next();
SimpleNode sn = (SimpleNode)throwStatement.jjtGetChild(0).jjtGetChild(0);
if(sn.getClass().equals(ASTCastExpression.class)){
ASTPrimaryExpression expr = (ASTPrimaryExpression)sn.jjtGetChild(1);
if(expr.jjtGetNumChildren()> 1 && expr.jjtGetChild(1).getClass().equals(ASTPrimaryPrefix.class)) {
RuleContext ctx = (RuleContext) data;
addViolation(ctx, throwStatement);
}
continue;
}
ASTArgumentList args = (ASTArgumentList) throwStatement.getFirstChildOfType(ASTArgumentList.class);
if (args != null) {
try {
List lst = args.findChildNodesWithXPath("//Name[@Image='" + target + "']");
if (lst.size() == 0) {
RuleContext ctx = (RuleContext) data;
addViolation(ctx, throwStatement);
}
} catch (JaxenException e) {
e.printStackTrace();
}
} else if (args == null) {
SimpleNode child = (SimpleNode) throwStatement.jjtGetChild(0);
while (child != null && child.jjtGetNumChildren() > 0 && !child.getClass().getName().equals("net.sourceforge.pmd.ast.ASTName")) {
child = (SimpleNode) child.jjtGetChild(0);
}
if (child != null && (!target.equals(child.getImage()) && !child.getImage().equals(target + ".fillInStackTrace"))) {
RuleContext ctx = (RuleContext) data;
addViolation(ctx, throwStatement);
}
}
}
return super.visit(node, data);
}
}