Issue #464: Enum trailing comma check implemented

eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties

@@ -43,6 +43,9 @@ EmptyPublicCtorInClassCheck.desc = <p>This Check looks for useless empty public
 EmptyPublicCtorInClassCheck.classAnnotationNames = Regex which matches names of class annotations which require class to have public no-argument ctor. Default value is "javax\\.persistence\\.Entity".
 EmptyPublicCtorInClassCheck.ctorAnnotationNames = Regex which matches names of ctor annotations which make empty public ctor essential. Default value is "com\\.google\\.inject\\.Inject".
 
+EnumTrailingComma.name = Enum Trailing Comma
+EnumTrailingComma.desc = <p>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 enum.</p>
+
 FinalizeImplementationCheck.name = Finalize Implementation
 FinalizeImplementationCheck.desc = <p>This Check detects 3 most common cases of incorrect finalize() method implementation:</p><ul><li>negates effect of superclass finalize<br/><code>protected void finalize() { } <br/> protected void finalize() { doSomething(); }</code></li><li>useless (or worse) finalize<br/><code>protected void finalize() { super.finalize(); }</code></li><li>public finalize<br/><code>public void finalize() { try { doSomething(); } finally { super.finalize() } }</code></li></ul>
 

eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml

@@ -162,6 +162,13 @@
       <message-key key="empty.public.ctor"/>
     </rule-metadata>
 
+    <rule-metadata name="%EnumTrailingComma.name" internal-name="EnumTrailingComma" parent="TreeWalker">
+      <alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.EnumTrailingComma"/>
+      <description>%EnumTrailingComma.desc</description>
+
+      <message-key key="enum.trailing.comma"/>
+    </rule-metadata>
+
     <rule-metadata name="%ForbidCertainImportsCheck.name" internal-name="ForbidCertainImportsCheck" parent="TreeWalker">
       <alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.ForbidCertainImportsCheck"/>
       <description>%ForbidCertainImportsCheck.desc</description>

sevntu-checks/sevntu-checks.xml

@@ -152,6 +152,7 @@
       <property name="ignoreFieldNamePattern" value="serialVersionUID"/>
     </module>
     <module name="com.github.sevntu.checkstyle.checks.coding.EmptyPublicCtorInClassCheck"/>
+    <module name="com.github.sevntu.checkstyle.checks.coding.EnumTrailingComma"/>
     <module name="com.github.sevntu.checkstyle.checks.coding.DiamondOperatorForVariableDefinitionCheck"/>
     <module name="com.github.sevntu.checkstyle.checks.coding.NameConventionForJunit4TestClassesCheck"/>
     <module name="com.github.sevntu.checkstyle.checks.coding.UselessSuperCtorCallCheck"/>

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

@@ -0,0 +1,147 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2020 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;
+
+/**
+ * <p>
+ * Checks that enums contains a trailing comma.
+ * </p>
+ * <pre>
+ * enum MovieType {
+ *  GOOD,
+ *  BAD,
+ *  UGLY,
+ *  ;
+ * }
+ * </pre>
+ * <p>
+ * 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 enum.
+ * </p>
+ * <pre>
+ * enum MovieType {GOOD, BAD, UGLY;}
+ * enum MovieType {GOOD, BAD, UGLY
+ *  ;}
+ * enum MovieType {GOOD, BAD,
+ *  UGLY;}
+ * enum MovieType {GOOD, BAD,
+ *  UGLY; // Violation
+ * }
+ * </pre>
+ * <p>
+ * Putting this comma in makes it easier to change the order of the elements or add new elements
+ * at the end. Main benefit of a trailing comma is that when you add new entry to an enum, no
+ * surrounding lines are changed.
+ * </p>
+ * <pre>
+ * enum MovieType {
+ *   GOOD,
+ *   BAD, //OK
+ * }
+ *
+ * enum MovieType {
+ *   GOOD,
+ *   BAD,
+ *   UGLY,  // Just this line added, no other changes
+ * }
+ * </pre>
+ * <p>
+ * If closing brace is on the same line as trailing comma, this benefit is gone (as the check does
+ * not demand a certain location of curly braces the following two cases will not produce a
+ * violation):
+ * </p>
+ * <pre>
+ * enum MovieType {GOOD,
+ *  BAD,} // Trailing comma not needed, line needs to be modified anyway
+ *
+ * enum MovieType {GOOD,
+ *  BAD, // Modified line
+ *  UGLY,} // Added line
+ * </pre>
+ *
+ * @author <a href="mailto:yasser.aziza@gmail.com"> Yasser Aziza </a>
+ */
+public class EnumTrailingComma extends AbstractCheck {
+
+    /**
+     * Warning message key pointing to the warning message text in "messages.properties" file.
+     */
+    public static final String MSG_KEY = "enum.trailing.comma";
+
+    @Override
+    public int[] getDefaultTokens() {
+        return new int[] {
+            TokenTypes.ENUM_CONSTANT_DEF,
+        };
+    }
+
+    @Override
+    public int[] getAcceptableTokens() {
+        return getDefaultTokens();
+    }
+
+    @Override
+    public int[] getRequiredTokens() {
+        return getDefaultTokens();
+    }
+
+    @Override
+    public void visitToken(DetailAST ast) {
+        final DetailAST nextSibling = ast.getNextSibling();
+
+        if (TokenTypes.COMMA != nextSibling.getType() && isSeparateLine(ast)) {
+            log(ast, MSG_KEY);
+        }
+    }
+
+    /**
+     * Checks if the given {@link TokenTypes#ENUM_CONSTANT_DEF} element is in the same line with
+     * the {@link TokenTypes#LCURLY} or {@link TokenTypes#RCURLY}.
+     *
+     * @param ast the enum node
+     * @return {@code true} if the element is on the same line with {@link TokenTypes#LCURLY} or
+     * {@link TokenTypes#RCURLY}, {@code false} otherwise.
+     */
+    private static boolean isSeparateLine(DetailAST ast) {
+        final DetailAST objBlock = ast.getParent();
+        final DetailAST leftCurly = objBlock.findFirstToken(TokenTypes.LCURLY);
+        final DetailAST rightCurly = objBlock.findFirstToken(TokenTypes.RCURLY);
+
+        return !areOnSameLine(leftCurly, ast)
+                && !areOnSameLine(ast, rightCurly);
+    }
+
+    /**
+     * Determines if two ASTs are on the same line.
+     *
+     * @param ast1   the first AST
+     * @param ast2   the second AST
+     *
+     * @return true if they are on the same line.
+     */
+    private static boolean areOnSameLine(DetailAST ast1, DetailAST ast2) {
+        return ast1.getLineNo() == ast2.getLineNo();
+    }
+
+}

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

@@ -293,7 +293,8 @@ private enum NullnessAnnotation {
         PARAMETERS_ARE_NULLABLE_BY_DEFAULT("ParametersAreNullableByDefault", PKG_JAVAX_ANNOTATION),
         /** ReturnValuesAreNonnullByDefault. */
         RETURN_VALUES_ARE_NONNULL_BY_DEFAULT("ReturnValuesAreNonnullByDefault",
-                "edu.umd.cs.findbugs.annotations");
+                "edu.umd.cs.findbugs.annotations"),
+        ;
 
         /** The annotation's name. */
         private final String annotationName;

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

@@ -167,7 +167,7 @@ protected enum NumericType {
         /**
          * Denotes a binary literal. For example, 0b0011
          */
-        BINARY
+        BINARY,
 
     }
 

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

@@ -14,6 +14,7 @@ custom.declaration.order.method=Method definition in wrong order. Expected ''{0}
 diamond.operator.for.variable.definition=Diamond operator expected.
 either.log.or.throw=Either log or throw exception.
 empty.public.ctor=This empty public constructor is useless.
+enum.trailing.comma=Enum should contain trailing comma.
 finalize.implementation.missed.super.finalize=You have to call super.finalize() right after finally opening brace.
 finalize.implementation.missed.try.finally=finalize() method should contain try-finally block with super.finalize() call inside finally block.
 finalize.implementation.public=finalize() method should have a "protected" visibility.

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

@@ -0,0 +1,58 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2020 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 static com.github.sevntu.checkstyle.checks.coding.EnumTrailingComma.MSG_KEY;
+
+import org.junit.Test;
+
+import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
+import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+
+/**
+ * @author <a href="mailto:yasser.aziza@gmail.com"> Yasser Aziza </a>
+ */
+public class EnumTrailingCommaTest extends AbstractModuleTestSupport {
+
+    /**
+     * An error message for current check.
+     */
+    private final String warningMessage = getCheckMessage(MSG_KEY);
+
+    @Override
+    protected String getPackageLocation() {
+        return "com/github/sevntu/checkstyle/checks/coding";
+    }
+
+    @Test
+    public void testDefault() throws Exception {
+        final DefaultConfiguration checkConfig = createModuleConfig(EnumTrailingComma.class);
+
+        final String[] expected = {
+            "14:9: " + warningMessage,
+            "19:9: " + warningMessage,
+            "27:9: " + warningMessage,
+            "39:9: " + warningMessage,
+        };
+
+        verify(checkConfig, getPath("InputEnumTrailingCommaCheck.java"), expected);
+    }
+
+}

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

@@ -0,0 +1,61 @@
+package com.github.sevntu.checkstyle.checks.coding;
+
+public class InputEnumTrailingCommaCheck {
+
+    enum MovieType {
+        GOOD,
+        BAD,
+        UGLY,
+        ;
+    }
+
+    enum Gender {
+        FEMALE,
+        MALE // Violation
+    }
+
+    enum Language {
+        ENGLISH,
+        GERMAN // Violation
+        ;
+    }
+
+    // Violation
+    enum ErrorMessage {
+        USER_DOES_NOT_EXIST,
+        INVALID_USER_PASS,
+        EVERYTHING_ELSE; // Violation
+    }
+
+    enum BinType {ZERO, ONE
+    ;}
+
+    enum SameLine {FIRST, SECOND, THIRD;}
+
+    enum LineBreak {FIRST, SECOND,
+        THIRD;}
+
+    enum LastLineBreak {FIRST, SECOND,
+        THIRD; // Violation
+    }
+
+    enum LastBreakSemicolon {FIRST, SECOND, THIRD
+        ;
+    }
+
+    enum RgbColor {
+        RED,
+        GREEN,
+        BLUE
+        ,
+    }
+
+    enum OperatingSystem {
+        LINUX,
+        MAC
+        ,}
+
+    enum EMPTY {E
+    }
+
+}

sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml

@@ -346,6 +346,14 @@
     </param>
   </rule>
 
+  <rule>
+    <key>com.github.sevntu.checkstyle.checks.coding.EnumTrailingComma</key>
+    <name>Enum Trailing Comma</name>
+    <tag>coding</tag>
+    <description>This check ensures that enums contains a trailing comma.</description>
+    <configKey>Checker/TreeWalker/com.github.sevntu.checkstyle.checks.coding.EnumTrailingComma</configKey>
+  </rule>
+
   <rule>
     <key>com.github.sevntu.checkstyle.checks.coding.FinalizeImplementationCheck</key>
     <name>Finalize Implementation Check</name>

sevntu-checkstyle-sonar-plugin/src/test/java/com/github/sevntu/checkstyle/sonar/CheckstyleExtensionRulesDefinitionTest.java

@@ -40,7 +40,7 @@ public void testAllRulesValid() {
         Assert.assertEquals("Incorrect repository language", "java", repository.language());
 
         final List<RulesDefinition.Rule> rules = repository.rules();
-        Assert.assertEquals("Incorrect number of loaded rules", 59, rules.size());
+        Assert.assertEquals("Incorrect number of loaded rules", 60, rules.size());
 
         for (RulesDefinition.Rule rule : rules) {
             Assert.assertNotNull("Rule key is not set", rule.key());