diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index ef667bd519..6d1c0b8a12 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -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 diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/design/PreserveStackTraceTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/design/PreserveStackTraceTest.java new file mode 100644 index 0000000000..9b98125d4e --- /dev/null +++ b/pmd/regress/test/net/sourceforge/pmd/rules/design/PreserveStackTraceTest.java @@ -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 + + "}"; +} diff --git a/pmd/rulesets/design.xml b/pmd/rulesets/design.xml index aa3728dcca..a75a43338f 100644 --- a/pmd/rulesets/design.xml +++ b/pmd/rulesets/design.xml @@ -1282,4 +1282,36 @@ public class Foo { + + +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 + + + 3 + + + + + diff --git a/pmd/rulesets/releases/37.xml b/pmd/rulesets/releases/37.xml index e2a4db388c..a62be41040 100644 --- a/pmd/rulesets/releases/37.xml +++ b/pmd/rulesets/releases/37.xml @@ -7,6 +7,7 @@ This ruleset contains links to rules that are new in PMD v3.7 + diff --git a/pmd/src/net/sourceforge/pmd/rules/design/PreserveStackTrace.java b/pmd/src/net/sourceforge/pmd/rules/design/PreserveStackTrace.java new file mode 100644 index 0000000000..7aaaf9c377 --- /dev/null +++ b/pmd/src/net/sourceforge/pmd/rules/design/PreserveStackTrace.java @@ -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); + } +}