phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pmd: fix #984 Cyclomatic complexity should treat constructors like methods

Browse Source
This commit is contained in:
Andreas Dangel 2013-03-16 17:15:40 +01:00
parent 13ef401a63
commit cece698b04
4 changed files with 60 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pmd/etc/changelog.txt
View File

@ -1,5 +1,6 @@
????? ??, 2013 - 5.0.3:
Fixed bug 984: Cyclomatic complexity should treat constructors like methods
Fixed bug 992: Class java.beans.Statement triggered in CloseResource rule
Fixed bug 997: Rule NonThreadSafeSingleton gives analysis problem
Fixed bug 1002: False +: FinalFieldCouldBeStatic on inner class

7
pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/codesize/CyclomaticComplexityRule.java
View File

@ -204,7 +204,6 @@ public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
public Object visit(ASTMethodDeclaration node, Object data) {
entryStack.push( new Entry( node ) );
super.visit( node, data );
if ( showMethodsComplexity ) {
Entry methodEntry = entryStack.pop();
int methodDecisionPoints = methodEntry.decisionPoints;
Entry classEntry = entryStack.peek();
@ -224,12 +223,11 @@ public Object visit(ASTMethodDeclaration node, Object data) {
}
}
if ( methodEntry.decisionPoints >= reportLevel ) {
if ( showMethodsComplexity && methodEntry.decisionPoints >= reportLevel ) {
addViolation( data, node, new String[] { "method",
methodDeclarator == null ? "" : methodDeclarator.getImage(),
String.valueOf( methodEntry.decisionPoints ) } );
}
}
return data;
}
@ -261,12 +259,11 @@ public Object visit(ASTConstructorDeclaration node, Object data) {
if ( constructorDecisionPointCount > classEntry.highestDecisionPoints ) {
classEntry.highestDecisionPoints = constructorDecisionPointCount;
}
if ( constructorEntry.decisionPoints >= reportLevel ) {
if ( showMethodsComplexity && constructorEntry.decisionPoints >= reportLevel ) {
addViolation( data, node, new String[] { "constructor",
classEntry.node.getImage(),
String.valueOf( constructorDecisionPointCount ) } );
}
return data;
}
}

33
pmd/src/test/java/net/sourceforge/pmd/lang/java/rule/codesize/CyclomaticComplexityTest.java
View File

@ -13,12 +13,17 @@ import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleViolation;
import net.sourceforge.pmd.lang.java.rule.codesize.CyclomaticComplexityRule;
import net.sourceforge.pmd.testframework.RuleTst;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst;
import net.sourceforge.pmd.testframework.TestDescriptor;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst.CustomXmlTestClassMethodsRunner;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runner.notification.Failure;
@RunWith(SimpleAggregatorTst.CustomXmlTestClassMethodsRunner.class)
public class CyclomaticComplexityTest extends RuleTst {
private Rule rule;
private TestDescriptor[] tests;
@ -33,8 +38,8 @@ import org.junit.Test;
rule.setProperty(CyclomaticComplexityRule.REPORT_LEVEL_DESCRIPTOR, 1);
Report report = new Report();
runTestFromString(tests[0].getCode(), rule, report);
Iterator i = report.iterator();
RuleViolation rv = (RuleViolation) i.next();
Iterator<RuleViolation> i = report.iterator();
RuleViolation rv = i.next();
assertNotSame(rv.getDescription().indexOf("Highest = 1"), -1);
}
@ -43,8 +48,8 @@ import org.junit.Test;
rule.setProperty(CyclomaticComplexityRule.REPORT_LEVEL_DESCRIPTOR, 10);
Report report = new Report();
runTestFromString(tests[1].getCode(), rule, report);
Iterator i = report.iterator();
RuleViolation rv = (RuleViolation) i.next();
Iterator<RuleViolation> i = report.iterator();
RuleViolation rv = i.next();
assertNotSame(rv.getDescription().indexOf("Highest = 11"), -1);
}
@ -53,8 +58,8 @@ import org.junit.Test;
rule.setProperty(CyclomaticComplexityRule.REPORT_LEVEL_DESCRIPTOR, 1);
Report report = new Report();
runTestFromString(tests[2].getCode(), rule, report);
Iterator i = report.iterator();
RuleViolation rv = (RuleViolation) i.next();
Iterator<RuleViolation> i = report.iterator();
RuleViolation rv = i.next();
assertNotSame(rv.getDescription().indexOf("Highest = 1"), -1);
}
@ -66,6 +71,22 @@ import org.junit.Test;
assertEquals(0, report.size());
}
@Test
public void testRemainingTestCases() {
for (int i = 0; i < tests.length; i++) {
if (i == 0 || i == 1 || i == 2) {
continue; // skip - covered by above test methods
}
try {
runTest(tests[i]);
} catch (Throwable t) {
Failure f = CustomXmlTestClassMethodsRunner.createFailure(rule, t);
CustomXmlTestClassMethodsRunner.addFailure(f);
}
}
}
public static junit.framework.Test suite() {
return new junit.framework.JUnit4TestAdapter(CyclomaticComplexityTest.class);
}

31
pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/codesize/xml/CyclomaticComplexity.xml
View File

@ -112,9 +112,36 @@ Testing new parameter showMethodsMethods
Testing default value of showClassMethods and showClassesComplexity
]]>
</description>
<rule-property name="showClassesComplexity">""</rule-property>
<rule-property name="showMethodsComplexity">""</rule-property>
<expected-problems>2</expected-problems>
<code-ref id="basic-violation"/>
</test-code>
<code-fragment id="constructor-violation"><![CDATA[
public class Test {
public Test() {
if (a == 1) {
if (b == 2) {
System.out.println("b");
} else if (b == 1) {
}
} else {
}
}
}
]]></code-fragment>
<test-code>
<description>#984 Cyclomatic complexity should treat constructors like methods: 1 - showMethodsComplexity=true</description>
<rule-property name="showClassesComplexity">false</rule-property>
<rule-property name="showMethodsComplexity">true</rule-property>
<rule-property name="reportLevel">1</rule-property>
<expected-problems>1</expected-problems>
<code-ref id="constructor-violation"/>
</test-code>
<test-code>
<description>#984 Cyclomatic complexity should treat constructors like methods: 2 - showMethodsComplexity=false</description>
<rule-property name="showClassesComplexity">false</rule-property>
<rule-property name="showMethodsComplexity">false</rule-property>
<rule-property name="reportLevel">1</rule-property>
<expected-problems>0</expected-problems>
<code-ref id="constructor-violation"/>
</test-code>
</test-data>