From 6899c63597fcf5060282bbffa0e61c21cecaba78 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <adangel@users.sourceforge.net>
Date: Sat, 28 Jan 2017 11:45:59 +0100
Subject: [PATCH] Fixes #1495 [java] UnnecessaryLocalBeforeReturn with assert

---
 .../UnnecessaryLocalBeforeReturnRule.java     | 36 +++++++++++++++++++
 .../xml/UnnecessaryLocalBeforeReturn.xml      | 13 +++++++
 src/site/markdown/overview/changelog.md       |  1 +
 3 files changed, 50 insertions(+)

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UnnecessaryLocalBeforeReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UnnecessaryLocalBeforeReturnRule.java
index 198c4cebdb..5dc882ab03 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UnnecessaryLocalBeforeReturnRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UnnecessaryLocalBeforeReturnRule.java
@@ -6,6 +6,9 @@ package net.sourceforge.pmd.lang.java.rule.design;
 import java.util.List;
 import java.util.Map;
 
+import net.sourceforge.pmd.lang.java.ast.ASTAssertStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTBlock;
+import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTName;
@@ -44,6 +47,12 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
         for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry: vars.entrySet()) {
             VariableNameDeclaration key = entry.getKey();
             List<NameOccurrence> usages = entry.getValue();
+
+            // skip, if there is an assert between declaration and return
+            if (hasAssertStatement(key, rtn)) {
+                continue;
+            }
+
             for (NameOccurrence occ: usages) {
                 if (occ.getLocation().equals(name)) {
                     // only check declarations that occur one line earlier
@@ -76,4 +85,31 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
       }
       return false;
     }
+
+    /**
+     * Checks whether there is an assert statement between the variable declaration
+     * and the return statement, that uses the variable.
+     * @param variableDeclaration
+     * @param rtn
+     * @return
+     */
+    private boolean hasAssertStatement(VariableNameDeclaration variableDeclaration, ASTReturnStatement rtn) {
+        boolean hasAssert = false;
+        ASTBlockStatement blockStatement = variableDeclaration.getAccessNodeParent().getFirstParentOfType(ASTBlockStatement.class);
+        int startIndex = blockStatement.jjtGetChildIndex() + 1;
+        int endIndex = rtn.getFirstParentOfType(ASTBlockStatement.class).jjtGetChildIndex();
+        ASTBlock block = (ASTBlock)blockStatement.jjtGetParent();
+        for (int i = startIndex; i < endIndex; i++) {
+            List<ASTAssertStatement> asserts = block.jjtGetChild(i).findDescendantsOfType(ASTAssertStatement.class);
+            for (ASTAssertStatement assertStatement : asserts) {
+                List<ASTName> names = assertStatement.findDescendantsOfType(ASTName.class);
+                for (ASTName n : names) {
+                    if (n.hasImageEqualTo(variableDeclaration.getName())) {
+                        hasAssert = true;
+                    }
+                }
+            }
+        }
+        return hasAssert;
+    }
 }
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UnnecessaryLocalBeforeReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UnnecessaryLocalBeforeReturn.xml
index ac3ae07dcf..15297a73e9 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UnnecessaryLocalBeforeReturn.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UnnecessaryLocalBeforeReturn.xml
@@ -65,4 +65,17 @@ public class Foo {
 }
      ]]></code>
     </test-code>
+
+    <test-code>
+        <description>#1495 [java] UnnecessaryLocalBeforeReturn with assert</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    public int bar() {
+        int res = 2; assert res>=0;
+        return res;
+    }
+}
+        ]]></code>
+    </test-code>
 </test-data>
diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md
index c55a3006f8..fc9ed4d4fd 100644
--- a/src/site/markdown/overview/changelog.md
+++ b/src/site/markdown/overview/changelog.md
@@ -48,6 +48,7 @@ See also [bugfix #1556](https://sourceforge.net/p/pmd/bugs/1556/).
     *   [#213](https://github.com/pmd/pmd/issues/213): \[java] CPD: OutOfMemory when analyzing Lucene
 *   java-design
     *   [#1448](https://sourceforge.net/p/pmd/bugs/1448/): \[java] ImmutableField: Private field in inner class gives false positive with lambdas
+    *   [#1495](https://sourceforge.net/p/pmd/bugs/1495/): \[java] UnnecessaryLocalBeforeReturn with assert
     *   [#1552](https://sourceforge.net/p/pmd/bugs/1552/): \[java] MissingBreakInSwitch - False positive for continue
     *   [#1556](https://sourceforge.net/p/pmd/bugs/1556/): \[java] UseLocaleWithCaseConversions does not works with `ResultSet` (false negative)
     *   [#177](https://github.com/pmd/pmd/issues/177): \[java] SingularField with lambdas as final fields