Repository: checkstyle/checkstyle

Problem statement:
Allow to configure if ArrayTrailingComma should ignore single-line arrays
Currently the [ArrayTrailingComma](http://checkstyle.sourceforge.net/config_coding.html#ArrayTrailingComma) automatically ignores inline declarations. So using a configuration such as:

``` xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
        "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="ArrayTrailingComma"/>
    </module>
</module>
```

will never report on constructs such as:

``` java
public class Foo {
    public int[] getBar() {
        return new int[] { 0 };
        //                  ^ doesn't report missing trailing comma
    }
}
```

``` bash
$ java -jar ~/Downloads/checkstyle-6.18/checkstyle-6.18-all.jar -c config.xml Foo.java
Starting audit...
Audit done.
```

This behaviour is by design and is documented. However, it would be nice if such behaviour could be configured through a property (keeping the current behaviour as default for backwards compatibility), so that we may write a configuration such as:

``` xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
        "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="ArrayTrailingComma">
            <property name="ignoreInlineArrays" value="false"/>
        </module>
    </module>
</module>
```

And have it report a missing trailing comma on the given code, forcing developers to write:

``` java
public class Foo {
    public int[] getBar() {
        return new int[] { 0, };
    }
}
```

Making the new `ignoreInlineArrays` property default to `true` would guarantee backwards compatibility, allowing users to opt-in to this behaviour.
## 



Hints:
connection with @unrealwork  is lost, PR is closed as incomplete.
If smb want to continue from scratch or from where @unrealwork is finished, please be welcome .
@jsotuyod ,

we got PullRequest with fix for your issue, so it bring my attention back to this issue.

can you provide more details on why it is reasonable to ignore single line array declarations ? We need to put reason in documentation.

we need to find better name for property as `ignore.....` usually mean completely skip from consideration but you do expect violation.

@kukreja-vikramaditya , can you suggest good name ? 

from documentation:
> The check demands a comma at the end if neither left nor right curly braces are on the same line as the last element of the array

so there should be violation on a case that @jsotuyod  is provided , it is even show in documentation .... strange ... looks like we have a bug.
The whole point of having a trailing comma is to simplify reordering the array.

In general, and in particular under the current implementation, this is restricted to swapping lines in the IDE / cutting and pasting whole lines in a different order.

However, the cut and paste approach should still work for single line arrays. For instance, given the following array:

```java
int color[] = { BLACK, RED, BLUE, YELLOW, }
```

I can stand *at any one point*, and performing a 3 word cut, go paste it at a different position and the result will be consistent (for all VIM fans out there `3ciw`!). This wouldn't work without the trailing comma after `YELLOW`.

As for the name, I suggested `ignoreInlineArrays` default to true so as to completely ignore them by default (backwards-compatible), and I would only expect a violation when set to `false`. I believe this is right according to your own reasoning.
@jsotuyod , thanks for explanation.

> I believe this is right according to your own reasoning.

yes, thanks for correcting me.

I have some concerns about name of property - `...Inline...`.
To my mind it is better to use smth like `ignoreSameLineDeclaration` ..... but in result and consider your reason ... Why we so focused on single line arrays ?
If follow you reasoning you always want to have comma in array, examples:
```
$ cat MyClass.java 
public class MyClass {
    public int[] getBar() {
        return new int[] { 0 };
        return new int[] { 0, 1 };
        return new int[]
                         { 0 };
        return new int[]
                         { 0, 1 };
        return new int[]
                         { 0, 1,
                           2, 3 // now violation is only there
                         };
    }
}

$ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
        "http://checkstyle.sourceforge.net/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="ArrayTrailingComma">
        </module>
    </module>
</module>

$ java -jar checkstyle-8.1-all.jar -c config.xml MyClass.java 
Starting audit...
[ERROR] /var/tmp/MyClass.java:12: Array should contain trailing comma. [ArrayTrailingComma]
Audit done.
Checkstyle ends with 1 errors.
```

and by your reasoning/logic you will expect violation in all that cases if some option is use, am I right ?
if yes, so we can have property `alwaysDemandTrailingComma`
@romani yes, definitely better. I agree with your reasoning.
@romani 
So I would only need to update the property name to `alwaysDemandTrailingComma` (
**UPDATE 1** which defaults to `false`)
 and update the documentation of the ArrayTrailingComma check? 
>The check demands a comma at the end if neither left nor right curly braces are on the same line as the last element of the array

**UPDATE 1** So if the property name is `alwaysDemandTrailingComma`,  I would only have to detect a `}` and verify a comma exists (which can be ignored in the case of an empty array as per the reasoning given by @jsotuyod ?)
@jsotuyod Do you think we should have special processing for anonymous arrays (like only check them or skip them) ? Or i guess it would be clear more once reports are generated .
@kukreja-vikramaditya I think all arrays should be treated equally.
yes, lets treat all arrays in the same way.

@kukreja-vikramaditya , name of property is good.
if that property is TRUE all you need to make sure that there is `,` at the end of array AST.
Please close your current PR, as it should seriously redone.
@kukreja-vikramaditya , please review why we do not report violation on `return new int[] { 0 };` , html documentation clain there should be violation. Smth is not right code or documentation.
On it
@romani if @kukreja-vikramaditya is not working on this issue, can I take it? 
Since it hasn't been touched for a long time?
@Gaurav-Punjabi yes, you can take this issue
I'm on it.

Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheck.java
@@ -38,7 +38,7 @@
  * };
  * </pre>
  * <p>
- * The check demands a comma at the end if neither left nor right curly braces
+ * By default, the check demands a comma at the end if neither left nor right curly braces
  * are on the same line as the last element of the array.
  * </p>
  * <pre>
@@ -89,6 +89,13 @@
  *  100000000000000000001, // More work needed to duplicate
  * }
  * </pre>
+ * <ul>
+ * <li>
+ * Property {@code alwaysDemandTrailingComma} - Control whether to always check for a trailing
+ * comma, even when an array is inline.
+ * Default value is {@code false}.
+ * </li>
+ * </ul>
  * <p>
  * To configure the check:
  * </p>
@@ -132,6 +139,47 @@
  *   ,};        // no violation
  * </pre>
  *
+ * <p>To configure check to always validate trailing comma:</p>
+ * <pre>
+ * &lt;module name="ArrayTrailingComma"&gt;
+ *   &lt;property name="alwaysDemandTrailingComma" value="true"/&gt;
+ * &lt;/module&gt;
+ * </pre>
+ * <p>Example:</p>
+ * <pre>
+ * int[] numbers = {1, 2, 3}; // violation
+ * boolean[] bools = {
+ * true,
+ * true,
+ * false // violation
+ * };
+ *
+ * String[][] text = {{},{},}; // OK
+ *
+ * double[][] decimals = {
+ * {0.5, 2.3, 1.1,}, // OK
+ * {1.7, 1.9, 0.6}, // violation
+ * {0.8, 7.4, 6.5} // violation
+ * }; // violation, previous line misses a comma
+ *
+ * char[] chars = {'a', 'b', 'c'  // violation
+ *   };
+ *
+ * String[] letters = {
+ *   "a", "b", "c"}; // violation
+ *
+ * int[] a1 = new int[]{
+ *   1,
+ *   2
+ *   ,
+ * }; // OK
+ *
+ * int[] a2 = new int[]{
+ *   1,
+ *   2
+ *   ,}; // OK
+ * </pre>
+ *
  * @since 3.2
  */
 @StatelessCheck
@@ -143,6 +191,20 @@ public class ArrayTrailingCommaCheck extends AbstractCheck {
      */
     public static final String MSG_KEY = "array.trailing.comma";
 
+    /**
+     * Control whether to always check for a trailing comma, even when an array is inline.
+     */
+    private boolean alwaysDemandTrailingComma;
+
+    /**
+     * Setter to control whether to always check for a trailing comma, even when an array is inline.
+     *
+     * @param alwaysDemandTrailingComma whether to always check for a trailing comma.
+     */
+    public void setAlwaysDemandTrailingComma(boolean alwaysDemandTrailingComma) {
+        this.alwaysDemandTrailingComma = alwaysDemandTrailingComma;
+    }
+
     @Override
     public int[] getDefaultTokens() {
         return getRequiredTokens();
@@ -164,8 +226,9 @@ public void visitToken(DetailAST arrayInit) {
         final DetailAST previousSibling = rcurly.getPreviousSibling();
 
         if (arrayInit.getChildCount() != 1
-                && !TokenUtil.areOnSameLine(rcurly, previousSibling)
-                && !TokenUtil.areOnSameLine(arrayInit, previousSibling)
+                && (alwaysDemandTrailingComma
+                    || !TokenUtil.areOnSameLine(rcurly, previousSibling)
+                        && !TokenUtil.areOnSameLine(arrayInit, previousSibling))
                 && previousSibling.getType() != TokenTypes.COMMA) {
             log(previousSibling, MSG_KEY);
         }


Test patch:
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheckTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/ArrayTrailingCommaCheckTest.java
@@ -57,4 +57,25 @@ public void testTokensNotNull() {
         assertNotNull(check.getRequiredTokens(), "Invalid required tokens");
     }
 
+    @Test
+    public void testAlwaysDemandTrailingComma() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createModuleConfig(ArrayTrailingCommaCheck.class);
+        checkConfig.addAttribute("alwaysDemandTrailingComma", "true");
+        final String[] expected = {
+            "12:26: " + getCheckMessage(MSG_KEY),
+            "19:29: " + getCheckMessage(MSG_KEY),
+            "24:14: " + getCheckMessage(MSG_KEY),
+            "26:17: " + getCheckMessage(MSG_KEY),
+            "29:20: " + getCheckMessage(MSG_KEY),
+            "35:17: " + getCheckMessage(MSG_KEY),
+            "44:13: " + getCheckMessage(MSG_KEY),
+            "49:28: " + getCheckMessage(MSG_KEY),
+            "51:17: " + getCheckMessage(MSG_KEY),
+            "53:13: " + getCheckMessage(MSG_KEY),
+        };
+        verify(checkConfig,
+            getPath("InputArrayTrailingCommaAlwaysDemandTrailingComma.java"), expected);
+    }
+
 }
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/arraytrailingcomma/InputArrayTrailingCommaAlwaysDemandTrailingComma.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/arraytrailingcomma/InputArrayTrailingCommaAlwaysDemandTrailingComma.java
new file mode 100644
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/arraytrailingcomma/InputArrayTrailingCommaAlwaysDemandTrailingComma.java
@@ -0,0 +1,56 @@
+package com.puppycrawl.tools.checkstyle.checks.coding.arraytrailingcomma;
+
+/**
+ * Config:
+ * alwaysDemandTrailingComma = true
+ */
+public class InputArrayTrailingCommaAlwaysDemandTrailingComma {
+    public int[] test() {
+        if (true) {
+            return new int[]{0,};
+        }
+        return new int[]{0}; // violation
+    }
+
+    public int[] test2() {
+        if (true) {
+            return new int[]{0, 1,};
+        }
+        return new int[]{0, 1}; // violation
+    }
+
+    public void test3() {
+        int[] a = new int[]
+            {0}; // violation
+        int[] b = new int[]
+            {0, 1}; // violation
+        int[] c = new int[]
+            {0, 1,
+                2, 3
+            }; // violation
+        int[] d = new int[]
+            {
+                1,
+                2,
+                3
+            }; // violation
+        int[] e = new int[]
+            {
+                1,
+                5,
+                6,
+            };
+        int[] f = {1,
+            2
+        }; // violation
+        int[] g = {1,
+            2,
+        };
+        int[][] empty2d = {{}}; // violation
+        int[][] multiDimensionalArray = {
+            {1, 2}, // violation
+            {1,},
+            {3, 2, 1,}
+        }; // violation
+    }
+}