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

MutableException check requires class to explicitly extend some other class (for issue #60)

This commit is contained in:
Michal Kordas 2014-12-19 00:22:18 +01:00 committed by Roman Ivanov
parent 17ebdc5c37
commit 4297ca50a9
6 changed files with 99 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
contrib/examples/checks/all-checkstyle-checks.xml
View File

@ -266,11 +266,12 @@
<property name="allowMarkerInterfaces" value="true"/>
</module>
<!-- Ensures that exceptions (defined as any class name conforming to some regular expression) are immutable. !-->
<!-- That is, have only final fields. !-->
<!-- Ensures that exceptions (classes with names conforming to some regular expression and explicitly !-->
<!-- extending classes with names conforming to other regular expression) are immutable. !-->
<!-- That is, they have only final fields. !-->
<!-- See http://checkstyle.sf.net/config_design.html !-->
<module name="MutableException">
<property name="format" value="^.*Exception$|^.*Error$"/>
<property name="format" value="^.*Exception$|^.*Error$|^.*Throwable$"/>
</module>
<!-- Restricts throws statements to a specified count. !-->

5
contrib/examples/conf/template_config.xml
View File

@ -637,9 +637,8 @@
</module>
<!-- See http://checkstyle.sf.net/config_design.html -->
<!-- Ensures that exceptions (defined as any class name conforming
to some regular expression) are immutable. That is, have only final fields. -->
<!-- <property name="format" value="^.*Exception$|^.*Error$"/> -->
<!-- Ensures that exceptions classes are immutable. !-->
<!-- <property name="format" value="^.*Exception$|^.*Error$|^.*Throwable$"/> -->
<module name="MutableException">
</module>

57
src/main/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java
View File

@ -24,9 +24,9 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.AbstractFormatCheck;
/**
* <p> Ensures that exceptions (defined as any class name conforming
* to some regular expression) are immutable. That is, have only final
* fields.</p>
* <p> Ensures that exceptions (classes with names conforming to some regular
* expression and explicitly extending classes with names conforming to other
* regular expression) are immutable. That is, they 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
@ -38,8 +38,10 @@ import com.puppycrawl.tools.checkstyle.checks.AbstractFormatCheck;
*/
public final class MutableExceptionCheck extends AbstractFormatCheck
{
/** Default value for format property. */
private static final String DEFAULT_FORMAT = "^.*Exception$|^.*Error$";
/** Default value for format and extendedClassNameFormat properties. */
private static final String DEFAULT_FORMAT = "^.*Exception$|^.*Error$|^.*Throwable$";
/** Pattern for class name that is being extended */
private String mExtendedClassNameFormat;
/** Stack of checking information for classes. */
private final FastStack<Boolean> mCheckingStack = FastStack.newInstance();
/** Should we check current class or not. */
@ -49,6 +51,16 @@ public final class MutableExceptionCheck extends AbstractFormatCheck
public MutableExceptionCheck()
{
super(DEFAULT_FORMAT);
setExtendedClassNameFormat(DEFAULT_FORMAT);
}
/**
* Sets the format of extended class name to the specified regular expression.
* @param aExtendedClassNameFormat a <code>String</code> value
*/
public void setExtendedClassNameFormat(String aExtendedClassNameFormat)
{
mExtendedClassNameFormat = aExtendedClassNameFormat;
}
@Override
@ -97,19 +109,18 @@ public final class MutableExceptionCheck extends AbstractFormatCheck
private void visitClassDef(DetailAST aAST)
{
mCheckingStack.push(mChecking ? Boolean.TRUE : Boolean.FALSE);
mChecking =
isExceptionClass(aAST.findFirstToken(TokenTypes.IDENT).getText());
mChecking = isNamedAsException(aAST) && isExtendedClassNamedAsException(aAST);
}
/** Called when we leave class definition. */
private void leaveClassDef()
{
mChecking = (mCheckingStack.pop()).booleanValue();
mChecking = mCheckingStack.pop();
}
/**
* Checks variable definition.
* @param aAST variable def node for check.
* @param aAST variable def node for check
*/
private void visitVariableDef(DetailAST aAST)
{
@ -117,7 +128,7 @@ public final class MutableExceptionCheck extends AbstractFormatCheck
final DetailAST modifiersAST =
aAST.findFirstToken(TokenTypes.MODIFIERS);
if (!(modifiersAST.findFirstToken(TokenTypes.FINAL) != null)) {
if (modifiersAST.findFirstToken(TokenTypes.FINAL) == null) {
log(aAST.getLineNo(), aAST.getColumnNo(), "mutable.exception",
aAST.findFirstToken(TokenTypes.IDENT).getText());
}
@ -125,11 +136,29 @@ public final class MutableExceptionCheck extends AbstractFormatCheck
}
/**
* @param aClassName class name to check
* @return true if a given class name confirms specified format
* @param aAST class definition node
* @return true if a class name conforms to specified format
*/
private boolean isExceptionClass(String aClassName)
private boolean isNamedAsException(DetailAST aAST)
{
return getRegexp().matcher(aClassName).find();
final String className = aAST.findFirstToken(TokenTypes.IDENT).getText();
return getRegexp().matcher(className).find();
}
/**
* @param aAST class definition node
* @return true if extended class name conforms to specified format
*/
private boolean isExtendedClassNamedAsException(DetailAST aAST)
{
final DetailAST extendsClause = aAST.findFirstToken(TokenTypes.EXTENDS_CLAUSE);
if (extendsClause != null) {
final DetailAST extendedClass = extendsClause.findFirstToken(TokenTypes.IDENT);
if (extendedClass != null) {
final String extendedClassName = extendedClass.getText();
return extendedClassName.matches(mExtendedClassNameFormat);
}
}
return false;
}
}

22
src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java
View File

@ -20,19 +20,33 @@ package com.puppycrawl.tools.checkstyle.checks.design;
import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
import java.io.File;
import org.junit.Test;
import java.io.File;
public class MutableExceptionCheckTest extends BaseCheckTestSupport
{
@Test
public void test() throws Exception
public void testDefault() 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.",
"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);
}
@Test
public void testFormat() throws Exception
{
DefaultConfiguration checkConfig = createCheckConfig(MutableExceptionCheck.class);
checkConfig.addAttribute("format", "^.*Failure$");
checkConfig.addAttribute("extendedClassNameFormat", "^.*ThreadDeath$");
String[] expected = {
"34:13: The field 'errorCode' must be declared final.",
};
verify(checkConfig, getPath("design" + File.separator + "InputMutableException.java"), expected);

32
src/test/resources/com/puppycrawl/tools/checkstyle/design/InputMutableException.java
View File

@ -1,25 +1,37 @@
package com.puppycrawl.tools.checkstyle.design;
public class InputMutableException {
public class FooException {
private final int _finalErrorCode;
private int _errorCode = 1;
public class FooException extends Exception {
private final int finalErrorCode;
private int errorCode = 1;
public FooException() {
_finalErrorCode = 1;
finalErrorCode = 1;
}
public class FooExceptionThisIsNot {
private final int _finalErrorCode;
private int _errorCode = 1;
public class FooExceptionThisIsNot extends RuntimeException {
private final int finalErrorCode;
private int errorCode = 1;
/** constructor */
public FooExceptionThisIsNot() {
_finalErrorCode = 1;
finalErrorCode = 1;
}
}
}
public class FooError {
private int _errorCode;
public class BarError extends Throwable {
private int errorCode;
}
public class BazDoesNotExtendError {
private int errorCode;
}
public class CustomProblem extends ThreadDeath {
private int errorCode;
public class CustomFailure extends ThreadDeath {
private int errorCode;
}
}
}

14
src/xdocs/config_design.xml
View File

@ -313,9 +313,9 @@ public class StringUtils // not final to allow subclassing
<section name="MutableException">
<subsection name="Description">
<p>
Ensures that exception classes (classes with names conforming to
some regular expression) are immutable, that is, that they have only final
fields.
Ensures that exception classes (classes with names conforming to some regular
expression and explicitly extending classes with names conforming to other
regular expression) are immutable, that is, that they have only final fields.
</p>
<p>
@ -348,7 +348,13 @@ public class StringUtils // not final to allow subclassing
<td>format</td>
<td>pattern for exception class names</td>
<td><a href="property_types.html#regexp">regular expression</a></td>
<td><code>^.*Exception$|^.*Error$</code></td>
<td><code>^.*Exception$|^.*Error$|^.*Throwable$</code></td>
</tr>
<tr>
<td>extendedClassNameFormat</td>
<td>pattern for extended class names</td>
<td><a href="property_types.html#regexp">regular expression</a></td>
<td><code>^.*Exception$|^.*Error$|^.*Throwable$</code></td>
</tr>
</table>
</subsection>