Skip to content
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

fix for #3 & #3 (does not touch SniffTest.php) #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/vendor
.phpunit.result.cache
/vendor
3 changes: 2 additions & 1 deletion phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit bootstrap="tests/bootstrap.php">
<testsuites>
<testsuite name="CognitiveComplexity">
Expand All @@ -9,4 +10,4 @@
<directory suffix=".php">src</directory>
</whitelist>
</filter>
</phpunit>
</phpunit>
118 changes: 62 additions & 56 deletions src/CognitiveComplexity/Analyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

namespace Rarst\PHPCS\CognitiveComplexity;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;

/**
* Based on https://www.sonarsource.com/docs/CognitiveComplexity.pdf
*
Expand All @@ -14,15 +17,6 @@
*/
final class Analyzer
{
/** @var int */
private $cognitiveComplexity = 0;

/** @var bool */
private $isInTryConstruction = false;

/** @var int */
private $lastBooleanOperator = 0;

/**
* B1. Increments
*
Expand Down Expand Up @@ -62,6 +56,9 @@ final class Analyzer
* @var int[]|string[]
*/
private const nestingIncrements = [
T_CLOSURE => T_CLOSURE,
T_ELSEIF => T_ELSEIF, // increments, but does not receive
T_ELSE => T_ELSE, // increments, but does not receive
T_IF => T_IF,
T_INLINE_THEN => T_INLINE_THEN,
T_SWITCH => T_SWITCH,
Expand All @@ -82,11 +79,23 @@ final class Analyzer
T_BREAK => T_BREAK,
];

/** @var int */
private $cognitiveComplexity = 0;

/** @var int */
private $lastBooleanOperator = 0;

private $phpcsFile;

/**
* @param mixed[] $tokens
* @param File $phpcsFile phpcs File instance
* @param int $position current index
*/
public function computeForFunctionFromTokensAndPosition(array $tokens, int $position): int
public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $position): int
{
$this->phpcsFile = $phpcsFile;
$tokens = $phpcsFile->getTokens();

// function without body, e.g. in interface
if (!isset($tokens[$position]['scope_opener'])) {
return 0;
Expand All @@ -96,14 +105,41 @@ public function computeForFunctionFromTokensAndPosition(array $tokens, int $posi
$functionStartPosition = $tokens[$position]['scope_opener'];
$functionEndPosition = $tokens[$position]['scope_closer'];

$this->isInTryConstruction = false;
$this->lastBooleanOperator = 0;
$this->cognitiveComplexity = 0;

/*
Keep track of parser's level stack
We push to this stak whenever we encounter a Tokens::$scopeOpeners
*/
$levelStack = array();
/*
We look for changes in token[level] to know when to remove from the stack
however ['level'] only increases when there are tokens inside {}
after pushing to the stack watch for a level change
*/
$levelIncreased = false;

for ($i = $functionStartPosition + 1; $i < $functionEndPosition; ++$i) {
$currentToken = $tokens[$i];

$this->resolveTryControlStructure($currentToken);
$isNestingToken = false;
if (\in_array($currentToken['code'], Tokens::$scopeOpeners)) {
$isNestingToken = true;
if ($levelIncreased === false && \count($levelStack)) {
// parser's level never increased
// caused by empty condition such as `if ($x) { }`
\array_pop($levelStack);
}
$levelStack[] = $currentToken;
$levelIncreased = false;
} elseif (isset($tokens[$i - 1]) && $currentToken['level'] < $tokens[$i - 1]['level']) {
$diff = $tokens[$i - 1]['level'] - $currentToken['level'];
\array_splice($levelStack, 0 - $diff);
} elseif (isset($tokens[$i - 1]) && $currentToken['level'] > $tokens[$i - 1]['level']) {
$levelIncreased = true;
}

$this->resolveBooleanOperatorChain($currentToken);

if (!$this->isIncrementingToken($currentToken, $tokens, $i)) {
Expand All @@ -112,16 +148,20 @@ public function computeForFunctionFromTokensAndPosition(array $tokens, int $posi

++$this->cognitiveComplexity;

if (isset(self::breakingTokens[$currentToken['code']])) {
$addNestingIncrement = isset(self::nestingIncrements[$currentToken['code']])
&& !\in_array($currentToken['code'], array(T_ELSEIF, T_ELSE));
if (!$addNestingIncrement) {
continue;
}

$isNestingIncrement = isset(self::nestingIncrements[$currentToken['code']]);
$measuredNestingLevel = $this->getMeasuredNestingLevel($currentToken, $tokens, $position);

$measuredNestingLevel = \count(\array_filter($levelStack, function ($token) {
return \in_array($token['code'], self::nestingIncrements);
}));
if ($isNestingToken) {
$measuredNestingLevel--;
}
// B3. Nesting increment
if ($isNestingIncrement && $measuredNestingLevel > 1) {
$this->cognitiveComplexity += $measuredNestingLevel - 1;
if ($measuredNestingLevel > 0) {
$this->cognitiveComplexity += $measuredNestingLevel;
}
}

Expand Down Expand Up @@ -155,23 +195,6 @@ private function resolveBooleanOperatorChain(array $token): void
$this->lastBooleanOperator = $token['code'];
}

/**
* @param mixed[] $token
*/
private function resolveTryControlStructure(array $token): void
{
// code entered "try { }"
if ($token['code'] === T_TRY) {
$this->isInTryConstruction = true;
return;
}

// code left "try { }"
if ($token['code'] === T_CATCH) {
$this->isInTryConstruction = false;
}
}

/**
* @param mixed[] $token
* @param mixed[] $tokens
Expand All @@ -189,29 +212,12 @@ private function isIncrementingToken(array $token, array $tokens, int $position)

// B1. goto LABEL, break LABEL, continue LABEL
if (isset(self::breakingTokens[$token['code']])) {
$nextToken = $tokens[$position + 1]['code'];
if ($nextToken !== T_SEMICOLON) {
$nextToken = $this->phpcsFile->findNext(Tokens::$emptyTokens, $position + 1, null, true);
if ($nextToken === false || $tokens[$nextToken]['code'] !== T_SEMICOLON) {
return true;
}
}

return false;
}

/**
* @param mixed[] $currentToken
* @param mixed[] $tokens
*/
private function getMeasuredNestingLevel(array $currentToken, array $tokens, int $functionTokenPosition): int
{
$functionNestingLevel = $tokens[$functionTokenPosition]['level'];

$measuredNestingLevel = $currentToken['level'] - $functionNestingLevel;

if ($this->isInTryConstruction) {
return --$measuredNestingLevel;
}

return $measuredNestingLevel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,16 @@ public function register(): array
*/
public function process(File $phpcsFile, $stackPtr): void
{
$tokens = $phpcsFile->getTokens();

$cognitiveComplexity = $this->analyzer->computeForFunctionFromTokensAndPosition(
$tokens,
$phpcsFile,
$stackPtr
);

if ($cognitiveComplexity <= $this->maxCognitiveComplexity) {
return;
}

$name = $tokens[$stackPtr + 2]['content'];
$name = $phpcsFile->getDeclarationName($stackPtr);

$phpcsFile->addError(
'Cognitive complexity for "%s" is %d but has to be less than or equal to %d.',
Expand All @@ -54,7 +52,7 @@ public function process(File $phpcsFile, $stackPtr): void
[
$name,
$cognitiveComplexity,
$this->maxCognitiveComplexity
$this->maxCognitiveComplexity,
]
);
}
Expand Down
49 changes: 19 additions & 30 deletions tests/AnalyzerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use Iterator;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Ruleset;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Tokenizers\PHP;
use PHPUnit\Framework\TestCase;
use Rarst\PHPCS\CognitiveComplexity\Analyzer;
Expand All @@ -24,21 +26,12 @@ protected function setUp(): void
*/
public function test(string $filePath, int $expectedCognitiveComplexity): void
{
$fileContent = file_get_contents($filePath);
$tokens = $this->fileToTokens($fileContent);
$functionTokenPosition = null;
foreach ($tokens as $position => $token) {
if ($token['code'] === T_FUNCTION) {
$functionTokenPosition = $position;
break;
}
}

$file = $this->fileFactory($filePath);
$functionTokenPos = $file->findNext(T_FUNCTION, 0);
$cognitiveComplexity = $this->analyzer->computeForFunctionFromTokensAndPosition(
$tokens,
$functionTokenPosition
$file,
$functionTokenPos
);

$this->assertSame($expectedCognitiveComplexity, $cognitiveComplexity);
}

Expand All @@ -49,34 +42,30 @@ public function provideTokensAndExpectedCognitiveComplexity(): Iterator
{
yield [__DIR__ . '/Data/function.php.inc', 9];
yield [__DIR__ . '/Data/function2.php.inc', 6];
yield [__DIR__ . '/Data/function3.php.inc', 1];
yield [__DIR__ . '/Data/function3.php.inc', 9];
yield [__DIR__ . '/Data/function4.php.inc', 2];
yield [__DIR__ . '/Data/function5.php.inc', 19];
yield [__DIR__ . '/Data/function6.php.inc', 0];
yield [__DIR__ . '/Data/function7.php.inc', 3];
yield [__DIR__ . '/Data/function8.php.inc', 7];
yield [__DIR__ . '/Data/function9.php.inc', 5];
yield [__DIR__ . '/Data/function10.php.inc', 19];
yield [__DIR__ . '/Data/function11.php.inc', 5];
yield [__DIR__ . '/Data/function12.php.inc', 8];
}

/**
* @return mixed[]
*/
private function fileToTokens(string $fileContent): array
{
return (new PHP($fileContent, $this->getLegacyConfig()))->getTokens();
}

/**
* @return Config|stdClass
* @param string $filePath
*
* @return File
*/
private function getLegacyConfig()
private function fileFactory($filePath)
{
$config = new stdClass();
$config->tabWidth = 4;
$config->annotations = false;
$config->encoding = 'UTF-8';

return $config;
$config = new Config();
$ruleset = new Ruleset($config);
$file = new File($filePath, $ruleset, $config);
$file->setContent(\file_get_contents($filePath));
$file->parse();
return $file;
}
}
30 changes: 30 additions & 0 deletions tests/Data/function11.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/**
* Test that our token stack is properly maintained with the "missing" break statement
*/
function caseWithoutBreak($string, $thing)
{
switch ($string) {
case 'foo':
switch ($thing) {
case 'ding':
case 'dong':
$string = 'dang';
// no break here!!
}
break;
case 'bar':
case 'baz':
case 'biz':
switch ($thing) {
case 'ding':
case 'dong':
$string = 'dang';
break;
}
break;
}
}

// Cognitive Complexity 5
18 changes: 18 additions & 0 deletions tests/Data/function12.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

function testHybrid($foo, $bar)
{
if ($foo === 'a') { // +1
return;
} elseif ($foo === 'b') { // +1
if ($bar === 'a') { // +2
if (\rand() === 0.5) { // +3
return;
}
} elseif ($bar === 'b') { // +1 (does not receive nesting increment)
return;
}
}
}

// Cognitive Complexity 8
Loading