Fix from Jason Bennett: Fixed bug 1371753 - UnnecessaryLocalBeforeReturn is now less aggressive in its reporting.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4641 51baf565-9d33-0410-a72c-fc3788e3496d

pmd/etc/changelog.txt

@@ -10,6 +10,7 @@ Fixed CSVRenderer - had flipped line and priority columns
 Applied patch 1551189 - SingularField false + for initialization blocks
 Fixed bug 1573795 - PreserveStackTrace doesn't throw CastClassException on exception with 0 args
 Fixed bug 1573591 - NonThreadSafeSingleton doesn't throw NPE when using this keyword
+Fixed bug 1371753 - UnnecessaryLocalBeforeReturn is now less aggressive in its reporting.
 CloseResource rule now checks code without java.sql import.
 Applied patch 1573981 - false + in CloneMethodMustImplementCloneable
 Applied patch 1574988 - false + in OverrideBothEqualsAndHashcode

pmd/regress/test/net/sourceforge/pmd/rules/design/UnnecessaryLocalBeforeReturnRuleTest.java

@@ -19,6 +19,7 @@ public class UnnecessaryLocalBeforeReturnRuleTest extends SimpleAggregatorTst {
             new TestDescriptor(TEST2, "skip literal returns", 0, rule),
             new TestDescriptor(TEST3, "simple failure case", 1, rule),
             new TestDescriptor(TEST4, "skip complicated returns", 0, rule),
+            new TestDescriptor(TEST5, "skip method calls", 0, rule),
         });
     }
 
@@ -50,4 +51,11 @@ public class UnnecessaryLocalBeforeReturnRuleTest extends SimpleAggregatorTst {
             " }" + PMD.EOL +
             "}";
 
+    private static final String TEST5 =
+            "public class Foo {" + PMD.EOL +
+            " public int bar() {" + PMD.EOL +
+            "  return doSomething(a, b, c);" + PMD.EOL +
+            " }" + PMD.EOL +
+            "}";
+
 }

pmd/src/net/sourceforge/pmd/rules/design/UnnecessaryLocalBeforeReturn.java

@@ -1,18 +1,19 @@
 package net.sourceforge.pmd.rules.design;
 
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
 import net.sourceforge.pmd.AbstractRule;
 import net.sourceforge.pmd.ast.ASTExpression;
 import net.sourceforge.pmd.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.ast.ASTName;
 import net.sourceforge.pmd.ast.ASTPrimaryExpression;
+import net.sourceforge.pmd.ast.ASTPrimarySuffix;
 import net.sourceforge.pmd.ast.ASTReturnStatement;
 import net.sourceforge.pmd.symboltable.NameOccurrence;
 import net.sourceforge.pmd.symboltable.VariableNameDeclaration;
 
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
 public class UnnecessaryLocalBeforeReturn extends AbstractRule {
 
     public Object visit(ASTMethodDeclaration meth, Object data) {
@@ -31,7 +32,7 @@ public class UnnecessaryLocalBeforeReturn extends AbstractRule {
         }
 
         // skip 'complicated' expressions
-        if (rtn.findChildrenOfType(ASTExpression.class).size() > 1 || rtn.findChildrenOfType(ASTPrimaryExpression.class).size() > 1) {
+        if (rtn.findChildrenOfType(ASTExpression.class).size() > 1 || rtn.findChildrenOfType(ASTPrimaryExpression.class).size() > 1 || isMethodCall(rtn)) {
             return data;
         }
 
@@ -55,4 +56,22 @@ public class UnnecessaryLocalBeforeReturn extends AbstractRule {
         }
         return data;
     }
+    
+    /**
+     * Determine if the given return statement has any embedded method calls.
+     * 
+     * @param rtn
+     *          return statement to analyze
+     * @return true if any method calls are made within the given return
+     */
+    private boolean isMethodCall(ASTReturnStatement rtn) {
+     List suffix = rtn.findChildrenOfType( ASTPrimarySuffix.class );
+     for ( Iterator iter = suffix.iterator(); iter.hasNext(); ) {
+        ASTPrimarySuffix element = (ASTPrimarySuffix) iter.next();
+        if ( element.isArguments() ) {
+          return true;
+        }
+      }
+      return false;
+    }
 }

pmd/xdocs/credits.xml

@@ -52,7 +52,7 @@
         </subsection>
         <subsection name="Contributors">
         <ul>
-            <li>Jason Bennett - Wrote NPathPatches to improve CyclomaticComplexity rule</li>
+            <li>Jason Bennett - Fix for UnnecessaryLocalBeforeReturn, wrote NPathComplexity rule, patches to improve CyclomaticComplexity rule</li>
             <li>Brent Fisher - SummaryHTML report improvements</li>
             <li>George Thomas - Wrote AvoidRethrowingException rule</li>
             <li>Robert Simmons - Reported bug in optimizations package along with suggestions for fix</li>