TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Added MutableException check (request 750750). 

This is a very simple algorithm, but, I believe, it catches most of
real-life problems.
This commit is contained in:
Oleg Sukhodolsky 2003-08-26 20:06:58 +00:00
parent 92d37dd95e
commit 5245c14883
7 changed files with 250 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
docs/config_design.html
View File

@ -23,19 +23,22 @@
<td class="menu" valign="top">
<ul>
<li>
<a href="#VisibilityModifier">VisibilityModifier</a>
<a href="#DesignForExtension">DesignForExtension</a>
</li>
<li>
<a href="#FinalClass">FinalClass</a>
</li>
<li>
<a href="#InterfaceIsType">InterfaceIsType</a>
</li>
<li>
<a href="#HideUtilityClassConstructor">HideUtilityClassConstructor</a>
</li>
<li>
<a href="#DesignForExtension">DesignForExtension</a>
<a href="#InterfaceIsType">InterfaceIsType</a>
</li>
<li>
<a href="#MutableException">MutableException</a>
</li>
<li>
<a href="#VisibilityModifier">VisibilityModifier</a>
</li>
</ul>
</td>
@ -284,6 +287,64 @@ public class StringUtils // not final to allow subclassing
<a href="config.html#treewalker">TreeWalker</a>
</p>
<!-- --> <a name="MutableException"></a> <h2>MutableException</h2> <h4>Description</h4>
<p class="body">
Ensures that exceptions (defined as any class name conforming
to some regular expression) are immutable. That is, have only
final fields.
</p>
<p class="body">
The current algorithm is very simple it checks that all
members of exception are final. User can still mutates an
exception's instance (e.g. Throwable has
setStackTrace(StackTraceElement[] stackTrace) method which
changes stack trace). But, at least, all information provided
by this exception type is unchangable.
</p>
<p class="body">
Rationale:
Exception instances should represent an error
condition. Having non final fields not only allows the
state to be modified by accident and therefore mask the
original condition but also allows developers to
accidentally forget to initialise state thereby leading
to code catching the exception to draw incorrect
conclusions based on the state.
</p>
<h4>Properties</h4>
<table width="100%" border="1" cellpadding="5" class="body">
<tr class="header">
<th>name</th>
<th>description</th>
<th>type</th>
<th>default value</th>
</tr>
<tr>
<td>format</td>
<td>pattern for name of exception class.</td>
<td><a href="property_types.html#regexp">regular expression</a></td>
<td><span class="default">^.*Exception$|^.*Error$</span></td>
</tr>
</table>
<h4>Examples</h4>
<p class="body">
To configure the check:
</p>
<pre class="body">
&lt;module name=&quot;MutableException&quot;/&gt;
</pre>
<h4>Package</h4>
<p class="body">
com.puppycrawl.tools.checkstyle.checks.design
</p>
<h4>Parent Module</h4>
<p class="body">
<a href="config.html#treewalker">TreeWalker</a>
</p>
</td>
</tr>

2
docs/releasenotes.html
View File

@ -147,6 +147,8 @@
<li class="body">Added JUnitTestCase check. (request 750761).</li>
<li class="body">Added MutableException check (request 750750).</li>
</ul>
<p class="body">

135
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java Normal file
View File

@ -0,0 +1,135 @@
////////////////////////////////////////////////////////////////////////////////
// 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.design;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.AbstractFormatCheck;
import java.util.Stack;
/**
* <p> Ensures that exceptions (defined as any class name conforming
* to some regular expression) are immutable. That is, have only final
* fields.</p>
* <p> Rationale: Exception instances should represent an error
* condition. Having non final fields not only allows the state to be
* modified by accident and therefore mask the original condition but
* also allows developers to accidentally forget to initialise state
* thereby leading to code catching the exception to draw incorrect
* conclusions based on the state.</p>
*
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
*/
public final class MutableExceptionCheck extends AbstractFormatCheck
{
/** Default value for format property. */
private static final String DEFAULT_FORMAT = "^.*Exception$|^.*Error$";
/** Stack of checking information for classes. */
private final Stack mCheckingStack = new Stack();
/** Should we check current class or not. */
private boolean mChecking;
/** Creates new instance of the check. */
public MutableExceptionCheck()
{
super(DEFAULT_FORMAT);
}
/** @see com.puppycrawl.tools.checkstyle.api.Check */
public int[] getDefaultTokens()
{
return new int[] {TokenTypes.CLASS_DEF, TokenTypes.VARIABLE_DEF};
}
/** @see com.puppycrawl.tools.checkstyle.api.Check */
public int[] getRequiredTokens()
{
return getDefaultTokens();
}
/** @see com.puppycrawl.tools.checkstyle.api.Check */
public void visitToken(DetailAST aAST)
{
switch (aAST.getType()) {
case TokenTypes.CLASS_DEF:
visitClassDef(aAST);
break;
case TokenTypes.VARIABLE_DEF:
visitVariableDef(aAST);
break;
default:
throw new IllegalStateException(aAST.toString());
}
}
/** @see com.puppycrawl.tools.checkstyle.api.Check */
public void leaveToken(DetailAST aAST)
{
switch (aAST.getType()) {
case TokenTypes.CLASS_DEF:
leaveClassDef();
break;
default:
// Do nothing
}
}
/**
* Called when we start processing class definition.
* @param aAST class definition node
*/
private void visitClassDef(DetailAST aAST)
{
mCheckingStack.push(Boolean.valueOf(mChecking));
mChecking =
isExceptionClass(aAST.findFirstToken(TokenTypes.IDENT).getText());
}
/** Called when we leave class definition. */
private void leaveClassDef()
{
mChecking = ((Boolean) mCheckingStack.pop()).booleanValue();
}
/**
* Checks variable definition.
* @param aAST variable def node for check.
*/
private void visitVariableDef(DetailAST aAST)
{
if (mChecking && aAST.getParent().getType() == TokenTypes.OBJBLOCK) {
DetailAST modifiersAST = aAST.findFirstToken(TokenTypes.MODIFIERS);
if (!(modifiersAST.findFirstToken(TokenTypes.FINAL) != null)) {
log(aAST.getLineNo(), aAST.getColumnNo(), "mutable.exception",
aAST.findFirstToken(TokenTypes.IDENT).getText());
}
}
}
/**
* @param aClassName class name to check
* @return true if a given class name confirms specified format
*/
private boolean isExceptionClass(String aClassName)
{
return getRegexp().match(aClassName);
}
}

1
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/design/messages.properties
View File

@ -2,3 +2,4 @@ design.forExtension=Method ''{0}'' is not designed for extension - needs to be a
final.class=Class {0} should be declared as final.
interface.type=interfaces should describe a type and hence have methods.
variable.notPrivate=Variable ''{0}'' must be private and have accessor methods.
mutable.exception=The field ''{0}'' must be declared final.

25
src/testinputs/com/puppycrawl/tools/checkstyle/design/InputMutableException.java Normal file
View File

@ -0,0 +1,25 @@
package com.puppycrawl.tools.checkstyle.checks.design;
public class InputMutableException {
public class FooException {
private final int _finalErrorCode;
private int _errorCode = 1;
public FooException() {
_finalErrorCode = 1;
}
public class FooExceptionThisIsNot {
private final int _finalErrorCode;
private int _errorCode = 1;
public FooExceptionThisIsNot() {
_finalErrorCode = 1;
}
}
}
public class FooError {
private int _errorCode;
}
}

2
src/tests/com/puppycrawl/tools/checkstyle/AllTests.java
View File

@ -48,6 +48,7 @@ import com.puppycrawl.tools.checkstyle.checks.design.DesignForExtensionCheckTest
import com.puppycrawl.tools.checkstyle.checks.design.FinalClassCheckTest;
import com.puppycrawl.tools.checkstyle.checks.design.HideUtilityClassConstructorCheckTest;
import com.puppycrawl.tools.checkstyle.checks.design.InterfaceIsTypeCheckTest;
import com.puppycrawl.tools.checkstyle.checks.design.MutableExceptionCheckTest;
import com.puppycrawl.tools.checkstyle.checks.design.VisibilityModifierCheckTest;
import com.puppycrawl.tools.checkstyle.checks.imports.AvoidStarImportTest;
import com.puppycrawl.tools.checkstyle.checks.imports.IllegalImportCheckTest;
@ -164,6 +165,7 @@ public class AllTests {
suite.addTest(new TestSuite(MethodLengthCheckTest.class));
suite.addTest(new TestSuite(MethodNameCheckTest.class));
suite.addTest(new TestSuite(ModifierOrderCheckTest.class));
suite.addTest(new TestSuite(MutableExceptionCheckTest.class));
suite.addTest(new TestSuite(NeedBracesCheckTest.class));
suite.addTest(new TestSuite(NestedIfDepthCheckTest.class));
suite.addTest(new TestSuite(NestedTryDepthCheckTest.class));

19
src/tests/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java Normal file
View File

@ -0,0 +1,19 @@
package com.puppycrawl.tools.checkstyle.checks.design;
import com.puppycrawl.tools.checkstyle.BaseCheckTestCase;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
import java.io.File;
public class MutableExceptionCheckTest extends BaseCheckTestCase {
public void test() throws Exception {
DefaultConfiguration checkConfig = createCheckConfig(MutableExceptionCheck.class);
String[] expected = {
"6:9: The field '_errorCode' must be declared final.",
"23:9: The field '_errorCode' must be declared final.",
};
verify(checkConfig, getPath("design" + File.separator + "InputMutableException.java"), expected);
}
}