IllegalCatchCheck (request 750746).
This commit is contained in:
parent
5bac11f295
commit
9aa9d75107
|
|
@ -43,6 +43,9 @@
|
|||
<li>
|
||||
<a href="#HiddenField">HiddenField</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="#IllegalCatch">IllegalCatch</a>
|
||||
</li>
|
||||
<li>
|
||||
<a href="#IllegalInstantiation">IllegalInstantiation</a>
|
||||
</li>
|
||||
|
|
@ -989,6 +992,57 @@ return !valid();
|
|||
<a href="config.html#treewalker">TreeWalker</a>
|
||||
</p>
|
||||
|
||||
<!-- --> <a name="SuperClone"></a> <h2>SuperClone</h2> <h4>Description</h4>
|
||||
<p class="body">
|
||||
Checks that an overriding <span class="code">clone()</span> method invokes <span class="code">super.clone()</span>.
|
||||
</p>
|
||||
<p class="body">
|
||||
Reference: <a href="http://java.sun.com/j2se/1.4.1/docs/api/java/lang/Object.html#clone()">Object.clone()</a>.
|
||||
</p>
|
||||
<h4>Examples</h4>
|
||||
<p class="body">
|
||||
To configure the check:
|
||||
</p>
|
||||
<pre class="body">
|
||||
<module name="SuperClone"/>
|
||||
</pre>
|
||||
<h4>Package</h4>
|
||||
<p class="body">
|
||||
com.puppycrawl.tools.checkstyle.checks.coding
|
||||
</p>
|
||||
<h4>Parent Module</h4>
|
||||
<p class="body">
|
||||
<a href="config.html#treewalker">TreeWalker</a>
|
||||
</p>
|
||||
|
||||
|
||||
<!-- --> <a name="IllegalCatch"></a> <h2>IllegalCatch</h2> <h4>Description</h4>
|
||||
<p class="body">
|
||||
Catching java.lang.Exception, java.lang.Error or java.lang.RuntimeException
|
||||
is almost never acceptable.
|
||||
</p>
|
||||
<p class="body">
|
||||
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.
|
||||
</p>
|
||||
<h4>Examples</h4>
|
||||
<p class="body">
|
||||
To configure the check:
|
||||
</p>
|
||||
<pre class="body">
|
||||
<module name="IllegalCatch"/>
|
||||
</pre>
|
||||
<h4>Package</h4>
|
||||
<p class="body">
|
||||
com.puppycrawl.tools.checkstyle.checks.coding
|
||||
</p>
|
||||
<h4>Parent Module</h4>
|
||||
<p class="body">
|
||||
<a href="config.html#treewalker">TreeWalker</a>
|
||||
</p>
|
||||
|
||||
</td>
|
||||
</tr>
|
||||
</table>
|
||||
|
|
|
|||
|
|
@ -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 <code>FullIdent</code> for given type node.
|
||||
* @param aTypeAST a type node.
|
||||
* @return <code>FullIdent</code> 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 <code>FullIdent</code> for given type.
|
||||
*/
|
||||
private static FullIdent createFullTypeNoArrays(DetailAST aTypeAST)
|
||||
{
|
||||
return FullIdent.createFullIdent((DetailAST) aTypeAST.getFirstChild());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
|
||||
* 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);
|
||||
}
|
||||
}
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue