From b422201d78da5b1469aa6aa3cbfa92f12678d53a Mon Sep 17 00:00:00 2001 From: Oskar Pfeifer-Bley Date: Thu, 24 May 2018 12:41:35 +0200 Subject: [PATCH 1/2] Change the calculation of cyclomatic complexity - xor has not been covered yet. - The ternary operator and the coalesce operator has been count both as 2. Also the default case of a switch statement has been counted. But as the else statement is not counted because it represents the default path (and can be elimated), the default case should not be counted (corresponds to else) and both the ternary and the coalesce operator should be counted as 1, because they can be exchanged by if-else / if which would be counted as 1. The spaceship operator by contrast is now counted as 2, because it would need if-elseif-else to replace it. - continue does not increase the cyclomatic complexity as its no decision. It's the if around the continue which increases the ccn. --- .../CyclomaticComplexityVisitor.php | 33 ++++----- .../CyclomaticComplexityVisitorTest.php | 13 ++-- tests/Metric/examples/cyclomatic_full.php | 72 +++++++++++++++++++ 3 files changed, 95 insertions(+), 23 deletions(-) create mode 100644 tests/Metric/examples/cyclomatic_full.php diff --git a/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php b/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php index 862d7186..f3b871bf 100644 --- a/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php +++ b/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php @@ -1,7 +1,6 @@ metrics = $metrics; } - /** - * @inheritdoc - */ public function leaveNode(Node $node) { if ($node instanceof Stmt\Class_ || $node instanceof Stmt\Interface_ || $node instanceof Stmt\Trait_ ) { - $class = $this->metrics->get(MetricClassNameGenerator::getName($node)); $ccn = 1; @@ -62,9 +52,9 @@ public function leaveNode(Node $node) $ccn = 0; foreach (get_object_vars($node) as $name => $member) { - foreach (is_array($member) ? $member : [$member] as $member_item) { - if ($member_item instanceof Node) { - $ccn += $cb($member_item); + foreach (is_array($member) ? $member : [$member] as $memberItem) { + if ($memberItem instanceof Node) { + $ccn += $cb($memberItem); } } } @@ -78,18 +68,23 @@ public function leaveNode(Node $node) case $node instanceof Stmt\Do_: case $node instanceof Node\Expr\BinaryOp\LogicalAnd: case $node instanceof Node\Expr\BinaryOp\LogicalOr: + case $node instanceof Node\Expr\BinaryOp\LogicalXor: case $node instanceof Node\Expr\BinaryOp\BooleanAnd: case $node instanceof Node\Expr\BinaryOp\BooleanOr: - case $node instanceof Node\Expr\BinaryOp\Spaceship: - case $node instanceof Stmt\Case_: // include default case $node instanceof Stmt\Catch_: - case $node instanceof Stmt\Continue_: - $ccn++; - break; case $node instanceof Node\Expr\Ternary: case $node instanceof Node\Expr\BinaryOp\Coalesce: - $ccn = $ccn + 2; + $ccn++; + break; + case $node instanceof Stmt\Case_: // include default + if ($node->cond !== null) { // exclude default + $ccn++; + } + break; + case $node instanceof Node\Expr\BinaryOp\Spaceship: + $ccn += 2; break; + } return $ccn; }; diff --git a/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php b/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php index 9c130b08..f97275bb 100644 --- a/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php +++ b/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php @@ -53,9 +53,14 @@ public function testCyclomaticComplexityOfMethodsIsWellCalculated($example, $cla public function provideExamplesForClasses() { return [ - [ __DIR__.'/../../examples/cyclomatic1.php', 'A', 8], - [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 5], - [ __DIR__.'/../../examples/cyclomatic_anon.php', 'Foo\C', 1], + 'A' => [ __DIR__.'/../../examples/cyclomatic1.php', 'A', 8], + 'B' => [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 4], + 'Foo\\C' => [ __DIR__.'/../../examples/cyclomatic_anon.php', 'Foo\\C', 1], + 'SwitchCase' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'SwitchCase', 4], + 'IfElseif' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'IfElseif', 7], + 'Loops' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'Loops', 5], + 'CatchIt' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'CatchIt', 3], + 'Logical' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'Logical', 11], ]; } @@ -63,7 +68,7 @@ public function provideExamplesForMethods() { return [ [ __DIR__.'/../../examples/cyclomatic1.php', 'A', 6], - [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 5], + [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 4], ]; } diff --git a/tests/Metric/examples/cyclomatic_full.php b/tests/Metric/examples/cyclomatic_full.php new file mode 100644 index 00000000..2ebfa9ca --- /dev/null +++ b/tests/Metric/examples/cyclomatic_full.php @@ -0,0 +1,72 @@ + $d; + } +} From f5f5c495e6e5084ddd5850267d65c93141d967ca Mon Sep 17 00:00:00 2001 From: Oskar Pfeifer-Bley Date: Mon, 9 Jul 2018 08:49:13 +0200 Subject: [PATCH 2/2] Finalize cyclomatic complexity changes - Add documentation about cc and wmc. - Change calculation of wmc. Until now the base path has been added just once per class, this has been changed to once per method to strictly follow wmc. Additional changes: - Code clean up for the touched classes (should now follow PSR-2) --- .../CyclomaticComplexityVisitor.php | 40 +++++++++----- .../CyclomaticComplexityVisitorTest.php | 53 +++++++++---------- 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php b/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php index f3b871bf..ee4fca04 100644 --- a/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php +++ b/src/Hal/Metric/Class_/Complexity/CyclomaticComplexityVisitor.php @@ -8,8 +8,9 @@ use PhpParser\NodeVisitorAbstract; /** - * Calculate cyclomatic complexity number + * Calculate cyclomatic complexity number and weighted method count. * + * The cyclomatic complexity (CC) is a measure of control structure complexity of a function or procedure. * We can calculate ccn in two ways (we choose the second): * * 1. Cyclomatic complexity (CC) = E - N + 2P @@ -18,14 +19,29 @@ * E = number of edges (transfers of control) * N = number of nodes (sequential group of statements containing only one transfer of control) * - * 2. CC = Number of each decision point + * 2. CC = Number of each decision point * + * The weighted method count (WMC) is count of methods parameterized by a algorithm to compute the weight of a method. + * Given a weight metric w and methods m it can be computed as + * + * sum m(w') over (w' in w) + * + * Possible algorithms are: + * + * - Cyclomatic Complexity + * - Lines of Code + * - 1 (unweighted WMC) + * + * This visitor provides two metrics, the maximal CC of all methods from one class (currently stored as ccnMethodMax) + * and the WMC using the CC as weight metric (currently stored as ccn). + * + * @see https://en.wikipedia.org/wiki/Cyclomatic_complexity + * @see http://www.literateprogramming.com/mccabe.pdf + * @see https://www.pitt.edu/~ckemerer/CK%20research%20papers/MetricForOOD_ChidamberKemerer94.pdf */ class CyclomaticComplexityVisitor extends NodeVisitorAbstract { - /** - * @var Metrics - */ + /** @var Metrics */ private $metrics; public function __construct(Metrics $metrics) @@ -41,8 +57,8 @@ public function leaveNode(Node $node) ) { $class = $this->metrics->get(MetricClassNameGenerator::getName($node)); - $ccn = 1; - $ccnByMethod = array(); + $ccn = 0; + $ccnByMethod = [0]; // default maxMethodCcn if no methods are available foreach ($node->stmts as $stmt) { if ($stmt instanceof Stmt\ClassMethod) { @@ -89,19 +105,15 @@ public function leaveNode(Node $node) return $ccn; }; - $methodCcn = $cb($stmt); + $methodCcn = $cb($stmt) + 1; // each method by default is CCN 1 even if it's empty $ccn += $methodCcn; - $ccnByMethod[] = $methodCcn + 1; // each method by default is CCN 1 even if it's empty + $ccnByMethod[] = $methodCcn; } } $class->set('ccn', $ccn); - - $class->set('ccnMethodMax', 0); - if (count($ccnByMethod)) { - $class->set('ccnMethodMax', max($ccnByMethod)); - } + $class->set('ccnMethodMax', max($ccnByMethod)); } } } diff --git a/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php b/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php index f97275bb..743d5424 100644 --- a/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php +++ b/tests/Metric/Class_/Complexity/CyclomaticComplexityVisitorTest.php @@ -3,23 +3,23 @@ use Hal\Metric\Class_\ClassEnumVisitor; use Hal\Metric\Class_\Complexity\CyclomaticComplexityVisitor; -use Hal\Metric\Class_\Complexity\McCabeVisitor; use Hal\Metric\Metrics; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor\NameResolver; use PhpParser\ParserFactory; -class CyclomaticComplexityVisitorTest extends \PHPUnit_Framework_TestCase { - - +class CyclomaticComplexityVisitorTest extends \PHPUnit_Framework_TestCase +{ /** - * @dataProvider provideExamplesForClasses + * @dataProvider provideExamplesForWmc */ - public function testCyclomaticComplexityOfClassesIsWellCalculated($example, $classname, $expectedCcn) + public function testWeightedMethodCountOfClassesIsWellCalculated($example, $classname, $expectedWmc) { $metrics = new Metrics(); $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); - $traverser = new \PhpParser\NodeTraverser(); - $traverser->addVisitor(new \PhpParser\NodeVisitor\NameResolver()); + $traverser = new NodeTraverser(); + $traverser->addVisitor(new NameResolver()); $traverser->addVisitor(new ClassEnumVisitor($metrics)); $traverser->addVisitor(new CyclomaticComplexityVisitor($metrics)); @@ -27,19 +27,19 @@ public function testCyclomaticComplexityOfClassesIsWellCalculated($example, $cla $stmts = $parser->parse($code); $traverser->traverse($stmts); - $this->assertSame($expectedCcn, $metrics->get($classname)->get('ccn')); + $this->assertSame($expectedWmc, $metrics->get($classname)->get('ccn')); } /** - * @dataProvider provideExamplesForMethods + * @dataProvider provideExamplesForMaxCc */ - public function testCyclomaticComplexityOfMethodsIsWellCalculated($example, $classname, $expectedCcnMethodMax) + public function testMaximalCyclomaticComplexityOfMethodsIsWellCalculated($example, $classname, $expectedCcnMethodMax) { $metrics = new Metrics(); $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); - $traverser = new \PhpParser\NodeTraverser(); - $traverser->addVisitor(new \PhpParser\NodeVisitor\NameResolver()); + $traverser = new NodeTraverser(); + $traverser->addVisitor(new NameResolver()); $traverser->addVisitor(new ClassEnumVisitor($metrics)); $traverser->addVisitor(new CyclomaticComplexityVisitor($metrics)); @@ -50,26 +50,25 @@ public function testCyclomaticComplexityOfMethodsIsWellCalculated($example, $cla $this->assertSame($expectedCcnMethodMax, $metrics->get($classname)->get('ccnMethodMax')); } - public function provideExamplesForClasses() + public static function provideExamplesForWmc() { return [ - 'A' => [ __DIR__.'/../../examples/cyclomatic1.php', 'A', 8], - 'B' => [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 4], - 'Foo\\C' => [ __DIR__.'/../../examples/cyclomatic_anon.php', 'Foo\\C', 1], - 'SwitchCase' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'SwitchCase', 4], - 'IfElseif' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'IfElseif', 7], - 'Loops' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'Loops', 5], - 'CatchIt' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'CatchIt', 3], - 'Logical' => [ __DIR__.'/../../examples/cyclomatic_full.php', 'Logical', 11], + 'A' => [__DIR__ . '/../../examples/cyclomatic1.php', 'A', 10], + 'B' => [__DIR__ . '/../../examples/cyclomatic1.php', 'B', 4], + 'Foo\\C' => [__DIR__ . '/../../examples/cyclomatic_anon.php', 'Foo\\C', 1], + 'SwitchCase' => [__DIR__ . '/../../examples/cyclomatic_full.php', 'SwitchCase', 4], + 'IfElseif' => [__DIR__ . '/../../examples/cyclomatic_full.php', 'IfElseif', 7], + 'Loops' => [__DIR__ . '/../../examples/cyclomatic_full.php', 'Loops', 5], + 'CatchIt' => [__DIR__ . '/../../examples/cyclomatic_full.php', 'CatchIt', 3], + 'Logical' => [__DIR__ . '/../../examples/cyclomatic_full.php', 'Logical', 11], ]; } - public function provideExamplesForMethods() + public static function provideExamplesForMaxCc() { return [ - [ __DIR__.'/../../examples/cyclomatic1.php', 'A', 6], - [ __DIR__.'/../../examples/cyclomatic1.php', 'B', 4], + [__DIR__ . '/../../examples/cyclomatic1.php', 'A', 6], + [__DIR__ . '/../../examples/cyclomatic1.php', 'B', 4], ]; } - -} \ No newline at end of file +}