Skip to content

added calculation for max cyclomatic complexity of class methods and use it for reporting and violations calculation #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public function leaveNode(Node $node)
$class = $this->metrics->get($node->namespacedName->toString());

$ccn = 1;
$ccnByMethod = array();

foreach ($node->stmts as $stmt) {
if ($stmt instanceof Stmt\ClassMethod) {
Expand Down Expand Up @@ -88,11 +89,19 @@ public function leaveNode(Node $node)
return $ccn;
};

$ccn += $cb($stmt);
$methodCcn = $cb($stmt);

$ccn += $methodCcn;
$ccnByMethod[] = $methodCcn + 1; // each method by default is CCN 1 even if it's empty
}
}

$class->set('ccn', $ccn);

$class->set('ccnMethodMax', 0);
if (count($ccnByMethod)) {
$class->set('ccnMethodMax', max($ccnByMethod));
}
}
}
}
4 changes: 3 additions & 1 deletion src/Hal/Report/Html/template/complexity.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
<thead>
<tr>
<th>Class</th>
<th class="js-sort-number">Cyclomatic</th>
<th class="js-sort-number">Class cycl.</th>
<th class="js-sort-number">Max method cycl.</th>
<?php if ($config->has('junit')) { ?>
<th class="js-sort-number">Unit testsuites calling it</th>
<?php } ?><th class="js-sort-number">Relative system complexity</th>
Expand All @@ -65,6 +66,7 @@
<tr>
<td><?php echo $class['name']; ?></td>
<td><?php echo isset($class['ccn']) ? $class['ccn'] : ''; ?></td>
<td><?php echo isset($class['ccnMethodMax']) ? $class['ccnMethodMax'] : ''; ?></td>
<?php if ($config->has('junit')) { ?>
<td><?php echo isset($class['numberOfUnitTests']) ? $class['numberOfUnitTests'] : ''; ?></td>
<?php } ?>
Expand Down
4 changes: 3 additions & 1 deletion src/Hal/Report/Html/template/oop.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@
<th>Class</th>
<th class="js-sort-number">LCOM</th>
<th class="js-sort-number">Volume</th>
<th class="js-sort-number">Cyclomatic</th>
<th class="js-sort-number">Class cycl.</th>
<th class="js-sort-number">Max method cycl.</th>
<th class="js-sort-number">Bugs</th>
<th class="js-sort-number">Difficulty</th>
</tr>
Expand All @@ -77,6 +78,7 @@
<td><?php echo isset($class['lcom']) ? $class['lcom'] : ''; ?></td>
<td><?php echo isset($class['volume']) ? $class['volume'] : ''; ?></td>
<td><?php echo isset($class['ccn']) ? $class['ccn'] : ''; ?></td>
<td><?php echo isset($class['ccnMethodMax']) ? $class['ccnMethodMax'] : ''; ?></td>
<td><?php echo isset($class['bugs']) ? $class['bugs'] : ''; ?></td>
<td><?php echo isset($class['difficulty']) ? $class['difficulty'] : ''; ?></td>
</tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
use Hal\Metric\Metric;
use Hal\Violation\Violation;

class TooComplexCode implements Violation
class TooComplexClassCode implements Violation
{

/**
* @inheritdoc
*/
public function getName()
{
return 'Too complex code';
return 'Too complex class code';
}

/**
Expand Down Expand Up @@ -51,10 +51,10 @@ public function getDescription()
return <<<EOT
This class looks really complex.

* Algorithm are complex (Cyclomatic complexity is {$this->metric->get('ccn')})
* Algorithms are complex (Total cyclomatic complexity of class is {$this->metric->get('ccn')})
* Component uses {$this->metric->get('number_operators')} operators

Maybe you should delegate some code to another objects.
Maybe you should delegate some code to other objects.
EOT;

}
Expand Down
60 changes: 60 additions & 0 deletions src/Hal/Violation/Class_/TooComplexMethodCode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php
namespace Hal\Violation\Class_;


use Hal\Metric\ClassMetric;
use Hal\Metric\Metric;
use Hal\Violation\Violation;

class TooComplexMethodCode implements Violation
{

/**
* @inheritdoc
*/
public function getName()
{
return 'Too complex method code';
}

/**
* @inheritdoc
*/
public function apply(Metric $metric)
{
if (!$metric instanceof ClassMetric) {
return;
}

$this->metric = $metric;

if ($metric->get('ccnMethodMax') >= 8) {
$metric->get('violations')->add($this);
return;
}

}

/**
* @inheritdoc
*/
public function getLevel()
{
return Violation::ERROR;
}

/**
* @inheritdoc
*/
public function getDescription()
{
return <<<EOT
This class looks really complex.

* Algorithms are complex (Max cyclomatic complexity of class methods is {$this->metric->get('ccnMethodMax')})

Maybe you should delegate some code to other objects or split complex method.
EOT;

}
}
3 changes: 2 additions & 1 deletion src/Hal/Violation/ViolationParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ public function apply(Metrics $metrics)

$violations = [
new Class_\Blob(),
new Class_\TooComplexCode(),
new Class_\TooComplexClassCode(),
new Class_\TooComplexMethodCode(),
new Class_\ProbablyBugged(),
new Class_\TooLong(),
new Class_\TooDependent(),
Expand Down
38 changes: 33 additions & 5 deletions tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ class CyclomaticComplexityVisitorTest extends \PHPUnit_Framework_TestCase {


/**
* @dataProvider provideExamples
* @dataProvider provideExamplesForClasses
*/
public function testLackOfCohesionOfMethodsIsWellCalculated($example, $classname, $expected)
public function testCyclomaticComplexityOfClassesIsWellCalculated($example, $classname, $expectedCcn)
{
$metrics = new Metrics();

Expand All @@ -27,14 +27,42 @@ public function testLackOfCohesionOfMethodsIsWellCalculated($example, $classname
$stmts = $parser->parse($code);
$traverser->traverse($stmts);

$this->assertSame($expected, $metrics->get($classname)->get('ccn'));
$this->assertSame($expectedCcn, $metrics->get($classname)->get('ccn'));
}

public function provideExamples()
/**
* @dataProvider provideExamplesForMethods
*/
public function testCyclomaticComplexityOfMethodsIsWellCalculated($example, $classname, $expectedCcnMethodMax)
{
$metrics = new Metrics();

$parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7);
$traverser = new \PhpParser\NodeTraverser();
$traverser->addVisitor(new \PhpParser\NodeVisitor\NameResolver());
$traverser->addVisitor(new ClassEnumVisitor($metrics));
$traverser->addVisitor(new CyclomaticComplexityVisitor($metrics));

$code = file_get_contents($example);
$stmts = $parser->parse($code);
$traverser->traverse($stmts);

$this->assertSame($expectedCcnMethodMax, $metrics->get($classname)->get('ccnMethodMax'));
}

public function provideExamplesForClasses()
{
return [
[ __DIR__.'/../../examples/cyclomatic1.php', 'A', 4],
[ __DIR__.'/../../examples/cyclomatic1.php', 'B', 5],
];
}

public function provideExamplesForMethods()
{
return [
[ __DIR__.'/../../examples/cyclomatic1.php', 'A', 3],
[ __DIR__.'/../../examples/cyclomatic1.php', 'B', 3],
[ __DIR__.'/../../examples/cyclomatic1.php', 'B', 5],
];
}

Expand Down
19 changes: 16 additions & 3 deletions tests/Metric/examples/cyclomatic1.php
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
<?php
class A {
public function foo()
public function foo1()
{
if(true) {
if(false) {

}
}
}

public function foo2()
{
if(true) {

}
}

public function foo3()
{
}
}

class B {
Expand All @@ -17,8 +28,10 @@ public function foo()

}

if(false) {

foreach(array() as $foo) {
if(false) {
continue;
}
}
}
}