From 59432bbfa806c70d52f81aecca33f1ca5be1ce20 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Fri, 29 Jul 2016 15:10:56 +0200 Subject: [PATCH] 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 --- .../EnumTrailingCommaAndSemicolonCheck.java | 153 ++++++++++++++++++ .../checks/coding/messages.properties | 2 + ...numTrailingCommaAndSemicolonCheckTest.java | 42 +++++ .../checks/coding/InputEnumTrailingComma.java | 61 +++++++ 4 files changed, 258 insertions(+) create mode 100644 sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheck.java create mode 100644 sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheckTest.java create mode 100644 sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputEnumTrailingComma.java diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheck.java new file mode 100644 index 0000000000..0674e9df65 --- /dev/null +++ b/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. + * + *

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. + * + *

The following is a normal enum type declaration: + * 

+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA
+ * }
+ *
+ * + *

However, if you want to append something to the list, you would need to change the line + * containing the last enum constant: + * + * 

+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,   // changed due to the ','
+ *     DELTA    // new line
+ * }
+ *
+ * + *

This check makes sure that also the last enum constant has a trailing comma, which is + * valid according to the Java Spec (see Enum Constants) + * + * 

+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,
+ *     DELTA, // removing this comma will result in a violation with the check activated
+ * }
+ *
+ * + *

However, you could also add a semicolon behind that comma on the same line, which would raise + * a violation + * + * 

+ * enum Type {
+ *     ALPHA,
+ *     BETA,
+ *     GAMMA,
+ *     DELTA,; // violation
+ * }
+ *
+ * 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. + * + *

An example of how to configure the check is: + * 

+ * <module name="EnumTrailingComma"/>
+ *
+ * + * @author Kariem Hussein + */ +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; + } +} diff --git a/sevntu-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties b/sevntu-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties index dde333fbcd..1db1574900 100644 --- a/sevntu-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties +++ b/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. diff --git a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheckTest.java b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaAndSemicolonCheckTest.java new file mode 100644 index 0000000000..db036e1a6f --- /dev/null +++ b/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()); + } +} diff --git a/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputEnumTrailingComma.java b/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputEnumTrailingComma.java new file mode 100644 index 0000000000..8babed1f7d --- /dev/null +++ b/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,; + } +}