New Check EmptyPublicCtorInClassCheck implemented. fixes sevntu-check…

…style#186
alex-zuy · Jan 2, 2015 · 582dc3c · 582dc3c
1 parent d3ba69f
commit 582dc3c
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 107 deletions.
diff --git a/...src/main/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheck.java b/...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<DetailAST, ClassCtorInfo> 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/...-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties b/...-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/...test/java/com/github/sevntu/checkstyle/checks/coding/EmptyPublicCtorInClassCheckTest.java b/...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);