Added CyclomaticComplexityCheck from Simon Harris.

Refactored quite a bit
This commit is contained in:
Oliver Burn 2003-06-24 12:35:40 +00:00
parent 3da302031b
commit 3a75454e8d
17 changed files with 535 additions and 20 deletions

View File

@ -118,5 +118,8 @@
<property name="caseIndent" value="0"/>
</module>
<module name="ArrayTrailingComma"/>
<!-- Generates quite a few errors -->
<!-- module name="CyclomaticComplexity"/ -->
</module>
</module>

View File

@ -87,6 +87,8 @@
<li class="body">Added excludes property to AvoidStarImport, contributed
by Bill Schneider (request 744955).</li>
<li class="body">Added CyclomaticComplexityCheck from Simon Harris.</li>
</ul>

View File

@ -37,7 +37,6 @@ import java.util.Set;
public abstract class AbstractTypeAwareCheck
extends Check
{
/** imports details **/
private Set mImports = new HashSet();

View File

@ -0,0 +1,109 @@
////////////////////////////////////////////////////////////////////////////////
// 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;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
/**
* Contains utility methods for the checks.
*
* @author Oliver Burn
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
*/
public final class CheckUtils
{
// TODO: Make this a regex?
// private static final String EQUALS_METHOD_NAME = "equals";
/** prevent instances */
private CheckUtils()
{
throw new UnsupportedOperationException();
}
// public static FullIdent createFullType(DetailAST typeAST) {
// DetailAST arrayDeclAST =
// typeAST.findFirstToken(TokenTypes.ARRAY_DECLARATOR);
//
// return createFullTypeNoArrays(
// arrayDeclAST == null ? typeAST : arrayDeclAST);
// }
//
// public static boolean isEqualsMethod(DetailAST detailAST) {
// return detailAST.findFirstToken(TokenTypes.IDENT).getText().equals(
// EQUALS_METHOD_NAME);
// }
//
// public static boolean isFinal(DetailAST detailAST) {
// DetailAST modifiersAST =
//detailAST.findFirstToken(TokenTypes.MODIFIERS);
//
// return modifiersAST.findFirstToken(TokenTypes.FINAL) != null;
// }
//
// public static boolean isInObjBlock(DetailAST detailAST) {
// return detailAST.getParent().getType() == TokenTypes.OBJBLOCK;
// }
//
// public static String getIdentText(DetailAST detailAST) {
// return detailAST.findFirstToken(TokenTypes.IDENT).getText();
// }
//
// private static FullIdent createFullTypeNoArrays(DetailAST typeAST) {
// return FullIdent.createFullIdent((DetailAST) typeAST.getFirstChild());
// }
/**
* Returns whether a token represents an ELSE as part of an ELSE / IF set.
* @param aAST the token to check
* @return whether it is
*/
public static boolean isElseIf(DetailAST aAST)
{
final DetailAST parentAST = aAST.getParent();
return (aAST.getType() == TokenTypes.LITERAL_IF)
&& (isElse(parentAST) || isElseWithCurlyBraces(parentAST));
}
/**
* Returns whether a token represents an ELSE.
* @param aAST the token to check
* @return whether the token represents an ELSE
*/
private static boolean isElse(DetailAST aAST)
{
return aAST.getType() == TokenTypes.LITERAL_ELSE;
}
/**
* Returns whether a token represents an SLIST as part of an ELSE
* statement.
* @param aAST the token to check
* @return whether the toke does represent an SLIST as part of an ELSE
*/
private static boolean isElseWithCurlyBraces(DetailAST aAST)
{
return (aAST.getType() == TokenTypes.SLIST)
&& (aAST.getChildCount() == 2)
&& isElse(aAST.getParent());
}
}

View File

@ -175,9 +175,8 @@ public class LeftCurlyCheck
* @param aBrace token for left curly brace
* @param aStartToken token for start of expression
*/
private void verifyBrace(
final DetailAST aBrace,
final DetailAST aStartToken)
private void verifyBrace(final DetailAST aBrace,
final DetailAST aStartToken)
{
final String braceLine = getLines()[aBrace.getLineNo() - 1];

View File

@ -31,7 +31,7 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST;
* </p>
*
* @author lkuehne
* @version $Revision: 1.2 $
* @version $Revision: 1.3 $
*/
public class HideUtilityClassConstructorCheck extends Check
{
@ -76,7 +76,7 @@ public class HideUtilityClassConstructorCheck extends Check
child = (DetailAST) child.getNextSibling();
}
boolean hasAccessibleCtor = (hasDefaultCtor || hasPublicCtor);
final boolean hasAccessibleCtor = (hasDefaultCtor || hasPublicCtor);
if (hasMethod && !hasNonStaticMethod && hasAccessibleCtor) {
log(aAST.getLineNo(), aAST.getColumnNo(),

View File

@ -131,8 +131,8 @@ public class VisibilityModifierCheck
/** @see Check */
public void visitToken(DetailAST aAST)
{
if (aAST.getType() != TokenTypes.VARIABLE_DEF
|| aAST.getParent().getType() != TokenTypes.OBJBLOCK)
if ((aAST.getType() != TokenTypes.VARIABLE_DEF)
|| (aAST.getParent().getType() != TokenTypes.OBJBLOCK))
{
return;
}

View File

@ -147,18 +147,18 @@ public class SlistHandler extends BlockParentHandler
*/
private boolean hasBlockParent()
{
int parentType = getMainAst().getParent().getType();
return parentType == TokenTypes.LITERAL_IF
|| parentType == TokenTypes.LITERAL_FOR
|| parentType == TokenTypes.LITERAL_WHILE
|| parentType == TokenTypes.LITERAL_DO
|| parentType == TokenTypes.LITERAL_ELSE
|| parentType == TokenTypes.LITERAL_TRY
|| parentType == TokenTypes.LITERAL_CATCH
|| parentType == TokenTypes.LITERAL_FINALLY
|| parentType == TokenTypes.CTOR_DEF
|| parentType == TokenTypes.METHOD_DEF
|| parentType == TokenTypes.STATIC_INIT;
final int parentType = getMainAst().getParent().getType();
return (parentType == TokenTypes.LITERAL_IF)
|| (parentType == TokenTypes.LITERAL_FOR)
|| (parentType == TokenTypes.LITERAL_WHILE)
|| (parentType == TokenTypes.LITERAL_DO)
|| (parentType == TokenTypes.LITERAL_ELSE)
|| (parentType == TokenTypes.LITERAL_TRY)
|| (parentType == TokenTypes.LITERAL_CATCH)
|| (parentType == TokenTypes.LITERAL_FINALLY)
|| (parentType == TokenTypes.CTOR_DEF)
|| (parentType == TokenTypes.METHOD_DEF)
|| (parentType == TokenTypes.STATIC_INIT);
}
/**

View File

@ -0,0 +1,203 @@
////////////////////////////////////////////////////////////////////////////////
// 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.metrics;
import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import java.util.Stack;
/**
* Base class for checks the calculate complexity based around methods.
*
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
* @author Oliver Burn
*/
public abstract class AbstractComplexityCheck
extends Check
{
/** the initial current value */
private static final int INITIAL_VALUE = 1;
/** stack of values - all but the current value */
private final Stack mValueStack = new Stack();
/** the current value */
private int mCurrentValue;
/** threshold to report error for */
private int mMax;
/**
* Creates an instance.
* @param aMax the threshold of when to report an error
*/
public AbstractComplexityCheck(int aMax)
{
mMax = aMax;
}
/**
* @return the message ID to log violations with
*/
protected abstract String getMessageID();
/**
* Hook called when visiting a token. Will not be called the method
* definition tokens.
*
* @param aAST the token being visited
*/
protected void visitTokenHook(DetailAST aAST)
{
}
/**
* Hook called when leaving a token. Will not be called the method
* definition tokens.
*
* @param aAST the token being left
*/
protected void leaveTokenHook(DetailAST aAST)
{
}
/** {@inheritDoc} */
public final int[] getRequiredTokens()
{
return new int[] {
TokenTypes.CTOR_DEF,
TokenTypes.METHOD_DEF,
TokenTypes.STATIC_INIT,
};
}
/** @return the maximum threshold allowed */
public final int getMax()
{
return mMax;
}
/**
* Set the maximum threshold allowed.
*
* @param aMax the maximum threshold
*/
public final void setMax(int aMax)
{
mMax = aMax;
}
/** {@inheritDoc} */
public final void visitToken(DetailAST aAST)
{
switch (aAST.getType()) {
case TokenTypes.CTOR_DEF:
case TokenTypes.METHOD_DEF:
case TokenTypes.STATIC_INIT:
visitMethodDef();
break;
default:
visitTokenHook(aAST);
}
}
/** {@inheritDoc} */
public final void leaveToken(DetailAST aAST)
{
switch (aAST.getType()) {
case TokenTypes.CTOR_DEF:
case TokenTypes.METHOD_DEF:
case TokenTypes.STATIC_INIT:
leaveMethodDef(aAST);
break;
default:
leaveTokenHook(aAST);
}
}
/**
* @return the current value
*/
protected final int getCurrentValue()
{
return mCurrentValue;
}
/**
* Set the current value
* @param aValue the new value
*/
protected final void setCurrentValue(int aValue)
{
mCurrentValue = aValue;
}
/**
* Increments the current value by a specified amount.
*
* @param aBy the amount to increment by
*/
protected final void incrementCurrentValue(int aBy)
{
setCurrentValue(getCurrentValue() + aBy);
}
/** Push the current value on the stack */
protected final void pushValue()
{
mValueStack.push(new Integer(mCurrentValue));
mCurrentValue = INITIAL_VALUE;
}
/**
* @return pop a value off the stack and make it the current value
*/
protected final int popValue()
{
mCurrentValue = ((Integer) mValueStack.pop()).intValue();
return mCurrentValue;
}
/** Process the start of the method definition */
private void visitMethodDef()
{
pushValue();
}
/**
* Process the end of a method definition.
*
* @param aAST the token representing the method definition
*/
private void leaveMethodDef(DetailAST aAST)
{
if (mCurrentValue > mMax) {
log(aAST.getLineNo(),
aAST.getColumnNo(),
getMessageID(),
new Integer(mCurrentValue),
new Integer(mMax));
}
popValue();
}
}

View File

@ -0,0 +1,83 @@
////////////////////////////////////////////////////////////////////////////////
// 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.metrics;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.CheckUtils;
/**
* Checks cyclomatic complexity against a specified limit. The complexity is
* measured by the number of "if", "while", "do", "for", "?:", "catch",
* "switch", "case", "&&" and "||" statements (plus one) in the body of the
* member. It is a measure of the minimum number of possible paths through the
* source and therefore the number of required tests tests. Generally 1-4 is
* considered good, 5-7 ok, 8-10 consider re-factoring, and 11+ re-factor now!
*
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
* @author Oliver Burn
*/
public class CyclomaticComplexityCheck
extends AbstractComplexityCheck
{
/** default allowed complexity */
private static final int DEFAULT_VALUE = 10;
/** Create an instance */
public CyclomaticComplexityCheck()
{
super(DEFAULT_VALUE);
}
/** {@inheritDoc} */
public int[] getDefaultTokens()
{
return new int[] {
TokenTypes.CTOR_DEF,
TokenTypes.METHOD_DEF,
TokenTypes.STATIC_INIT,
TokenTypes.LITERAL_WHILE,
TokenTypes.LITERAL_DO,
TokenTypes.LITERAL_FOR,
TokenTypes.LITERAL_IF,
TokenTypes.LITERAL_ELSE,
TokenTypes.LITERAL_SWITCH,
TokenTypes.LITERAL_CASE,
TokenTypes.LITERAL_TRY,
TokenTypes.LITERAL_CATCH,
TokenTypes.QUESTION,
TokenTypes.LAND,
TokenTypes.LOR,
};
}
/** {@inheritDoc} */
protected final void visitTokenHook(DetailAST aAST)
{
if (!CheckUtils.isElseIf(aAST)) {
incrementCurrentValue(1);
}
}
/** {@inheritDoc} */
protected final String getMessageID()
{
return "cyclomaticComplexity";
}
}

View File

@ -0,0 +1,30 @@
# $Id: messages.properties,v 1.1 2003-06-24 12:35:39 oburn Exp $
booleanExpressionComplexity=Boolean expression complexity is {0,number,integer} (max allowed is {1,number,integer}).
classDataAbstractionCoupling=Class Data Abstraction Coupling is {0,number,integer} (max allowed is {1,number,integer}).
classFanOutComplexity=Class Fan-Out Complexity is {0,number,integer} (max allowed is {1,number,integer}).
cyclomaticComplexity=Cyclomatic Complexity is {0,number,integer} (max allowed is {1,number,integer}).
duplicateLiteral=Duplicate instances of literal ''{0}'' are not allowed.
executableStatementCount=Executable statement count is {0,number,integer} (max allowed is {1,number,integer}).
finalField=The field ''{0}'' should be declared final.
illegalAbstractClassName=Name ''{0}'' must match pattern ''{1}''.
illegalCatch=Catching ''{0}'' is not allowed.
illegalThrows=Throwing ''{0}'' is not allowed.
illegalToken=Using ''{0}'' is not allowed.
illegalType=Declaring variables, return values or parameters of type ''{0}'' is not allowed.
mutableException=The field ''{0}'' must be declared final.
nestedIfDepth=Nested if-else depth is {0,number,integer} (max allowed is {1,number,integer}).
nestedTryDepth=Nested try depth is {0,number,integer} (max allowed is {1,number,integer}).
npathComplexity=NPath Complexity is {0,number,integer} (max allowed is {1,number,integer}).
packageDeclaration=Missing package declaration.
parameterAssignment=Assignment of parameter ''{0}'' is not allowed.
returnFromCatch=Return from catch is not allowed.
returnFromFinally=Return from finally is not allowed.
returnCount=Return count is {0,number,integer} (max allowed is {1,number,integer}).
throwsCount=Throws count is {0,number,integer} (max allowed is {1,number,integer}).
unusedVariable=Variable ''{0}'' is never used.
junit.methodName=The method ''{0}'' should be named ''{1}''
junit.methodPublicOrProtected=The method {0} must be declared public or protected
junit.methodReturnType=The method ''{0}'' must de declared with a void return type.
junit.methodParameters=The method ''{0}'' must be declared with no parameters.
junit.methodPublicAndStatic=The method ''{0}'' must be declared static.

View File

@ -0,0 +1,10 @@
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head></head>
<body>
<p>Contains the Metrics checks that are bundled with the main distribution.</p>
</body>
</html>

View File

@ -13,6 +13,7 @@
<package name="imports"/>
<package name="indentation"/>
<package name="javadoc"/>
<package name="metrics"/>
<package name="naming"/>
<package name="sizes"/>
<package name="whitespace"/>

View File

@ -0,0 +1,47 @@
package au.com.redhillconsulting.jamaica.tools.checkstyle;
public class ComplexityCheckTestInput {
public void foo() {
while (true) {
Runnable runnable = new Runnable() {
public void run() {
while (true) {
}
}
};
new Thread(runnable).start();
}
}
public void bar() {
if (System.currentTimeMillis() == 0) {
if (System.currentTimeMillis() == 0 && System.currentTimeMillis() == 0) {
}
if (System.currentTimeMillis() == 0 || System.currentTimeMillis() == 0) {
}
}
}
public void simpleElseIf() {
if (System.currentTimeMillis() == 0) {
} else if (System.currentTimeMillis() == 0) {
} else {
}
}
public void stupidElseIf() {
if (System.currentTimeMillis() == 0) {
} else {
if (System.currentTimeMillis() == 0) {
} else {
if (System.currentTimeMillis() == 0) {
}
}
if (System.currentTimeMillis() == 0) {
}
}
}
}

View File

@ -46,6 +46,7 @@ import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheckTest;
import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheckTest;
import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCheckTest;
import com.puppycrawl.tools.checkstyle.checks.javadoc.PackageHtmlCheckTest;
import com.puppycrawl.tools.checkstyle.checks.metrics.CyclomaticComplexityCheckTest;
import com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheckTest;
import com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableNameCheckTest;
import com.puppycrawl.tools.checkstyle.checks.naming.LocalVariableNameCheckTest;
@ -97,6 +98,7 @@ public class AllTests {
suite.addTest(new TestSuite(ArrayTrailingCommaCheckTest.class));
suite.addTest(new TestSuite(ConfigurationLoaderTest.class));
suite.addTest(new TestSuite(ConstantNameCheckTest.class));
suite.addTest(new TestSuite(CyclomaticComplexityCheckTest.class));
suite.addTest(new TestSuite(DesignForExtensionCheckTest.class));
suite.addTest(new TestSuite(DoubleCheckedLockingCheckTest.class));
suite.addTest(new TestSuite(EmptyBlockCheckTest.class));

View File

@ -58,6 +58,7 @@ public class PackageNamesLoaderTest extends TestCase
"com.puppycrawl.tools.checkstyle.checks.imports.",
"com.puppycrawl.tools.checkstyle.checks.indentation.",
"com.puppycrawl.tools.checkstyle.checks.javadoc.",
"com.puppycrawl.tools.checkstyle.checks.metrics.",
"com.puppycrawl.tools.checkstyle.checks.naming.",
"com.puppycrawl.tools.checkstyle.checks.sizes.",
"com.puppycrawl.tools.checkstyle.checks.whitespace."

View File

@ -0,0 +1,26 @@
package com.puppycrawl.tools.checkstyle.checks.metrics;
import com.puppycrawl.tools.checkstyle.BaseCheckTestCase;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
public class CyclomaticComplexityCheckTest
extends BaseCheckTestCase
{
public void test() throws Exception
{
final DefaultConfiguration checkConfig =
createCheckConfig(CyclomaticComplexityCheck.class);
checkConfig.addAttribute("max", "0");
final String[] expected = {
"4:5: Cyclomatic Complexity is 2 (max allowed is 0).",
"7:17: Cyclomatic Complexity is 2 (max allowed is 0).",
"17:5: Cyclomatic Complexity is 6 (max allowed is 0).",
"27:5: Cyclomatic Complexity is 4 (max allowed is 0).",
"34:5: Cyclomatic Complexity is 6 (max allowed is 0).",
};
verify(checkConfig, getPath("ComplexityCheckTestInput.java"), expected);
}
}