Merge branch 'pr-1847'

wangjie66 · Jun 14, 2019 · 81447e8 · 81447e8
2 parents 2267413 + ef90a7c
commit 81447e8
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 5 deletions.
diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
@@ -47,6 +47,7 @@ datetime field.
     *   [#1848](https://github.com/pmd/pmd/issues/1848): \[java] Local classes should preserve their modifiers
 *   java-bestpractices
     *   [#1703](https://github.com/pmd/pmd/issues/1703): \[java] UnusedPrivateField on member annotated with lombok @Delegate
+    *   [#1845](https://github.com/pmd/pmd/issues/1845): \[java] Regression in MethodReturnsInternalArray not handling enums
 *   java-multithreading
     *   [#1814](https://github.com/pmd/pmd/issues/1814): \[java] UnsynchronizedStaticFormatter documentation and implementation wrong
     *   [#1815](https://github.com/pmd/pmd/issues/1815): \[java] False negative in UnsynchronizedStaticFormatter
@@ -85,6 +86,7 @@ of deprecations.
 *   [#1792](https://github.com/pmd/pmd/pull/1792): \[java] Added lombok.experimental to AbstractLombokAwareRule - [jakivey32](https://github.com/jakivey32)
 *   [#1808](https://github.com/pmd/pmd/pull/1808): \[plsql] Fix PL/SQL Syntax errors - [kabroxiko](https://github.com/kabroxiko)
 *   [#1829](https://github.com/pmd/pmd/pull/1829): \[java] Fix false negative in UnsynchronizedStaticFormatter - [Srinivasan Venkatachalam](https://github.com/Srini1993)
+*   [#1847](https://github.com/pmd/pmd/pull/1847): \[java] Regression in MethodReturnsInternalArray not handling enums - [Artem](https://github.com/KroArtem)
 *   [#1863](https://github.com/pmd/pmd/pull/1863): \[plsql] Add Table InlineConstraint - [kabroxiko](https://github.com/kabroxiko)
 *   [#1864](https://github.com/pmd/pmd/pull/1864): \[plsql] Add support for Subquery Views - [kabroxiko](https://github.com/kabroxiko)
 *   [#1865](https://github.com/pmd/pmd/pull/1865): \[plsql] Add Support for Extract Expression - [kabroxiko](https://github.com/kabroxiko)

diff --git a/...src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractSunSecureRule.java b/...src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractSunSecureRule.java
@@ -6,8 +6,8 @@
 
 import java.util.List;
 
-import net.sourceforge.pmd.lang.ast.AbstractNode;
 import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
@@ -38,7 +38,7 @@ public abstract class AbstractSunSecureRule extends AbstractJavaRule {
      * @return <code>true</code> if there is a field in the type declaration
      *         named varName, <code>false</code> in other case
      */
-    protected final boolean isField(String varName, AbstractNode typeDeclaration) {
+    protected final boolean isField(String varName, ASTAnyTypeDeclaration typeDeclaration) {
         final List<ASTFieldDeclaration> fds = typeDeclaration.findDescendantsOfType(ASTFieldDeclaration.class);
         if (fds != null) {
             for (ASTFieldDeclaration fd : fds) {

diff --git a/...java/net/sourceforge/pmd/lang/java/rule/bestpractices/MethodReturnsInternalArrayRule.java b/...java/net/sourceforge/pmd/lang/java/rule/bestpractices/MethodReturnsInternalArrayRule.java
@@ -8,9 +8,9 @@
 
 import org.jaxen.JaxenException;
 
-import net.sourceforge.pmd.lang.ast.AbstractNode;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTArrayInitializer;
 import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTExpression;
@@ -46,7 +46,7 @@ public Object visit(ASTMethodDeclaration method, Object data) {
             return data;
         }
         List<ASTReturnStatement> returns = method.findDescendantsOfType(ASTReturnStatement.class);
-        ASTClassOrInterfaceDeclaration td = method.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
+        ASTAnyTypeDeclaration td = method.getFirstParentOfType(ASTAnyTypeDeclaration.class);
         for (ASTReturnStatement ret : returns) {
             final String vn = getReturnedVariableName(ret);
             if (!isField(vn, td)) {
@@ -111,7 +111,7 @@ private boolean hasArraysCopyOf(ASTReturnStatement ret) {
         return false;
     }
 
-    private boolean isEmptyArray(String varName, AbstractNode typeDeclaration) {
+    private boolean isEmptyArray(String varName, ASTAnyTypeDeclaration typeDeclaration) {
         final List<ASTFieldDeclaration> fds = typeDeclaration.findDescendantsOfType(ASTFieldDeclaration.class);
         if (fds != null) {
             for (ASTFieldDeclaration fd : fds) {

diff --git a/...urces/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/MethodReturnsInternalArray.xml b/...urces/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/MethodReturnsInternalArray.xml
@@ -239,4 +239,146 @@ public class Outer {
         ]]></code>
     </test-code>
 
+    <test-code>
+        <description>#1845 Regression in MethodReturnsInternalArray not handling enums</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Arrays;
+public enum MethodReturnsInternalArrayCaseEnum {
+    ONE("One"),
+    TWO("Two", "Three");
+
+    private String[] titles;
+
+    MethodReturnsInternalArrayCaseEnum(String... titles) {
+        this.titles = Arrays.copyOf(titles, titles.length);
+    }
+
+    public String[] getTitles() {
+        return titles.clone();
+    }
+
+    @Override
+    public String toString() {
+        return titles[0];
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Inner interface</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public interface OuterInterface {
+       interface InnerInterface {
+            String[] method();
+       }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Inner annotation</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Retention;
+public final class I {
+    private I() {
+    }
+    @Retention(RetentionPolicy.RUNTIME)
+    public static @interface Inner {
+        String[] value();
+    }
+}
+        ]]>
+        </code>
+    </test-code>
+
+    <test-code>
+        <description>Top-level annotation</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface TopLevelAnnotation {
+
+    String[] value();
+
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Enum that returns array instead of copy</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public enum MethodReturnsInternalArrayCaseEnum {
+    ONE("One"),
+    TWO("Two", "Three");
+
+    private String[] titles;
+
+    MethodReturnsInternalArrayCaseEnum(String... titles) {
+        this.titles = titles;
+    }
+
+    public String[] getTitles() {
+        return titles;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Inner enum that returns array instead of copy</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class OuterClass {
+
+    public enum InnerEnum {
+        INNER_ENUM("first", "second");
+        private String[] titles;
+
+        InnerEnum(String... titles) {
+            this.titles = titles;
+        }
+
+        public String[] getTitles() {
+            return titles;
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Inner enum that returns copy of array</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.Arrays;
+public class OuterClass {
+
+    public enum InnerEnum {
+        INNER_ENUM("first", "second");
+        private String[] titles;
+
+        InnerEnum(String... titles) {
+            this.titles = titles;
+        }
+
+        public String[] getTitles() {
+            return Arrays.copyOf(titles, titles.length);
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
 </test-data>