Small upgrade to CyclomaticComplexity. By default, the rule add two kind of violation (method violation, but also a class average complexity). Some user find that troublesome and it maybe messy if an aggregated report. So i had a simple configuration to the rule. I also had a couple of unit test to ensure proper behavior of this modification.

Code cleaning, switching inheritance to AbstractJavaRule.

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@5773 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Romain Pelisse
2008-02-14 09:06:49 +00:00
parent 17546aaa70
commit dd4b2b9112
4 changed files with 116 additions and 73 deletions

View File

@ -14,6 +14,8 @@ New rules:
StrictExceptions : DoNotThrowExceptionInFinally
Strings : AvoidStringBufferField
Rule upgrade:
CyclomaticComplexity now can be configure to display only classes average complexity or methods complexity, or both.
1.7 added as a valid option for targetjdk.
New elements under <ruleset>: <exclude-pattern> to match files exclude from processing, with <include-pattern> to override.

View File

@ -11,12 +11,8 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Complicated method
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
<code-fragment id="basic-violation">
<![CDATA[
public class Foo {
public void example() {
int x = 0;
@ -70,7 +66,14 @@ public class Foo {
}
}
}
]]></code>
]]>
</code-fragment>
<test-code>
<description><![CDATA[
Complicated method
]]></description>
<expected-problems>2</expected-problems>
<code-ref id="basic-violation"/>
</test-code>
<test-code>
<description><![CDATA[
@ -83,4 +86,25 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>
<![CDATA[
Testing new parameter showClassMethods
]]>
</description>
<rule-property name="showClassesComplexity">false</rule-property>
<expected-problems>1</expected-problems>
<code-ref id="basic-violation"/>
</test-code>
<test-code>
<description>
<![CDATA[
Testing new parameter showMethodsMethods
]]>
</description>
<rule-property name="showMethodsComplexity">false</rule-property>
<expected-problems>1</expected-problems>
<code-ref id="basic-violation"/>
</test-code>
</test-data>

View File

@ -112,19 +112,28 @@ public class Foo {
</rule>
<rule name="CyclomaticComplexity"
message = "The {0} ''{1}'' has a Cyclomatic Complexity of {2}."
class="net.sourceforge.pmd.rules.CyclomaticComplexity"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#CyclomaticComplexity">
<rule name="CyclomaticComplexity"
message = "The {0} ''{1}'' has a Cyclomatic Complexity of {2}."
class="net.sourceforge.pmd.rules.CyclomaticComplexity"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#CyclomaticComplexity">
<description>
<![CDATA[
Complexity is determined by the number of decision points in a method plus one for the
method entry. The decision points are 'if', 'while', 'for', and 'case labels'. Generally,
1-4 is low complexity, 5-7 indicates moderate complexity, 8-10 is high complexity,
and 11+ is very high complexity.
]]>
</description>
<priority>3</priority>
<properties>
<property name="reportLevel" description="The Cyclomatic Complexity reporting threshold" value="10"/>
<property name="showClassesComplexity"
description="The Cyclomatic Complexity reporting threshold"
value="true"/>
<property name="showMethodsComplexity"
description="The Cyclomatic Complexity reporting threshold"
value="true"/>
</properties>
<example>
<![CDATA[
@ -227,7 +236,7 @@ public class Person {
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssMethodCount">
<description>
This rule uses the NCSS (Non Commenting Source Statements) algorithm to determine the number of lines
of code for a given method. NCSS ignores comments, and counts actual statements. Using this algorithm,
of code for a given method. NCSS ignores comments, and counts actual statements. Using this algorithm,
lines of code that are split are counted as one.
</description>
<priority>3</priority>
@ -239,11 +248,11 @@ lines of code that are split are counted as one.
public class Foo extends Bar {
public int methd() {
super.methd();
//this method only has 1 NCSS lines
return 1;
}
@ -253,11 +262,11 @@ public class Foo extends Bar {
</rule>
<rule name="NcssTypeCount" message="The type has an NCSS line count of {0}"
class="net.sourceforge.pmd.rules.codesize.NcssTypeCount"
class="net.sourceforge.pmd.rules.codesize.NcssTypeCount"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssTypeCount">
<description>
This rule uses the NCSS (Non Commenting Source Statements) algorithm to determine the number of lines
of code for a given type. NCSS ignores comments, and counts actual statements. Using this algorithm,
of code for a given type. NCSS ignores comments, and counts actual statements. Using this algorithm,
lines of code that are split are counted as one.
</description>
<priority>3</priority>
@ -270,11 +279,11 @@ public class Foo extends Bar {
public Foo() {
//this class only has 6 NCSS lines
super();
super.foo();
}
}
@ -282,11 +291,11 @@ public class Foo extends Bar {
</example></rule>
<rule name="NcssConstructorCount" message="The constructor with {0} parameters has an NCSS line count of {1}"
class="net.sourceforge.pmd.rules.codesize.NcssConstructorCount"
class="net.sourceforge.pmd.rules.codesize.NcssConstructorCount"
externalInfoUrl="http://pmd.sourceforge.net/rules/codesize.html#NcssConstructorCount">
<description>
This rule uses the NCSS (Non Commenting Source Statements) algorithm to determine the number of lines
of code for a given constructor. NCSS ignores comments, and counts actual statements. Using this algorithm,
of code for a given constructor. NCSS ignores comments, and counts actual statements. Using this algorithm,
lines of code that are split are counted as one.
</description>
<priority>3</priority>
@ -298,11 +307,11 @@ lines of code that are split are counted as one.
public class Foo extends Bar {
public Foo() {
super();
//this constructor only has 1 NCSS lines
super.foo();
}
@ -327,16 +336,16 @@ have more fine grained objects.
<property name="maxmethods" description="The method count reporting threshold " value="10"/>
<property name="xpath">
<value>
<!-- FIXME: Refine XPath to discard 'get' and 'set' methods with Block no more than 3 lines,
<!-- FIXME: Refine XPath to discard 'get' and 'set' methods with Block no more than 3 lines,
something like this:
not (
(
starts-with(@Image,'get')
or
starts-with(@Image,'set')
)
starts-with(@Image,'set')
)
and ( (
(../Block/attribute::endLine)
(../Block/attribute::endLine)
-
(../Block/attribute::beginLine) ) <= 3 )
)
@ -346,12 +355,12 @@ have more fine grained objects.
//ClassOrInterfaceDeclaration/ClassOrInterfaceBody
[
count(descendant::MethodDeclarator[
not
not
(
starts-with(@Image,'get')
or
starts-with(@Image,'set')
)
starts-with(@Image,'set')
)
]) > $maxmethods
]
]]>

View File

@ -5,7 +5,7 @@ package net.sourceforge.pmd.rules;
import java.util.Stack;
import net.sourceforge.pmd.AbstractRule;
import net.sourceforge.pmd.AbstractJavaRule;
import net.sourceforge.pmd.ast.ASTBlockStatement;
import net.sourceforge.pmd.ast.ASTCatchStatement;
import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
@ -27,13 +27,16 @@ import net.sourceforge.pmd.ast.SimpleNode;
import net.sourceforge.pmd.rules.design.NpathComplexity;
/**
* @author Donald A. Leckie
* @author Donald A. Leckie,
*
* @version $Revision$, $Date$
* @since January 14, 2003
*/
public class CyclomaticComplexity extends AbstractRule {
public class CyclomaticComplexity extends AbstractJavaRule {
private int reportLevel;
private boolean showClassesComplexity = true;
private boolean showMethodsComplexity = true;
private static class Entry {
private SimpleNode node;
@ -62,7 +65,9 @@ public class CyclomaticComplexity extends AbstractRule {
private Stack<Entry> entryStack = new Stack<Entry>();
public Object visit(ASTCompilationUnit node, Object data) {
reportLevel = getIntProperty( "reportLevel" );
reportLevel = getIntProperty("reportLevel" );
showClassesComplexity = getBooleanProperty("showClassesComplexity");
showMethodsComplexity = getBooleanProperty("showMethodsComplexity");
super.visit( node, data );
return data;
}
@ -157,14 +162,16 @@ public class CyclomaticComplexity extends AbstractRule {
entryStack.push( new Entry( node ) );
super.visit( node, data );
Entry classEntry = entryStack.pop();
if ( ( classEntry.getComplexityAverage() >= reportLevel )
|| ( classEntry.highestDecisionPoints >= reportLevel ) ) {
addViolation( data, node, new String[] {
"class",
node.getImage(),
classEntry.getComplexityAverage() + " (Highest = "
+ classEntry.highestDecisionPoints + ')' } );
if ( showClassesComplexity ) {
Entry classEntry = entryStack.pop();
if ( ( classEntry.getComplexityAverage() >= reportLevel )
|| ( classEntry.highestDecisionPoints >= reportLevel ) ) {
addViolation( data, node, new String[] {
"class",
node.getImage(),
classEntry.getComplexityAverage() + " (Highest = "
+ classEntry.highestDecisionPoints + ')' } );
}
}
return data;
}
@ -172,31 +179,32 @@ public class CyclomaticComplexity extends AbstractRule {
public Object visit(ASTMethodDeclaration node, Object data) {
entryStack.push( new Entry( node ) );
super.visit( node, data );
Entry methodEntry = entryStack.pop();
int methodDecisionPoints = methodEntry.decisionPoints;
Entry classEntry = entryStack.peek();
classEntry.methodCount++;
classEntry.bumpDecisionPoints( methodDecisionPoints );
if ( showMethodsComplexity ) {
Entry methodEntry = entryStack.pop();
int methodDecisionPoints = methodEntry.decisionPoints;
Entry classEntry = entryStack.peek();
classEntry.methodCount++;
classEntry.bumpDecisionPoints( methodDecisionPoints );
if ( methodDecisionPoints > classEntry.highestDecisionPoints ) {
classEntry.highestDecisionPoints = methodDecisionPoints;
if ( methodDecisionPoints > classEntry.highestDecisionPoints ) {
classEntry.highestDecisionPoints = methodDecisionPoints;
}
ASTMethodDeclarator methodDeclarator = null;
for ( int n = 0; n < node.jjtGetNumChildren(); n++ ) {
Node childNode = node.jjtGetChild( n );
if ( childNode instanceof ASTMethodDeclarator ) {
methodDeclarator = (ASTMethodDeclarator) childNode;
break;
}
}
if ( methodEntry.decisionPoints >= reportLevel ) {
addViolation( data, node, new String[] { "method",
( methodDeclarator == null ) ? "" : methodDeclarator.getImage(),
String.valueOf( methodEntry.decisionPoints ) } );
}
}
ASTMethodDeclarator methodDeclarator = null;
for ( int n = 0; n < node.jjtGetNumChildren(); n++ ) {
Node childNode = node.jjtGetChild( n );
if ( childNode instanceof ASTMethodDeclarator ) {
methodDeclarator = (ASTMethodDeclarator) childNode;
break;
}
}
if ( methodEntry.decisionPoints >= reportLevel ) {
addViolation( data, node, new String[] { "method",
( methodDeclarator == null ) ? "" : methodDeclarator.getImage(),
String.valueOf( methodEntry.decisionPoints ) } );
}
return data;
}