diff --git a/contrib/examples/checks/all-checkstyle-checks.xml b/contrib/examples/checks/all-checkstyle-checks.xml index 5e412e32a..40da4723d 100644 --- a/contrib/examples/checks/all-checkstyle-checks.xml +++ b/contrib/examples/checks/all-checkstyle-checks.xml @@ -266,11 +266,12 @@ - - + + + - + diff --git a/contrib/examples/conf/template_config.xml b/contrib/examples/conf/template_config.xml index 16630613b..107e5bccc 100644 --- a/contrib/examples/conf/template_config.xml +++ b/contrib/examples/conf/template_config.xml @@ -637,9 +637,8 @@ - - + + diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java index af0394e18..318ba99a6 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheck.java @@ -24,9 +24,9 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.checks.AbstractFormatCheck; /** - *

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.

*

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 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 String 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; } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java index ef63ded17..451ec40b1 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java @@ -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); diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/design/InputMutableException.java b/src/test/resources/com/puppycrawl/tools/checkstyle/design/InputMutableException.java index e15eb0f0d..188d2bfc9 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/design/InputMutableException.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/design/InputMutableException.java @@ -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; + } } } diff --git a/src/xdocs/config_design.xml b/src/xdocs/config_design.xml index dac5c564c..63b02d3f9 100644 --- a/src/xdocs/config_design.xml +++ b/src/xdocs/config_design.xml @@ -313,9 +313,9 @@ public class StringUtils // not final to allow subclassing

- 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.

@@ -348,7 +348,13 @@ public class StringUtils // not final to allow subclassing format pattern for exception class names regular expression - ^.*Exception$|^.*Error$ + ^.*Exception$|^.*Error$|^.*Throwable$ + + + extendedClassNameFormat + pattern for extended class names + regular expression + ^.*Exception$|^.*Error$|^.*Throwable$