diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheck.java index e27ceb7b2..7d56b96bb 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheck.java @@ -11,127 +11,59 @@ public class EmptyPublicCtorInClassCheck extends Check { public static final String MSG_KEY = "empty.public.ctor"; - private HashMap mClassDefToClassCtorInfoMap = new HashMap<>(); - - @Override - public void beginTree(DetailAST aRoot) - { - mClassDefToClassCtorInfoMap.clear(); - } - @Override public int[] getDefaultTokens() { - return new int[] { TokenTypes.CTOR_DEF }; + return new int[] { TokenTypes.CLASS_DEF }; } @Override public void visitToken(DetailAST aCtorDefNode) { - DetailAST classDefNode = getClassDefNode(aCtorDefNode); - - addClassCtorInfoForClass(classDefNode, aCtorDefNode); - } - - @Override - public void finishTree(DetailAST aRoot) - { - for (ClassCtorInfo classCtorInfo : mClassDefToClassCtorInfoMap.values()) + if(getClassCtorCount(aCtorDefNode) == 1) { - if (classCtorInfo.getCtorCount() == 1) - { - DetailAST ctorDefNode = classCtorInfo.getLastCtorDefNode(); - - if (isCtorPublic(ctorDefNode) && isCtorEmpty(ctorDefNode)) - { - log(ctorDefNode.getLineNo(), MSG_KEY); - } - } + DetailAST aCtorDef = getClassCtorDefinition(aCtorDefNode); + + if(isCtorPublic(aCtorDef) && isCtorEmpty(aCtorDef)) + log(aCtorDef, MSG_KEY); } } - private boolean isCtorPublic(DetailAST aCtorDefNode) + private static DetailAST getClassCtorDefinition(DetailAST aClassDefNode) { - boolean result = false; - - DetailAST modifiers = aCtorDefNode.findFirstToken(TokenTypes.MODIFIERS); - - if (modifiers != null) - { - DetailAST publicLiteral = modifiers.findFirstToken(TokenTypes.LITERAL_PUBLIC); - result = publicLiteral != null; - } - - return result; + return aClassDefNode.findFirstToken(TokenTypes.OBJBLOCK).findFirstToken(TokenTypes.CTOR_DEF); } - - private boolean isCtorEmpty(DetailAST aCtorDefNode) + + private static boolean isCtorPublic(DetailAST aCtorDefNode) { - boolean result = false; - DetailAST ctorParametersNode = aCtorDefNode.findFirstToken(TokenTypes.PARAMETERS); - - if (ctorParametersNode.getChildCount() == 0) - { - DetailAST slistNode = aCtorDefNode.findFirstToken(TokenTypes.SLIST); - - if (slistNode.getChildCount() == 1 - && slistNode.getFirstChild().getType() == TokenTypes.RCURLY) - { - result = true; - } - } - - return result; + return aCtorDefNode + .findFirstToken(TokenTypes.MODIFIERS) + .findFirstToken(TokenTypes.LITERAL_PUBLIC) != null; } - - private static DetailAST getClassDefNode(DetailAST aCtorDefNode) + + private static boolean isCtorEmpty(DetailAST aCtorDefNode) { - DetailAST result = aCtorDefNode; - - while (result.getType() != TokenTypes.CTOR_DEF) - result = result.getParent(); - - return result; + return isCtorHasNoParameters(aCtorDefNode) && isCtorHasNoStatements(aCtorDefNode); } - - private void addClassCtorInfoForClass(DetailAST aClassDefNode, DetailAST aCtorDefNode) + + private static boolean isCtorHasNoParameters(DetailAST aCtorDefNode) { - ClassCtorInfo classCtorInfo = mClassDefToClassCtorInfoMap.get(aClassDefNode); - - if (classCtorInfo == null) - { - classCtorInfo = new ClassCtorInfo(); - classCtorInfo.putLastCtorDefNode(aCtorDefNode); - mClassDefToClassCtorInfoMap.put(aClassDefNode, classCtorInfo); - } - else - { - classCtorInfo.putLastCtorDefNode(aCtorDefNode); - } + return aCtorDefNode.findFirstToken(TokenTypes.PARAMETERS).getChildCount() == 0; } - - private static class ClassCtorInfo + + private static boolean isCtorHasNoStatements(DetailAST aCtorDefNode) { - private int mCtorCount = 0; - - private DetailAST mLastCtorDefNode; - - public void putLastCtorDefNode(DetailAST aCtorDefNode) - { - mLastCtorDefNode = aCtorDefNode; - - ++mCtorCount; - } - - public int getCtorCount() - { - return mCtorCount; - } - - public DetailAST getLastCtorDefNode() - { - return mLastCtorDefNode; - } + return aCtorDefNode.findFirstToken(TokenTypes.SLIST).getChildCount() == 1; } + /** + * Calculates constructor count for class. + * @param aClassDefNode + * a class definition node. + * @return ctor count for given class definition. + */ + private static int getClassCtorCount(DetailAST aClassDefNode) + { + return aClassDefNode.findFirstToken(TokenTypes.OBJBLOCK).getChildCount(TokenTypes.CTOR_DEF); + } } 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 dfadf09fa..b283662f9 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 @@ -26,7 +26,7 @@ default.comes.last=Default should be the last switch label. 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/protected constructor is useless +empty.public.ctor=This empty public constructor is useless 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/EmptyPublicCtorInClassCheckTest.java b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheckTest.java index 720cb0aa4..ccd57fd41 100644 --- a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheckTest.java +++ b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheckTest.java @@ -18,7 +18,7 @@ public void testEmptyPublicCtor() throws Exception { String expected[] = { - "5: " + mMessage, + "5:5: " + mMessage, }; verify(mCheckConfig, getPath("InputEmptyPublicCtorInClass1.java"), expected); @@ -37,9 +37,7 @@ public void testEmptyPrivateCtor() public void testEmptyProtectedCtor() throws Exception { - String expected[] = { - "5: " +mMessage - }; + String expected[] = {}; verify(mCheckConfig, getPath("InputEmptyPublicCtorInClass6.java"), expected); } @@ -67,8 +65,8 @@ public void testClassWithInnerClasses() throws Exception { String expected[] = { - "5: " +mMessage, - "14: " +mMessage, + "5:5: " +mMessage, + "14:9: " +mMessage, }; verify(mCheckConfig, getPath("InputEmptyPublicCtorInClass5.java"), expected);