Merge branch 'pr-141' into pmd/5.5.x

Closes #141 (rebased for pmd/5.4.x)

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/PreserveStackTraceRule.java

@@ -17,14 +17,12 @@ import net.sourceforge.pmd.lang.java.ast.ASTName;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
 import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
 import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
 import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
 
-import org.jaxen.JaxenException;
-
 /**
  *
  * @author Unknown,
@@ -33,12 +31,6 @@ import org.jaxen.JaxenException;
  */
 public class PreserveStackTraceRule extends AbstractJavaRule {
 
-    // FUTURE: This detection is name based, it should probably use Type
-    // Resolution, to become type "based"
-    // it assumes the exception class contains 'Exception' in its name
-    private static final String FIND_THROWABLE_INSTANCE = "./VariableInitializer/Expression/PrimaryExpression/PrimaryPrefix/AllocationExpression"
-            + "[ClassOrInterfaceType[contains(@Image,'Exception')] and Arguments[count(*)=0]]";
-
     private static final String FILL_IN_STACKTRACE = ".fillInStackTrace";
 
     @Override
@@ -82,10 +74,17 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
                             List<NameOccurrence> occurrences = entry.getValue();
                             if (decl.getImage().equals(child.getImage())) {
                                 if (!isInitCauseCalled(target, occurrences)) {
-                                    args = decl.getNode().jjtGetParent()
+                                    // Check how the variable is initialized
-                                            .getFirstDescendantOfType(ASTArgumentList.class);
+                                    ASTVariableInitializer initializer = decl.getNode().jjtGetParent()
-                                    if (args != null) {
+                                            .getFirstDescendantOfType(ASTVariableInitializer.class);
-                                        ck(data, target, throwStatement, args);
+                                    if (initializer != null) {
+                                        args = initializer.getFirstDescendantOfType(ASTArgumentList.class);
+                                        if (args != null) {
+                                            // constructor with args?
+                                            ck(data, target, throwStatement, args);
+                                        } else if (!isFillInStackTraceCalled(target, initializer)) {
+                                            addViolation(data, throwStatement);
+                                        }
                                     }
                                 }
                             }
@@ -99,6 +98,11 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
         return super.visit(catchStmt, data);
     }
 
+    private boolean isFillInStackTraceCalled(final String target, final ASTVariableInitializer initializer) {
+        final ASTName astName = initializer.getFirstDescendantOfType(ASTName.class);
+        return astName != null && astName.hasImageEqualTo(target + FILL_IN_STACKTRACE);
+    }
+
     private boolean isInitCauseCalled(String target, List<NameOccurrence> occurrences) {
         boolean initCauseCalled = false;
         for (NameOccurrence occurrence : occurrences) {
@@ -121,49 +125,6 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
         return initCauseCalled;
     }
 
-    @Override
-    public Object visit(ASTVariableDeclarator node, Object data) {
-        // Search Catch stmt nodes for variable used to store improperly created
-        // throwable or exception
-        try {
-            if (node.hasDescendantMatchingXPath(FIND_THROWABLE_INSTANCE)) {
-                String variableName = node.jjtGetChild(0).getImage(); // VariableDeclatorId
-                ASTCatchStatement catchStmt = node.getFirstParentOfType(ASTCatchStatement.class);
-
-                while (catchStmt != null) {
-                    List<Node> violations = catchStmt
-                            .findChildNodesWithXPath("//Expression/PrimaryExpression/PrimaryPrefix/Name[@Image = '"
-                                    + variableName + "']");
-                    if (!violations.isEmpty()) {
-                        // If, after this allocation, the 'initCause' method is
-                        // called, and the ex passed
-                        // this is not a violation
-                        if (!useInitCause(violations.get(0), catchStmt)) {
-                            addViolation(data, node);
-                        }
-                    }
-
-                    // check ASTCatchStatement higher up
-                    catchStmt = catchStmt.getFirstParentOfType(ASTCatchStatement.class);
-                }
-            }
-            return super.visit(node, data);
-        } catch (JaxenException e) {
-            // XPath is valid, this should never happens...
-            throw new IllegalStateException(e);
-        }
-    }
-
-    private boolean useInitCause(Node node, ASTCatchStatement catchStmt) {
-        // In case of NPE...
-        if (node != null && node.getImage() != null) {
-            return catchStmt
-                    .hasDescendantMatchingXPath("./Block/BlockStatement/Statement/StatementExpression/PrimaryExpression/PrimaryPrefix/Name[@Image = '"
-                            + node.getImage() + ".initCause']");
-        }
-        return false;
-    }
-
     /**
      * Checks whether the given target is in the argument list. If this is the
      * case, then the target (root exception) is used as the cause.

src/site/markdown/overview/changelog.md

@@ -38,6 +38,7 @@
 *   [#137](https://github.com/pmd/pmd/pull/137): \[apex] Adjusted remediation points
 *   [#138](https://github.com/pmd/pmd/pull/138): \[java] Make ClasspathClassLoader parallel capable
 *   [#140](https://github.com/pmd/pmd/pull/140): \[java] Make CloneMethodMustImplementCloneable over 500x faster
+*   [#141](https://github.com/pmd/pmd/pull/141): \[java] Speedup PreserveStackTraceRule by over 7X
 *   [#146](https://github.com/pmd/pmd/pull/146): \[apex] Detection of missing Apex CRUD checks for SOQL/DML operations
 *   [#147](https://github.com/pmd/pmd/pull/147): \[apex] Adding XSS detection to return statements
 *   [#148](https://github.com/pmd/pmd/pull/148): \[apex] Improving detection of SOQL injection