From 883bed3cf8ac3b108317dc4eb32d3d123fac63c0 Mon Sep 17 00:00:00 2001
From: Alberto Fernandez <infjaf@gmail.com>
Date: Wed, 17 Jan 2018 15:25:38 +0100
Subject: [PATCH] Better detection of corner cases

the firts changes checks for a exact level to search AdditiveExpression.

So code like
throw new Exception("something bad:" + (e)); gets undetected.

This change search if part of a AdditiveExpression to the base node.
---
 .../bestpractices/PreserveStackTraceRule.java | 22 ++++++++++++++++---
 .../bestpractices/xml/PreserveStackTrace.xml  | 10 ++++++---
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java
index 8f8576e905..9aaceef4cb 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PreserveStackTraceRule.java
@@ -140,9 +140,8 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
         if (target != null && baseNode != null) {
             List<ASTName> nameNodes = baseNode.findDescendantsOfType(ASTName.class);
             for (ASTName nameNode : nameNodes) {
-                if (target.equals(nameNode.getImage())) {
-                    boolean isPartOfStringConcatenation = 
-                        (nameNode.jjtGetParent().jjtGetParent().jjtGetParent() instanceof ASTAdditiveExpression);
+                if (target.equals(nameNode.getImage())) {                	
+                    boolean isPartOfStringConcatenation = isStringConcat(nameNode, baseNode);                        
                     if (!isPartOfStringConcatenation) {
                         match = true;
                         break;
@@ -152,6 +151,23 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
         }
         return match;
     }
+    /**
+     * Checks whether the given childNode is part of an additive expression (String concatenation) limiting search to base Node.
+     * @param childNode
+     * @param baseNode
+     * @return
+     */
+    private boolean isStringConcat(Node childNode, Node baseNode) {
+    	Node currentNode = childNode;
+    	// limit to 10 levels
+    	for (int i = 0; i < 10 && currentNode != null && currentNode != baseNode; i++) {
+    		currentNode = currentNode.jjtGetParent();
+    		if (currentNode instanceof ASTAdditiveExpression) {
+    			return true;
+    		}
+    	}
+    	return false;
+    }
 
     private void ck(Object data, String target, ASTThrowStatement throwStatement, Node baseNode) {
         if (!checkForTargetUsage(target, baseNode)) {
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml
index fc95008bd2..1900c10786 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/PreserveStackTrace.xml
@@ -542,14 +542,18 @@ public class Bug {
     
     <test-code>
     <description>#543 False negative with String concatenation</description>
-    <expected-problems>1</expected-problems>
+    <expected-problems>3</expected-problems>
     <code><![CDATA[
 public class Foo {
     public void foo(String a) throws Exception {
         try {
             int i = Integer.parseInt(a);
-        } catch(Exception e){
-            throw new Exception("something bad:" + e);
+        } catch(java.io.FileNotFoundException e) {
+            throw new Exception("file not found:" + e.toString());
+        } catch(java.io.IOException e) {
+            throw new Exception("I/O error:" + e);
+        } catch(Exception e) {
+            throw new Exception("something bad:" + (e));
         }
     }
 }