From 9aa9d751071e0597b268235f2dff4fc6373b2d5f Mon Sep 17 00:00:00 2001 From: Oleg Sukhodolsky Date: Thu, 21 Aug 2003 20:04:46 +0000 Subject: [PATCH] IllegalCatchCheck (request 750746). --- docs/config_coding.html | 54 +++++++++++ .../tools/checkstyle/checks/CheckUtils.java | 24 +++++ .../checks/coding/IllegalCatchCheck.java | 90 +++++++++++++++++++ .../checks/coding/messages.properties | 2 + .../coding/InputIllegalCatchCheck.java | 19 ++++ .../puppycrawl/tools/checkstyle/AllTests.java | 2 + .../checks/coding/IllegalCatchCheckTest.java | 25 ++++++ 7 files changed, 216 insertions(+) create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheck.java create mode 100644 src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputIllegalCatchCheck.java create mode 100644 src/tests/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheckTest.java diff --git a/docs/config_coding.html b/docs/config_coding.html index 96b5d7de2..8b35db278 100644 --- a/docs/config_coding.html +++ b/docs/config_coding.html @@ -43,6 +43,9 @@
  • HiddenField
    • +
  • + IllegalCatch +
  • IllegalInstantiation
    • @@ -989,6 +992,57 @@ return !valid(); TreeWalker

    +

    SuperClone

    Description

    +

    + Checks that an overriding clone() method invokes super.clone(). +

    +

    + Reference: Object.clone(). +

    +

    Examples

    +

    + To configure the check: +

    + 
    +<module name="SuperClone"/>
+
    +

    Package

    +

    + com.puppycrawl.tools.checkstyle.checks.coding +

    +

    Parent Module

    +

    + TreeWalker +

    + + +

    IllegalCatch

    Description

    +

    + Catching java.lang.Exception, java.lang.Error or java.lang.RuntimeException + is almost never acceptable. +

    +

    + Rationale: Junior developers often simply catch Exception in an + attempt to handle multiple exception classes. This unfortunately + leads to code that inadvertantly catchs NPE, OutOfMemoryErrors, + etc. +

    +

    Examples

    +

    + To configure the check: +

    + 
    +<module name="IllegalCatch"/>
+
    +

    Package

    +

    + com.puppycrawl.tools.checkstyle.checks.coding +

    +

    Parent Module

    +

    + TreeWalker +

    + diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/CheckUtils.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/CheckUtils.java index 5aadcf7a5..20effddc3 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/CheckUtils.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/CheckUtils.java @@ -19,6 +19,7 @@ package com.puppycrawl.tools.checkstyle.checks; import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.FullIdent; import com.puppycrawl.tools.checkstyle.api.TokenTypes; /** @@ -106,4 +107,27 @@ public final class CheckUtils && (aAST.getChildCount() == 2) && isElse(aAST.getParent()); } + + /** + * Creates FullIdent for given type node. + * @param aTypeAST a type node. + * @return FullIdent for given type. + */ + public static FullIdent createFullType(DetailAST aTypeAST) + { + DetailAST arrayDeclAST = + aTypeAST.findFirstToken(TokenTypes.ARRAY_DECLARATOR); + + return createFullTypeNoArrays(arrayDeclAST == null ? aTypeAST + : arrayDeclAST); + } + + /** + * @param aTypeAST a type node (no array) + * @return FullIdent for given type. + */ + private static FullIdent createFullTypeNoArrays(DetailAST aTypeAST) + { + return FullIdent.createFullIdent((DetailAST) aTypeAST.getFirstChild()); + } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheck.java new file mode 100644 index 000000000..220786627 --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheck.java @@ -0,0 +1,90 @@ +//////////////////////////////////////////////////////////////////////////////// +// checkstyle: Checks Java source code for adherence to a set of rules. +// Copyright (C) 2001-2003 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.coding; + +import com.puppycrawl.tools.checkstyle.api.Check; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.FullIdent; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.checks.CheckUtils; + +import java.util.HashSet; +import java.util.Set; + +/** + * Catching java.lang.Exception, java.lang.Error or java.lang.RuntimeException + * is almost never acceptable. + * @author Simon Harris + * TODO: Merge some code and make a base class with IllegalThrowsCheck + */ +public final class IllegalCatchCheck extends Check +{ + /** Illegal class names */ + private final Set mIllegalClassNames = new HashSet(); + + /** Creates new instance of the check. */ + public IllegalCatchCheck() + { + mIllegalClassNames.add("Exception"); + mIllegalClassNames.add("Error"); + mIllegalClassNames.add("RuntimeException"); + mIllegalClassNames.add("Throwable"); + mIllegalClassNames.add("java.lang.Error"); + mIllegalClassNames.add("java.lang.Exception"); + mIllegalClassNames.add("java.lang.RuntimeException"); + mIllegalClassNames.add("java.lang.Throwable"); + } + + /** @see Check */ + public int[] getDefaultTokens() + { + return new int[] {TokenTypes.LITERAL_CATCH}; + } + + /** @see Check */ + public int[] getRequiredTokens() + { + return getDefaultTokens(); + } + + /** @see Check */ + public void visitToken(DetailAST aDetailAST) + { + DetailAST paramDef = + aDetailAST.findFirstToken(TokenTypes.PARAMETER_DEF); + DetailAST excType = paramDef.findFirstToken(TokenTypes.TYPE); + FullIdent ident = CheckUtils.createFullType(excType); + + if (isIllegalClassName(ident.getText())) { + log(aDetailAST.getLineNo(), + aDetailAST.getColumnNo(), + "illegal.catch", ident.getText()); + } + } + + /** + * Checks if given exception class is illegal. + * @param aIdent ident to check. + * @return true if given ident is illegal. + */ + private boolean isIllegalClassName(String aIdent) + { + return mIllegalClassNames.contains(aIdent); + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties index 057147177..47792a74c 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/messages.properties @@ -35,3 +35,5 @@ nested.if.depth=Nested if-else depth is {0,number,integer} (max allowed is {1,nu nested.try.depth=Nested try depth is {0,number,integer} (max allowed is {1,number,integer}). string.literal.equality=Literal Strings should be compared using equals(), not ''{0}''. + +illegal.catch=Catching ''{0}'' is not allowed. diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputIllegalCatchCheck.java b/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputIllegalCatchCheck.java new file mode 100644 index 000000000..6440e4fa2 --- /dev/null +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputIllegalCatchCheck.java @@ -0,0 +1,19 @@ +package com.puppycrawl.tools.checkstyle.checks.coding; + +public class InputIllegalCatchCheck { + public void foo() { + try { + } catch (RuntimeException e) { + } catch (Exception e) { + } catch (Throwable e) { + } + } + + public void bar() { + try { + } catch (java.lang.RuntimeException e) { + } catch (java.lang.Exception e) { + } catch (java.lang.Throwable e) { + } + } +} diff --git a/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java b/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java index 06f97b4d0..3aa21ec0d 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/AllTests.java @@ -27,6 +27,7 @@ import com.puppycrawl.tools.checkstyle.checks.coding.DoubleCheckedLockingCheckTe import com.puppycrawl.tools.checkstyle.checks.coding.EmptyStatementCheckTest; import com.puppycrawl.tools.checkstyle.checks.coding.EqualsHashCodeCheckTest; import com.puppycrawl.tools.checkstyle.checks.coding.HiddenFieldCheckTest; +import com.puppycrawl.tools.checkstyle.checks.coding.IllegalCatchCheckTest; import com.puppycrawl.tools.checkstyle.checks.coding.IllegalInstantiationCheckTest; import com.puppycrawl.tools.checkstyle.checks.coding.IllegalTokenCheckTest; import com.puppycrawl.tools.checkstyle.checks.coding.IllegalTokenTextCheckTest; @@ -140,6 +141,7 @@ public class AllTests { suite.addTest(new TestSuite(HeaderCheckTest.class)); suite.addTest(new TestSuite(HiddenFieldCheckTest.class)); suite.addTest(new TestSuite(HideUtilityClassConstructorCheckTest.class)); + suite.addTest(new TestSuite(IllegalCatchCheckTest.class)); suite.addTest(new TestSuite(IllegalImportCheckTest.class)); suite.addTest(new TestSuite(IllegalInstantiationCheckTest.class)); suite.addTest(new TestSuite(IllegalTokenCheckTest.class)); diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheckTest.java new file mode 100644 index 000000000..e55aa6696 --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/IllegalCatchCheckTest.java @@ -0,0 +1,25 @@ +package com.puppycrawl.tools.checkstyle.checks.coding; + +import com.puppycrawl.tools.checkstyle.BaseCheckTestCase; +import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + +import java.io.File; + +public class IllegalCatchCheckTest extends BaseCheckTestCase +{ + public void test() throws Exception + { + DefaultConfiguration checkConfig = createCheckConfig(IllegalCatchCheck.class); + + String[] expected = { + "6:11: Catching 'RuntimeException' is not allowed.", + "7:11: Catching 'Exception' is not allowed.", + "8:11: Catching 'Throwable' is not allowed.", + "14:11: Catching 'java.lang.RuntimeException' is not allowed.", + "15:11: Catching 'java.lang.Exception' is not allowed.", + "16:11: Catching 'java.lang.Throwable' is not allowed.", + }; + + verify(checkConfig, getPath("coding" + File.separator + "InputIllegalCatchCheck.java"), expected); + } +}