From 669def894ec6fa19946acd3b586508aed450a4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20K=C3=BChne?= Date: Tue, 18 Feb 2003 05:19:08 +0000 Subject: [PATCH] finally added all the new checks that I built during feature freeze need to do more tests and write docs --- .../checks/ArrayTypeStyleCheck.java | 72 +++++++++ .../checks/AvoidInlineConditionalsCheck.java | 67 ++++++++ .../checks/AvoidNestedBlocksCheck.java | 71 +++++++++ .../checks/DesignForInheritanceCheck.java | 147 ++++++++++++++++++ .../HideUtilityClassConstructorCheck.java | 87 +++++++++++ .../checks/InterfaceIsTypeCheck.java | 90 +++++++++++ .../AvoidInlineConditionalsCheckTest.java | 21 +++ 7 files changed, 555 insertions(+) create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheck.java create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidNestedBlocksCheck.java create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DesignForInheritanceCheck.java create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/HideUtilityClassConstructorCheck.java create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/InterfaceIsTypeCheck.java create mode 100644 src/tests/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheckTest.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java new file mode 100644 index 000000000..a0e97ac68 --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/ArrayTypeStyleCheck.java @@ -0,0 +1,72 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; + +/** + * Checks the style of array type definitions. + * Some like Java-style: public static void main(String[] args) + * and some like C-style: public static void main(String args[]) + * + * By default the Check enforces Java style. + * @author lkuehne + */ +public class ArrayTypeStyleCheck extends Check +{ + /** controls whether to use Java or C style */ + private boolean mJavaStyle = true; + + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {TokenTypes.ARRAY_DECLARATOR}; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + final DetailAST typeAST = aAST.getParent(); + if (typeAST.getType() != TokenTypes.TYPE) { + return; + } + + DetailAST variableAST = (DetailAST) typeAST.getNextSibling(); + boolean isJavaStyle = (variableAST.getColumnNo() > aAST.getColumnNo()); + + if (isJavaStyle != mJavaStyle) { + log(aAST.getLineNo(), aAST.getColumnNo(), + "Array brackets at illegal position."); + } + + } + + /** + * Controls whether to check for Java or C style. + * @param aJavaStyle true if Java style should be used. + */ + public void setJavaStyle(boolean aJavaStyle) + { + mJavaStyle = aJavaStyle; + } + + +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheck.java new file mode 100644 index 000000000..70f2128bd --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheck.java @@ -0,0 +1,67 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; + +/** + * Detects inline conditionals. + * + * An example inline conditional is this: + * 
+ * String a = getParameter("a");
+ * String b = (a==null || a.length<1) ? null : a.substring(1);
+ *
+ * + * Rationale: Some developers find inline conditionals hard to read, + * so their company's coding standards forbids them. + * + * @author lkuehne + * @version $Revision: 1.1 $ + */ +public class AvoidInlineConditionalsCheck extends Check +{ + /** avoids multiple instantiations of equivalten arrays */ + private static final int[] DEFAULT_TOKENS = + new int[]{TokenTypes.QUESTION}; + + /** @see Check */ + public int[] getDefaultTokens() + { + return DEFAULT_TOKENS; + } + + /** @see Check */ + public int[] getRequiredTokens() + { + return DEFAULT_TOKENS; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + // the only place a QUESTION token can occur is in inline conditionals + // so no need to do any further tricks here - pretty trivial Check! + + log(aAST.getLineNo(), aAST.getColumnNo(), + "Avoid inline conditionals."); + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidNestedBlocksCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidNestedBlocksCheck.java new file mode 100644 index 000000000..a05f2940c --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/AvoidNestedBlocksCheck.java @@ -0,0 +1,71 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; + +/** + * Finds nested blocks. + * For example this Check finds the obsolete braces in + * 
+ * switch (a)
+ * {
+ *     case 0:
+ *         {
+ *             x = 1;
+ *         }
+ *         break;
+ *     default:
+ *         break;
+ * }
+ *
+ * and flags confusing code like + * 
+ * public void guessTheOutput()
+ * {
+ *     int whichIsWich = 0;
+ *     {
+ *         int whichIsWhich = 2;
+ *     }
+ *     System.out.println("value = " + whichIsWhich);
+ * }
+ *
+ * + * @author lkuehne + */ +public class AvoidNestedBlocksCheck extends Check +{ + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {TokenTypes.SLIST}; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + if (aAST.getParent().getType() == TokenTypes.SLIST) { + // TODO: i18n + log(aAST.getLineNo(), aAST.getColumnNo(), "Avoid nested blocks"); + } + } + +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DesignForInheritanceCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DesignForInheritanceCheck.java new file mode 100644 index 000000000..cc0310983 --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DesignForInheritanceCheck.java @@ -0,0 +1,147 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.ScopeUtils; + +/** + * Checks that classes are designed for inheritance. + * + *

+ * More specifically, it enforces a programming style + * where superclasses provide empty "hooks" that can be + * implemented by subclasses. + *

+ * + *

+ * The exact rule is that nonprivate methods in + * nonfinal classes (or classes that do not + * only have private constructors) must either be + *

+ *

+ * + *

+ * This protects superclasses against beeing broken by + * subclasses. The downside is that subclasses are limited + * in their flexibility, in particular they cannot prevent + * execution of code in the superclass, but that also + * means that subclasses can't forget to call their super + * method. + *

+ * + * @author lkuehne + * @version $Revision: 1.1 $ + */ +public class DesignForInheritanceCheck extends Check +{ + /** the default tokens for this check */ + private static final int[] DEFAULT_TOKENS = + new int[] {TokenTypes.METHOD_DEF}; + + /** @see Check */ + public int[] getDefaultTokens() + { + return DEFAULT_TOKENS; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + // nothing to do for Interfaces + if (ScopeUtils.inInterfaceBlock(aAST)) { + return; + } + + // method is ok if it is private or abstract or final + DetailAST modifiers = aAST.findFirstToken(TokenTypes.MODIFIERS); + if (modifiers.branchContains(TokenTypes.LITERAL_PRIVATE) + || modifiers.branchContains(TokenTypes.ABSTRACT) + || modifiers.branchContains(TokenTypes.FINAL)) + { + return; + } + + // method is ok if it is empty + DetailAST implemetation = aAST.findFirstToken(TokenTypes.SLIST); + if (implemetation.getFirstChild().getType() == TokenTypes.RCURLY) { + return; + } + + // check if the containing class can be subclassed + DetailAST classDef = findContainingClass(aAST); + DetailAST classMods = classDef.findFirstToken(TokenTypes.MODIFIERS); + if (classMods.branchContains(TokenTypes.FINAL)) { + return; + } + + // check if subclassing is prevented by having only private ctors + DetailAST objBlock = classDef.findFirstToken(TokenTypes.OBJBLOCK); + + boolean hasDefaultConstructor = true; + boolean hasExplNonPrivateCtor = false; + + DetailAST candidate = (DetailAST) objBlock.getFirstChild(); + + while (candidate != null) { + if (candidate.getType() == TokenTypes.CTOR_DEF) { + hasDefaultConstructor = false; + + DetailAST ctorMods = + candidate.findFirstToken(TokenTypes.MODIFIERS); + if (!ctorMods.branchContains(TokenTypes.LITERAL_PRIVATE)) { + hasExplNonPrivateCtor = true; + break; + } + } + candidate = (DetailAST) candidate.getNextSibling(); + } + + if (hasDefaultConstructor || hasExplNonPrivateCtor) { + String name = aAST.findFirstToken(TokenTypes.IDENT).getText(); + // TODO: i18n + log(aAST.getLineNo(), aAST.getColumnNo(), + "Method '" + name + "' is not designed for inheritance " + + "- needs to be abstract, final or empty."); + } + + + + } + + /** + * Searches the tree towards the root until it finds a CLASS_DEF node. + * @param aAST the start node for searching + * @return the CLASS_DEF node. + */ + private DetailAST findContainingClass(DetailAST aAST) + { + DetailAST searchAST = aAST; + while (searchAST.getType() != TokenTypes.CLASS_DEF) { + searchAST = searchAST.getParent(); + } + return searchAST; + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/HideUtilityClassConstructorCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/HideUtilityClassConstructorCheck.java new file mode 100644 index 000000000..9eb23867b --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/HideUtilityClassConstructorCheck.java @@ -0,0 +1,87 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; + +/** + * Make sure that utility classes (classes that contain only static methods) + * do not have a public constructor. + *

+ * Rationale: Instantiating utility classes does not make sense. + * A common mistake is forgetting to hide the default constructor. + *

+ * + * @author lkuehne + * @version $Revision: 1.1 $ + */ +public class HideUtilityClassConstructorCheck extends Check +{ + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {TokenTypes.CLASS_DEF}; + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + DetailAST objBlock = aAST.findFirstToken(TokenTypes.OBJBLOCK); + DetailAST child = (DetailAST) objBlock.getFirstChild(); + boolean hasMethod = false; + boolean hasNonStaticMethod = false; + boolean hasDefaultCtor = true; + boolean hasPublicCtor = false; + + while (child != null) { + if (child.getType() == TokenTypes.METHOD_DEF) { + hasMethod = true; + final DetailAST modifiers = + child.findFirstToken(TokenTypes.MODIFIERS); + if (!modifiers.branchContains(TokenTypes.LITERAL_STATIC)) { + hasNonStaticMethod = true; + } + } + if (child.getType() == TokenTypes.CTOR_DEF) { + hasDefaultCtor = false; + final DetailAST modifiers = + child.findFirstToken(TokenTypes.MODIFIERS); + if (!modifiers.branchContains(TokenTypes.LITERAL_PRIVATE) + && !modifiers.branchContains(TokenTypes.LITERAL_PROTECTED)) + { + // treat package visible as public + // for the purpose of this Check + hasPublicCtor = true; + } + + } + child = (DetailAST) child.getNextSibling(); + } + + boolean hasAccessibleCtor = (hasDefaultCtor || hasPublicCtor); + + if (hasMethod && !hasNonStaticMethod && hasAccessibleCtor) { + log(aAST.getLineNo(), aAST.getColumnNo(), + "Utility classes should not have " + + "a public or default constructor."); + } + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/InterfaceIsTypeCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/InterfaceIsTypeCheck.java new file mode 100644 index 000000000..18227574d --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/InterfaceIsTypeCheck.java @@ -0,0 +1,90 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2002 Oliver Burn +// +// 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.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.DetailAST; + +/** + * Implements Bloch, Effective Java, Item 17 - + * Use Interfaces only to define types. + * + *

+ * An interface should describe a type, it is therefore + * inappropriate to define an interface that does not contain any methods + * but only constants. + *

+ * + *

+ * The check can be configured to also disallow marker interfaces like + * java.io.Serializable, that do not contain methods or + * constants at all. + *

+ * + * @author lkuehne + * @version $Revision: 1.1 $ + */ +public final class InterfaceIsTypeCheck + extends Check +{ + /** flag to control whether marker interfaces are allowed. */ + private boolean mAllowMarkerInterfaces = true; + + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {TokenTypes.INTERFACE_DEF}; + } + + /** @see Check */ + public int[] getRequiredTokens() + { + return getDefaultTokens(); + } + + /** @see Check */ + public void visitToken(DetailAST aAST) + { + final DetailAST objBlock = + aAST.findFirstToken(TokenTypes.OBJBLOCK); + final DetailAST methodDef = + objBlock.findFirstToken(TokenTypes.METHOD_DEF); + final DetailAST variableDef = + objBlock.findFirstToken(TokenTypes.VARIABLE_DEF); + boolean methodRequired = + !mAllowMarkerInterfaces || variableDef != null; + + if (methodDef == null && methodRequired) { + // TODO: i18n + log(aAST.getLineNo(), "interfaces should describe a type " + + "and hence have methods."); + } + + } + + /** + * Controls whether marker interfaces like Serializable are allowed. + * @param aFlag whether to allow marker interfaces or not + */ + public void setAllowMarkerInterfaces(boolean aFlag) + { + mAllowMarkerInterfaces = aFlag; + } +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheckTest.java new file mode 100644 index 000000000..30c94a84f --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/AvoidInlineConditionalsCheckTest.java @@ -0,0 +1,21 @@ +package com.puppycrawl.tools.checkstyle.checks; + +import com.puppycrawl.tools.checkstyle.BaseCheckTestCase; +import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + +public class AvoidInlineConditionalsCheckTest + extends BaseCheckTestCase +{ + public void testIt() + throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(AvoidInlineConditionalsCheck.class); + final String[] expected = { + "97:29: Avoid inline conditionals.", + "98:20: Avoid inline conditionals.", + "150:34: Avoid inline conditionals.", + }; + verify(checkConfig, getPath("InputWhitespace.java"), expected); + } +}