From b80a97f92d3d181e1ac8052e86411cd342816397 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Fri, 29 Jul 2016 15:10:56 +0200 Subject: [PATCH 1/6] Issue #464: Check for trailing commas on enums. Original PR: #465 --- .../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,; + } +} From 5d138147685d67f5ed8106637b29048ed639aff9 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Sun, 4 Dec 2016 14:09:42 +0100 Subject: [PATCH 2/6] minor: Update to new checkstyle rules introduced in 7eac4b8a329a9bbd3dcc18ae0895d427f966716c --- ...numTrailingCommaAndSemicolonCheckTest.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) 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 index db036e1a6f..690dd42d2e 100644 --- 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 @@ -1,23 +1,43 @@ +//////////////////////////////////////////////////////////////////////////////// +// 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.BaseCheckTestSupport; -import com.puppycrawl.tools.checkstyle.DefaultConfiguration; -import org.junit.Assert; -import org.junit.Test; +import static com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck.MSG_KEY; +import static com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck.MSG_KEY_SEMI; 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; +import org.junit.Assert; +import org.junit.Test; + +import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport; +import com.puppycrawl.tools.checkstyle.DefaultConfiguration; public class EnumTrailingCommaAndSemicolonCheckTest extends BaseCheckTestSupport { @Override protected String getPath(String filename) throws IOException { - URL r = getClass().getResource(filename); - return new File(r.getPath()).getCanonicalPath(); + final URL resource = getClass().getResource(filename); + return new File(resource.getPath()).getCanonicalPath(); } @Test From da09a5cf6fae79017680c6ef7245621250d2aee5 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Sun, 4 Dec 2016 14:35:36 +0100 Subject: [PATCH 3/6] minor: add check to sevntu config --- sevntu-checks/sevntu-checks.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/sevntu-checks/sevntu-checks.xml b/sevntu-checks/sevntu-checks.xml index 6285c6af0c..c0c99166db 100644 --- a/sevntu-checks/sevntu-checks.xml +++ b/sevntu-checks/sevntu-checks.xml @@ -155,6 +155,7 @@ + From 4ea9fa4b097b10b61f6f46241a2f97fb6f688557 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Sun, 4 Dec 2016 14:36:01 +0100 Subject: [PATCH 4/6] minor: add check to sonar extension config --- .../sevntu/checkstyle/sonar/checkstyle-extensions.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml b/sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml index 198c64c526..d28225b142 100644 --- a/sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml +++ b/sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml @@ -889,6 +889,14 @@ Checker/TreeWalker/com.github.sevntu.checkstyle.checks.coding.WhitespaceBeforeArrayInitializerCheck + + com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck + Enum Trailing Comma + + This checks enforces a trailing comma at the end of a line in an enum constant definition. + Checker/TreeWalker/com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck + + com.github.sevntu.checkstyle.checks.design.InnerClassCheck Inner Class From 99f77483c519479988ea1bd15e5684d59e9b18e6 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Sun, 4 Dec 2016 14:52:56 +0100 Subject: [PATCH 5/6] minor: add check to eclipse-cs config --- .../checks/coding/checkstyle-metadata.properties | 3 +++ .../checkstyle/checks/coding/checkstyle-metadata.xml | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties b/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties index 5000acc0a6..00b4db3fb4 100755 --- a/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties +++ b/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties @@ -177,3 +177,6 @@ EitherLogOrThrowCheck.loggingMethodNames = Logging method names separated with c WhitespaceBeforeArrayInitializerCheck.name = Whitespace Before Array Initializer WhitespaceBeforeArrayInitializerCheck.desc = This checks enforces whitespace before array initializer. + +EnumTrailingCommaAndSemicolonCheck.name = Enum Trailing Comma +EnumTrailingCommaAndSemicolonCheck.desc = This checks enforces a trailing comma at the end of a line in an enum constant definition. diff --git a/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml b/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml index 9e53dda6bc..3455bd9e37 100644 --- a/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml +++ b/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml @@ -528,5 +528,13 @@ + + + %EnumTrailingCommaAndSemicolonCheck.desc + + + + + From cdc74e816dfc543d0ebc083e03524b12eb6e08b6 Mon Sep 17 00:00:00 2001 From: Kariem Hussein Date: Sun, 4 Dec 2016 15:09:47 +0100 Subject: [PATCH 6/6] minor: adhere to sevntu checks --- .../checks/coding/EnumTrailingCommaAndSemicolonCheck.java | 2 +- .../checks/coding/NumericLiteralNeedsUnderscoreCheck.java | 2 +- .../sevntu/checkstyle/internal/CommitValidationTest.java | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) 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 index 0674e9df65..eeefd2ebd5 100644 --- 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 @@ -98,7 +98,7 @@ public int[] getDefaultTokens() { @Override public int[] getAcceptableTokens() { - return new int[]{TokenTypes.ENUM_DEF}; + return new int[] {TokenTypes.ENUM_DEF}; } @Override diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NumericLiteralNeedsUnderscoreCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NumericLiteralNeedsUnderscoreCheck.java index 9922d4bb4a..5023a8d08c 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NumericLiteralNeedsUnderscoreCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NumericLiteralNeedsUnderscoreCheck.java @@ -164,7 +164,7 @@ protected enum NumericType { /** * Denotes a binary literal. For example, 0b0011 */ - BINARY; + BINARY, } /** diff --git a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CommitValidationTest.java b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CommitValidationTest.java index 4ede43ccf8..2b5a859a59 100644 --- a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CommitValidationTest.java +++ b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CommitValidationTest.java @@ -290,7 +290,8 @@ private static String getInvalidCommitMessageFormattingError(String commitId, } private enum CommitsResolutionMode { - BY_COUNTER, BY_LAST_COMMIT_AUTHOR + BY_COUNTER, + BY_LAST_COMMIT_AUTHOR, } private static class RevCommitsPair {