Issue #6301: ArrayTypeStyle support for method definitions

checkstyle · Feb 22, 2019 · 404143c · 404143c
1 parent 96483bd
commit 404143c
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 14 deletions.
diff --git a/...m/google/checkstyle/test/chapter4formatting/rule4832nocstylearray/ArrayTypeStyleTest.java b/...m/google/checkstyle/test/chapter4formatting/rule4832nocstylearray/ArrayTypeStyleTest.java
@@ -41,8 +41,11 @@ public void testArrayTypeStyle() throws Exception {
             "15:44: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
             "21:20: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
             "22:23: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
-            "41:16: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
-            "42:19: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
+            "41:33: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
+            "46:36: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
+            "52:27: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
+            "62:16: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
+            "63:19: " + getCheckMessage(ArrayTypeStyleCheck.class, MSG_KEY),
         };
 
         final Configuration checkConfig = getModuleConfig("ArrayTypeStyle");

diff --git a/.../google/checkstyle/test/chapter4formatting/rule4832nocstylearray/InputArrayTypeStyle.java b/.../google/checkstyle/test/chapter4formatting/rule4832nocstylearray/InputArrayTypeStyle.java
@@ -29,14 +29,35 @@ public class Test
 
         public Test[]
             getTests()
-        { // we shouldn't check methods because there is no alternatives.
+        {
             return null;
         }
 
         public Test[] getNewTest() //ok
         {
             return null;
         }
+
+        public Test getOldTest()[] //warn
+        {
+            return null;
+        }
+
+        public Test getOldTests()[][] //warn
+        {
+            return null;
+        }
+
+        public Test[]
+            getMoreTests()[] //warn
+        {
+            return null;
+        }
+
+        public Test[][] getTests2() //ok
+        {
+            return null;
+        }
     }
     int[] array[] = new int [2][2]; //warn
     int array2[][][] = new int[3][3][3]; //warn

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java
@@ -27,9 +27,13 @@
 /**
  * Checks the style of array type definitions.
  * Some like Java-style: {@code public static void main(String[] args)}
- * and some like C-style: public static void main(String args[])
+ * and some like C-style: {@code public static void main(String args[])}.
  *
- * <p>By default the Check enforces Java style.
+ * <p>By default the Check enforces Java style.</p>
+ *
+ * <p>This check strictly enforces only Java style for method return types
+ * regardless of the value for 'javaStyle'. For example, {@code byte[] getData()}.
+ * This is because C doesn't compile methods with array declarations on the name.</p>
  */
 @StatelessCheck
 public class ArrayTypeStyleCheck extends AbstractCheck {
@@ -61,17 +65,18 @@ public int[] getRequiredTokens() {
     @Override
     public void visitToken(DetailAST ast) {
         final DetailAST typeAST = ast.getParent();
-        if (typeAST.getType() == TokenTypes.TYPE
-                // Do not check method's return type.
-                // We have no alternatives here.
-                && typeAST.getParent().getType() != TokenTypes.METHOD_DEF) {
+        if (typeAST.getType() == TokenTypes.TYPE) {
             final DetailAST variableAST = typeAST.getNextSibling();
             if (variableAST != null) {
-                final boolean isJavaStyle =
-                    variableAST.getLineNo() > ast.getLineNo()
+                final boolean isMethod = typeAST.getParent().getType() == TokenTypes.METHOD_DEF;
+                final boolean isJavaStyle = variableAST.getLineNo() > ast.getLineNo()
                     || variableAST.getColumnNo() - ast.getColumnNo() > -1;
 
-                if (isJavaStyle != javaStyle) {
+                // force all methods to be Java style (see note in top Javadoc)
+                final boolean isMethodViolation = isMethod && !isJavaStyle;
+                final boolean isVariableViolation = !isMethod && isJavaStyle != javaStyle;
+
+                if (isMethodViolation || isVariableViolation) {
                     log(ast, MSG_KEY);
                 }
             }

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheckTest.java
@@ -54,6 +54,9 @@ public void testJavaStyleOn()
             "14:23: " + getCheckMessage(MSG_KEY),
             "15:18: " + getCheckMessage(MSG_KEY),
             "21:44: " + getCheckMessage(MSG_KEY),
+            "45:33: " + getCheckMessage(MSG_KEY),
+            "50:36: " + getCheckMessage(MSG_KEY),
+            "56:29: " + getCheckMessage(MSG_KEY),
         };
         verify(checkConfig, getPath("InputArrayTypeStyle.java"), expected);
     }
@@ -69,6 +72,9 @@ public void testJavaStyleOff()
             "17:39: " + getCheckMessage(MSG_KEY),
             "23:18: " + getCheckMessage(MSG_KEY),
             "31:20: " + getCheckMessage(MSG_KEY),
+            "45:33: " + getCheckMessage(MSG_KEY),
+            "50:36: " + getCheckMessage(MSG_KEY),
+            "56:29: " + getCheckMessage(MSG_KEY),
         };
         verify(checkConfig, getPath("InputArrayTypeStyle.java"), expected);
     }

diff --git a/.../resources/com/puppycrawl/tools/checkstyle/checks/arraytypestyle/InputArrayTypeStyle.java b/.../resources/com/puppycrawl/tools/checkstyle/checks/arraytypestyle/InputArrayTypeStyle.java
@@ -33,14 +33,35 @@ public class Test
 
         public Test[]
             getTests()
-        { // we shouldn't check methods because there is no alternatives.
+        {
             return null;
         }
 
         public Test[] getNewTest()
         {
             return null;
         }
+
+        public Test getOldTest()[]
+        {
+            return null;
+        }
+
+        public Test getOldTests()[][]
+        {
+            return null;
+        }
+
+        public Test[]
+            getMoreTests()[][]
+        {
+            return null;
+        }
+
+        public Test[][] getTests2()
+        {
+            return null;
+        }
     }
     public static void foo(java.util.Collection < ? extends InputArrayTypeStyle[] > collection) {}
 }
diff --git a/src/xdocs/config_misc.xml b/src/xdocs/config_misc.xml
@@ -28,7 +28,15 @@
         <p>
           Checks the style of array type definitions.  Some like Java style:
           <code>public static void main(String[] args)</code> and some like
-          C style: public static void main(String args[])
+          C style: <code>public static void main(String args[])</code>.
+        </p>
+        <p>
+          By default the Check enforces Java style.
+        </p>
+        <p>
+          This check strictly enforces only Java style for method return types
+          regardless of the value for 'javaStyle'. For example, <code>byte[] getData()</code>.
+          This is because C doesn't compile methods with array declarations on the name.
         </p>
       </subsection>
 
@@ -60,7 +68,23 @@
         <source>
 &lt;module name=&quot;ArrayTypeStyle&quot;/&gt;
         </source>
+        <p>
+          Example:
+        </p>
+        <source>
+public class MyClass {
+  int[] nums; // OK
+  String strings[]; // violation
+
+  char[] toCharArray() { // OK
+    return null;
+  }
 
+  byte getData()[] { // violation
+    return null;
+  }
+}
+        </source>
         <p>
           To configure the check to enforce C style:
         </p>
@@ -69,6 +93,23 @@
   &lt;property name=&quot;javaStyle&quot; value=&quot;false&quot;/&gt;
 &lt;/module&gt;
         </source>
+        <p>
+          Example:
+        </p>
+        <source>
+public class MyClass {
+  int[] nums; // violation
+  String strings[]; // OK
+
+  char[] toCharArray() { // OK
+    return null;
+  }
+
+  byte getData()[] { // violation
+    return null;
+  }
+}
+        </source>
       </subsection>
 
       <subsection name="Example of Usage" id="ArrayTypeStyle_Example_of_Usage">