From 2f7481ee4e20ae785298c31ec2f979752dd7eb03 Mon Sep 17 00:00:00 2001 From: liscju Date: Tue, 21 Jul 2015 23:40:34 +0200 Subject: [PATCH] Make RedundantModifier checks if enum constructor has redundant private modifier, fixes part of #1242 --- .../tools/checkstyle/api/JavadocTagInfo.java | 2 +- .../checks/LineSeparatorOption.java | 2 +- .../modifier/RedundantModifierCheck.java | 63 +++++++++++++++---- .../modifier/RedundantModifierTest.java | 11 ++++ .../InputRedundantConstructorModifier.java | 19 ++++++ 5 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/JavadocTagInfo.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/JavadocTagInfo.java index f19c1ef63..249d9ffc5 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/JavadocTagInfo.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/api/JavadocTagInfo.java @@ -432,7 +432,7 @@ public enum JavadocTagInfo { * @param name the tag name * @param type the type of tag */ - private JavadocTagInfo(final String text, final String name, + JavadocTagInfo(final String text, final String name, final Type type) { this.text = text; this.name = name; diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/LineSeparatorOption.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/LineSeparatorOption.java index f6629e1a9..4dd54232f 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/LineSeparatorOption.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/LineSeparatorOption.java @@ -51,7 +51,7 @@ public enum LineSeparatorOption { * Creates a new LineSeparatorOption instance. * @param sep the line separator, e.g. "\r\n" */ - private LineSeparatorOption(String sep) { + LineSeparatorOption(String sep) { lineSeparator = sep.getBytes(StandardCharsets.US_ASCII); } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java index c13759aad..0d84a0edf 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java @@ -28,9 +28,11 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; /** * Checks for redundant modifiers in interface and annotation definitions. - * Also checks for redundant final modifiers on methods of final classes. + * Also checks for redundant final modifiers on methods of final classes + * and redundant enum constructor modifier. * * @author lkuehne + * @author liscju */ public class RedundantModifierCheck extends Check { @@ -56,6 +58,7 @@ public class RedundantModifierCheck TokenTypes.VARIABLE_DEF, TokenTypes.ANNOTATION_FIELD_DEF, TokenTypes.INTERFACE_DEF, + TokenTypes.CTOR_DEF, }; } @@ -71,23 +74,18 @@ public class RedundantModifierCheck TokenTypes.VARIABLE_DEF, TokenTypes.ANNOTATION_FIELD_DEF, TokenTypes.INTERFACE_DEF, + TokenTypes.CTOR_DEF, }; } @Override public void visitToken(DetailAST ast) { if (TokenTypes.INTERFACE_DEF == ast.getType()) { - final DetailAST modifiers = - ast.findFirstToken(TokenTypes.MODIFIERS); - - for (final int tokenType : TOKENS_FOR_INTERFACE_MODIFIERS) { - final DetailAST modifier = - modifiers.findFirstToken(tokenType); - if (modifier != null) { - log(modifier.getLineNo(), modifier.getColumnNo(), - MSG_KEY, modifier.getText()); - } - } + checkInterfaceModifiers(ast); + } + else if (TokenTypes.CTOR_DEF == ast.getType() + && isEnumMember(ast)) { + checkEnumConstructorModifiers(ast); } else if (isInterfaceOrAnnotationMember(ast)) { processInterfaceOrAnnotation(ast); @@ -97,6 +95,37 @@ public class RedundantModifierCheck } } + /** + * Checks if interface has proper modifiers + * @param ast interface to check + */ + private void checkInterfaceModifiers(DetailAST ast) { + final DetailAST modifiers = + ast.findFirstToken(TokenTypes.MODIFIERS); + + for (final int tokenType : TOKENS_FOR_INTERFACE_MODIFIERS) { + final DetailAST modifier = + modifiers.findFirstToken(tokenType); + if (modifier != null) { + log(modifier.getLineNo(), modifier.getColumnNo(), + MSG_KEY, modifier.getText()); + } + } + } + + /** + * Check if enum constructor has proper modifiers + * @param ast constructor of enum + */ + private void checkEnumConstructorModifiers(DetailAST ast) { + final DetailAST modifiers = ast.findFirstToken(TokenTypes.MODIFIERS); + final DetailAST modifier = modifiers.getFirstChild(); + if (modifier != null) { + log(modifier.getLineNo(), modifier.getColumnNo(), + MSG_KEY, modifier.getText()); + } + } + /** * do validation of interface of annotation * @param ast token AST @@ -161,6 +190,16 @@ public class RedundantModifierCheck } } + /** + * Checks if current AST node is member of Enum + * @param ast AST node + * @return true if it is an enum member + */ + private boolean isEnumMember(DetailAST ast) { + final DetailAST parentTypeDef = ast.getParent().getParent(); + return parentTypeDef.getType() == TokenTypes.ENUM_DEF; + } + /** * Checks if current AST node is member of Interface or Annotation, not of their subnodes. * @param ast AST node diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java index ad69ed811..e34c01de9 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java @@ -82,6 +82,16 @@ public class RedundantModifierTest expected); } + @Test + public void testEnumConstructorIsImplicitlyPrivate() throws Exception { + final DefaultConfiguration checkConfig = + createCheckConfig(RedundantModifierCheck.class); + final String[] expected = { + "10:5: " + getCheckMessage(MSG_KEY, "private"), + }; + verify(checkConfig, getPath("InputRedundantConstructorModifier.java"), expected); + } + @Test public void testGetAcceptableTokens() { RedundantModifierCheck redundantModifierCheckObj = new RedundantModifierCheck(); @@ -91,6 +101,7 @@ public class RedundantModifierTest TokenTypes.VARIABLE_DEF, TokenTypes.ANNOTATION_FIELD_DEF, TokenTypes.INTERFACE_DEF, + TokenTypes.CTOR_DEF, }; Assert.assertNotNull(actual); Assert.assertArrayEquals(expected, actual); diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java new file mode 100644 index 000000000..29b16d3cf --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java @@ -0,0 +1,19 @@ +//////////////////////////////////////////////////////////////////////////////// +// Test case file for checkstyle. +// Created: 2015 +//////////////////////////////////////////////////////////////////////////////// +package com.puppycrawl.tools.checkstyle; + +public enum InputRedundantConstructorModifier { + VAL1, VAL2; + + private InputRedundantConstructorModifier() { } + + InputRedundantConstructorModifier(int i) { } + + InputRedundantConstructorModifier(char c) { } +} + +class ProperPrivateConstructor { + private ProperPrivateConstructor() { } +}