Issue #464: Check for trailing commas on enums

This Check assures that the last enum constant definition in a multi-line enum ends in a comma:

    enum E1 {
        ONE,
        TWO,
        THREE,
    }

Original PR: #465

2016-07-29 -- 2016-08-26

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheck.java

@@ -0,0 +1,153 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2016 the original author or authors.
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+////////////////////////////////////////////////////////////////////////////////
+
+package com.github.sevntu.checkstyle.checks.coding;
+
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+
+/**
+ * Checks if enum constant contains an optional trailing comma.
+ *
+ * <p>Rationale: Putting this comma in makes is easier to change the order of the elements or add
+ * new elements at the end of the list.
+ *
+ * <p>The following is a normal enum type declaration:
+ * <pre>
+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA
+ * }
+ * </pre>
+ *
+ * <p>However, if you want to append something to the list, you would need to change the line
+ * containing the last enum constant:
+ *
+ * <pre>
+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,   // changed due to the ','
+ *     DELTA    // new line
+ * }
+ * </pre>
+ *
+ * <p>This check makes sure that also the last enum constant has a trailing comma, which is
+ * valid according to the Java Spec (see <a
+ * href="http://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.9.1">Enum Constants</a>)
+ *
+ * <pre>
+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,
+ *     DELTA, // removing this comma will result in a violation with the check activated
+ * }
+ * </pre>
+ *
+ * <p>However, you could also add a semicolon behind that comma on the same line, which would raise
+ * a violation
+ *
+ * <pre>
+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,
+ *     DELTA,; // violation
+ * }
+ * </pre>
+ * In this case the semicolon should be removed. However, if there is more in the enum body, the
+ * semicolon should be placed on a line by itself.
+ *
+ * <p>An example of how to configure the check is:
+ * <pre>
+ * &lt;module name="EnumTrailingComma"/&gt;
+ * </pre>
+ *
+ * @author <a href="[email protected]">Kariem Hussein</a>
+ */
+public class EnumTrailingCommaAndSemicolonCheck extends AbstractCheck {
+
+    /** Key for warning message text in "messages.properties" file. */
+    public static final String MSG_KEY = "enum.trailing.comma";
+    /** Key for warning message text in "messages.properties" file. */
+    public static final String MSG_KEY_SEMI = "enum.trailing.comma.semi";
+
+    @Override
+    public int[] getDefaultTokens() {
+        return getAcceptableTokens();
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return new int[]{TokenTypes.ENUM_DEF};
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return getAcceptableTokens();
+    }
+
+    @Override
+    public void visitToken(DetailAST enumDef) {
+        final DetailAST enumConstBlock = enumDef.findFirstToken(TokenTypes.OBJBLOCK);
+
+        final DetailAST enumConstLeft = enumConstBlock.findFirstToken(TokenTypes.LCURLY);
+        final DetailAST enumConstRight = enumConstBlock.findFirstToken(TokenTypes.RCURLY);
+
+        // Only check, if block is multi-line and there are more than one enum constants
+        if (enumConstLeft.getLineNo() != enumConstRight.getLineNo()
+                && enumConstBlock.getChildCount(TokenTypes.ENUM_CONSTANT_DEF) > 1) {
+            final DetailAST constant = enumConstBlock.findFirstToken(TokenTypes.ENUM_CONSTANT_DEF);
+            final DetailAST lastComma = getLastComma(constant);
+
+            final DetailAST next = lastComma.getNextSibling();
+            final int type = next.getType();
+
+            if (type == TokenTypes.SEMI) {
+                if (next.getLineNo() == lastComma.getLineNo()) {
+                    // semi on the same line as last comma
+                    log(next.getLineNo(), MSG_KEY_SEMI);
+                }
+
+            }
+            else if (type == TokenTypes.ENUM_CONSTANT_DEF) {
+                log(next.getLineNo(), MSG_KEY);
+            }
+        }
+    }
+
+    /**
+     * Get the last comma in a series of siblings.
+     *
+     * @param start the first sibling
+     * @return the AST containing the last comma
+     */
+    private DetailAST getLastComma(DetailAST start) {
+        DetailAST comma = null;
+        for (DetailAST ast = start; ast != null; ast = ast.getNextSibling()) {
+            if (ast.getType() == TokenTypes.COMMA) {
+                comma = ast;
+            }
+        }
+        return comma;
+    }
+}

sevntu-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties

@@ -27,6 +27,8 @@ diamond.operator.for.variable.definition = Diamond operator expected.
 doublechecked.locking.avoid=The double-checked locking idiom is broken and should be avoided.
 empty.statement=Empty statement.
 empty.public.ctor=This empty public constructor is useless.
+enum.trailing.comma=Enum constant should contain trailing comma.
+enum.trailing.comma.semi=Enum constant should contain trailing comma without semi-colon afterwards.
 equals.avoid.null=String literal expressions should be on the left side of an equals comparison.
 equals.noHashCode=Definition of ''equals()'' without corresponding definition of ''hashCode()''.
 either.log.or.throw=Either log or throw exception.

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheckTest.java

@@ -0,0 +1,42 @@
+package com.github.sevntu.checkstyle.checks.coding;
+
+import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
+import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+
+import static com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck.MSG_KEY;
+import static com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck.MSG_KEY_SEMI;
+
+public class EnumTrailingCommaAndSemicolonCheckTest
+    extends BaseCheckTestSupport {
+    @Override
+    protected String getPath(String filename) throws IOException {
+        URL r = getClass().getResource(filename);
+        return new File(r.getPath()).getCanonicalPath();
+    }
+
+    @Test
+    public void testDefault() throws Exception {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(EnumTrailingCommaAndSemicolonCheck.class);
+        final String[] expected = {
+            "14: " + getCheckMessage(MSG_KEY),
+            "20: " + getCheckMessage(MSG_KEY),
+            "26: " + getCheckMessage(MSG_KEY_SEMI),
+        };
+        verify(checkConfig, getPath("InputEnumTrailingComma.java"), expected);
+    }
+
+    @Test
+    public void testTokensNotNull() {
+        final EnumTrailingCommaAndSemicolonCheck check = new EnumTrailingCommaAndSemicolonCheck();
+        Assert.assertNotNull(check.getAcceptableTokens());
+        Assert.assertNotNull(check.getDefaultTokens());
+        Assert.assertNotNull(check.getRequiredTokens());
+    }
+}

sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputEnumTrailingComma.java

@@ -0,0 +1,61 @@
+package com.github.sevntu.checkstyle.checks.coding;
+
+public interface InputEnumTrailingComma
+{
+    enum E1 {
+        ONE,
+        TWO,
+        THREE,
+    }
+
+    enum E2 {
+        ONE,
+        TWO,
+        THREE   // violation
+    }
+
+    enum E3 {
+        ONE,
+        TWO,
+        THREE;  // violation
+    }
+
+    enum E4 {
+        ONE,
+        TWO,
+        THREE,; // violation
+    }
+
+    enum E5 {
+        ONE,
+        TWO,
+        THREE,
+        ;
+    }
+
+    // enums below are ignored by the check, but were added for completenes
+    // Please don't remove, they are necessary for full cobertura branch coverage
+
+    // empty
+    enum E6 {}
+
+    // single enum const, single-line block
+    enum E7_1 { ONE }
+    enum E7_2 { ONE; }
+    enum E7_3 { ONE, }
+    enum E7_4 { ONE,; }
+
+    // single enum const, multi-line block
+    enum E8_1 {
+        ONE
+    }
+    enum E8_2 {
+        ONE;
+    }
+    enum E8_3 {
+        ONE,
+    }
+    enum E8_4 {
+        ONE,;
+    }
+}