Merge pull request #3704 from dykov:hotfix/3686

[java] Fix for #3686 - Fix ReturnEmptyCollectionRatherThanNull #3704

* pr-3704:
  Add @dykov as a contributor
  [doc] Update release notes (#3686, #3704)
  Fix testcase
  Fix ReturnEmptyCollectionRatherThanNull

.all-contributorsrc

@@ -6511,6 +6511,15 @@
       "contributions": [
         "code"
       ]
+    },
+    {
+      "login": "dykov",
+      "name": "Oleksii Dykov",
+      "avatar_url": "https://avatars.githubusercontent.com/u/36415196?v=4",
+      "profile": "https://github.com/dykov",
+      "contributions": [
+        "code"
+      ]
     }
   ],
   "contributorsPerLine": 7,

docs/pages/release_notes.md

@@ -25,6 +25,8 @@ This is a {{ site.pmd.release_type }} release.
 *   java-bestpractices
     *   [#3209](https://github.com/pmd/pmd/issues/3209): \[java] UnusedPrivateMethod false positive with static method and cast expression
     *   [#3468](https://github.com/pmd/pmd/issues/3468): \[java] UnusedPrivateMethod false positive when outer class calls private static method on inner class
+*   java-errorprone
+    *   [#3686](https://github.com/pmd/pmd/issues/3686): \[java] ReturnEmptyCollectionRatherThanNull - false negative with conditioned returns
 *   java-performance
     *   [#3492](https://github.com/pmd/pmd/issues/3492): \[java] UselessStringValueOf: False positive when there is no initial String to append to
 
@@ -36,6 +38,7 @@ This is a {{ site.pmd.release_type }} release.
 *   [#3683](https://github.com/pmd/pmd/pull/3683): \[java] Fixed 3468 UnusedPrivateMethod false positive when outer class calls private static method on inner class - [John Armgardt](https://github.com/johnra2)
 *   [#3688](https://github.com/pmd/pmd/pull/3688): \[java] Bump log4j to 2.16.0 - [Sergey Nuyanzin](https://github.com/snuyanzin)
 *   [#3693](https://github.com/pmd/pmd/pull/3693): \[apex] ApexDoc: Add reportProperty property - [Steve Babula](https://github.com/babula)
+*   [#3704](https://github.com/pmd/pmd/pull/3704): \[java] Fix for #3686 - Fix ReturnEmptyCollectionRatherThanNull - [Oleksii Dykov](https://github.com/dykov)
 *   [#3713](https://github.com/pmd/pmd/pull/3713): \[java] Enhance UnnecessaryModifier to support records - [Vincent Galloy](https://github.com/vgalloy)
 *   [#3719](https://github.com/pmd/pmd/pull/3719): \[java] Upgrade log4j to 2.17.1 - [Daniel Paul Searles](https://github.com/squaresurf)
 

pmd-java/src/main/resources/category/java/errorprone.xml

@@ -2925,10 +2925,12 @@ See Effective Java, 3rd Edition, Item 54: Return empty collections or arrays ins
 <![CDATA[
 //MethodDeclaration
 [
-(./ResultType/Type[pmd-java:typeIs('java.util.Collection') or pmd-java:typeIs('java.util.Map') or @ArrayType=true()])
-and
-(./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral)
+    (./ResultType/Type[pmd-java:typeIs('java.util.Collection')
+    or pmd-java:typeIs('java.util.Map')
+    or @ArrayType=true()])
 ]
+//ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral
+[not(./ancestor::StatementExpression)]
 ]]>
                 </value>
             </property>

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ReturnEmptyCollectionRatherThanNull.xml

@@ -33,7 +33,7 @@ public class Foo {
    <test-code>
       <description>Returning null instead of collection (List)</description>
       <expected-problems>1</expected-problems>
-      <expected-linenumbers>5</expected-linenumbers>
+      <expected-linenumbers>8</expected-linenumbers>
       <code><![CDATA[
 import java.util.List;
 
@@ -67,7 +67,7 @@ public class Foo {
    <test-code>
       <description>Returning null instead of collection (Set)</description>
       <expected-problems>1</expected-problems>
-      <expected-linenumbers>5</expected-linenumbers>
+      <expected-linenumbers>8</expected-linenumbers>
       <code><![CDATA[
 import java.util.Set;
 
@@ -84,7 +84,7 @@ public class Foo {
    <test-code>
       <description>Returning null instead of collection (Map)</description>
       <expected-problems>1</expected-problems>
-      <expected-linenumbers>5</expected-linenumbers>
+      <expected-linenumbers>8</expected-linenumbers>
       <code><![CDATA[
 import java.util.Map;
 
@@ -98,4 +98,80 @@ public class Foo {
 }
         ]]></code>
    </test-code>
+
+  <test-code>
+    <description>False negative case #3686</description>
+    <expected-problems>2</expected-problems>
+    <expected-linenumbers>4, 6</expected-linenumbers>
+    <code><![CDATA[
+public class Foo {
+    public int[] foo() {
+     if(true) {
+         return null;   //violation
+     } else {
+         return null;   //violation
+     }
+   }
+}
+    ]]></code>
+  </test-code>
+
+  <test-code>
+    <description>Null is returned from an internal statement, not from the method</description>
+    <expected-problems>0</expected-problems>
+    <code><![CDATA[
+public class Foo {
+
+    private int[] foo() {
+        bar(new SomeInterface() {
+            @Override
+            public Object run() {
+                baz();
+                return null;
+            }
+        });
+
+        return new int[0];
+    }
+}
+    ]]></code>
+  </test-code>
+
+  <test-code>
+    <description>Null is returned from an internal statement, not from the method</description>
+    <expected-problems>0</expected-problems>
+    <code><![CDATA[
+public class Foo {
+
+    private int[] foo() {
+        bar(() -> {
+            baz();
+            return null;
+        });
+
+        return new int[0];
+    }
+}
+    ]]></code>
+  </test-code>
+
+  <test-code>
+    <description>Null is returned from a loop</description>
+    <expected-problems>1</expected-problems>
+    <expected-linenumbers>7</expected-linenumbers>
+    <code><![CDATA[
+  import java.util.List;
+
+  public class Foo {
+      private List<Integer> foo(List<Integer> list) {
+          for(Integer i : list) {
+              if(condition) {
+                  return null;
+              }
+          }
+          return list;
+      }
+  }
+    ]]></code>
+  </test-code>
 </test-data>