From b142c593512c9ef1179f4254ec406c23d180ed2f Mon Sep 17 00:00:00 2001 From: Oliver Burn Date: Thu, 7 Oct 2010 12:19:35 +1100 Subject: [PATCH 1/2] Fix HideUtilityClassConstructor to handle empty and inner classes. Thanks to Roman Ivanov for patch #3045720. --- .../HideUtilityClassConstructorCheck.java | 37 ++++++++++++-- .../HideUtilityClassContructor3041574_1.java | 8 ++++ .../HideUtilityClassContructor3041574_2.java | 8 ++++ .../HideUtilityClassContructor3041574_3.java | 48 +++++++++++++++++++ .../HideUtilityClassConstructorCheckTest.java | 30 ++++++++++++ src/xdocs/config_design.xml | 3 +- src/xdocs/releasenotes.xml | 6 +++ 7 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_1.java create mode 100644 src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_2.java create mode 100644 src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_3.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheck.java index a008a7004..783c1f4a5 100755 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheck.java @@ -31,7 +31,6 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST; *

* * @author lkuehne - * @version $Revision: 1.12 $ */ public class HideUtilityClassConstructorCheck extends Check { @@ -44,10 +43,17 @@ public class HideUtilityClassConstructorCheck extends Check @Override public void visitToken(DetailAST aAST) { + if (isAbstract(aAST)) { + // abstract class could not have private constructor + return; + } + final DetailAST objBlock = aAST.findFirstToken(TokenTypes.OBJBLOCK); DetailAST child = objBlock.getFirstChild(); + final boolean hasStaticModifier = isStatic(aAST); boolean hasMethodOrField = false; boolean hasNonStaticMethodOrField = false; + boolean hasNonPrivateStaticMethodOrField = false; boolean hasDefaultCtor = true; boolean hasPublicCtor = false; @@ -67,6 +73,9 @@ public class HideUtilityClassConstructorCheck extends Check if (!isStatic && !isPrivate) { hasNonStaticMethodOrField = true; } + if (isStatic && !isPrivate) { + hasNonPrivateStaticMethodOrField = true; + } } if (type == TokenTypes.CTOR_DEF) { hasDefaultCtor = false; @@ -93,11 +102,31 @@ public class HideUtilityClassConstructorCheck extends Check final boolean extendsJLO = // J.Lo even made it into in our sources :-) aAST.findFirstToken(TokenTypes.EXTENDS_CLAUSE) == null; - final boolean isUtilClass = - extendsJLO && hasMethodOrField && !hasNonStaticMethodOrField; + final boolean isUtilClass = extendsJLO && hasMethodOrField + && !hasNonStaticMethodOrField && hasNonPrivateStaticMethodOrField; - if (isUtilClass && hasAccessibleCtor) { + if (isUtilClass && (hasAccessibleCtor && !hasStaticModifier)) { log(aAST.getLineNo(), aAST.getColumnNo(), "hide.utility.class"); } } + + /** + * @param aAST class definition for check. + * @return true if a given class declared as abstract. + */ + private boolean isAbstract(DetailAST aAST) + { + return aAST.findFirstToken(TokenTypes.MODIFIERS) + .branchContains(TokenTypes.ABSTRACT); + } + + /** + * @param aAST class definition for check. + * @return true if a given class declared as static. + */ + private boolean isStatic(DetailAST aAST) + { + return aAST.findFirstToken(TokenTypes.MODIFIERS) + .branchContains(TokenTypes.LITERAL_STATIC); + } } diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_1.java b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_1.java new file mode 100644 index 000000000..dc80fb3ff --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_1.java @@ -0,0 +1,8 @@ +package com.puppycrawl.tools.checkstyle.design; + +import java.io.Serializable; + +public abstract class HideUtilityClassContructor3041574_1 implements Serializable { + private static final long serialVersionUID = 1L; + +} \ No newline at end of file diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_2.java b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_2.java new file mode 100644 index 000000000..af76f0d87 --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_2.java @@ -0,0 +1,8 @@ +package com.puppycrawl.tools.checkstyle.design; + +import java.io.Serializable; + +public class HideUtilityClassContructor3041574_2 implements Serializable { + private static final long serialVersionUID = 1L; + +} \ No newline at end of file diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_3.java b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_3.java new file mode 100644 index 000000000..2fc8da047 --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/design/HideUtilityClassContructor3041574_3.java @@ -0,0 +1,48 @@ +package com.puppycrawl.tools.checkstyle.design; + +import java.io.Serializable; + +public class HideUtilityClassContructor3041574_3 implements Serializable { + private static final long serialVersionUID = 1L; + + public HideUtilityClassContructor3041574_3(int i) { + // no code + } + + public String getValue() { + return ""; + } + + // It is NOT Utility Inner class + @SuppressWarnings("unused") + public static class Event { + // Top level class have access to fields - no need in public getters + private String ind; + private String ind1; + + public Event(String value){ + // do a lot of calculations + } + + // static because this method is utility + public static String getEmptyString() { + return ""; + } + } + + // It is Utility Inner class + @SuppressWarnings("unused") + public static class Event1 { + private String ind; + private String ind1; + + private Event1(){ + // do a lot of calculations + } + + // static because this method is utility + public static String getEmptyString() { + return ""; + } + } +} \ No newline at end of file diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheckTest.java index cd4a48051..130fbe7c8 100755 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/design/HideUtilityClassConstructorCheckTest.java @@ -69,4 +69,34 @@ public class HideUtilityClassConstructorCheckTest verify(checkConfig, getPath("design" + File.separator + "InputRegression1762702.java"), expected); } + @Test + public void testEmptyAbstractClass() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(HideUtilityClassConstructorCheck.class); + final String[] expected = { + }; + verify(checkConfig, getPath("design" + File.separator + "HideUtilityClassContructor3041574_1.java"), expected); + } + + @Test + public void testEmptyClassWithOnlyPrivateFields() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(HideUtilityClassConstructorCheck.class); + final String[] expected = { + }; + verify(checkConfig, getPath("design" + File.separator + "HideUtilityClassContructor3041574_2.java"), expected); + } + + @Test + public void testClassWithStaticInnerClass() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(HideUtilityClassConstructorCheck.class); + final String[] expected = { + }; + verify(checkConfig, getPath("design" + File.separator + "HideUtilityClassContructor3041574_3.java"), expected); + } + } diff --git a/src/xdocs/config_design.xml b/src/xdocs/config_design.xml index 5d42c58c8..ccb4e3d9e 100755 --- a/src/xdocs/config_design.xml +++ b/src/xdocs/config_design.xml @@ -219,7 +219,8 @@ public class StringUtils // not final to allow subclassing { protected StringUtils() { - throw new UnsupportedOperationException(); // prevents calls from subclass + // prevents calls from subclass + throw new UnsupportedOperationException(); } public static int count(char c, String s) { diff --git a/src/xdocs/releasenotes.xml b/src/xdocs/releasenotes.xml index 4a40d7087..ea2810b3d 100755 --- a/src/xdocs/releasenotes.xml +++ b/src/xdocs/releasenotes.xml @@ -65,6 +65,12 @@ checking of @throws tags for methods that throw multiple non-runtime exceptions. Thanks to Daan Kets for patch #3039869. +
  • + Fix + HideUtilityClassConstructor + to handle empty and inner classes. Thanks to Roman Ivanov for + patch #3045720. +

    • Notes: