Skip to content

fix for #2 & #3 (also updates SniffTest.php) #4

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

Closed
wants to merge 1 commit into from
Closed
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
25 changes: 13 additions & 12 deletions phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
<phpunit bootstrap="tests/bootstrap.php">
<testsuites>
<testsuite name="CognitiveComplexity">
<directory>tests</directory>
</testsuite>
</testsuites>
<filter>
<whitelist processUncoveredFilesFromWhitelist="true">
<directory suffix=".php">src</directory>
</whitelist>
</filter>
</phpunit>
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="tests/bootstrap.php" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage processUncoveredFiles="true">
<include>
<directory suffix=".php">src</directory>
</include>
</coverage>
<testsuites>
<testsuite name="CognitiveComplexity">
<directory>tests</directory>
</testsuite>
</testsuites>
</phpunit>
141 changes: 73 additions & 68 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,23 +17,14 @@
*/
final class Analyzer
{
/** @var int */
private $cognitiveComplexity = 0;

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

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

/**
* B1. Increments
*
* Boolean operators are handled separately due to their chain logic.
*
* @var int[]|string[]
*/
private const increments = [
private const INCREMENTS = [
T_IF => T_IF,
T_ELSE => T_ELSE,
T_ELSEIF => T_ELSEIF,
Expand All @@ -43,13 +37,13 @@ final class Analyzer
];

/** @var int[]|string[] */
private const booleanOperators = [
private const BOOLEAN_OPERATORS = [
T_BOOLEAN_AND => T_BOOLEAN_AND, // &&
T_BOOLEAN_OR => T_BOOLEAN_OR, // ||
];

/** @var int[]|string[] */
private const operatorChainBreaks = [
private const OPERATOR_CHAIN_BREAKS = [
T_OPEN_PARENTHESIS => T_OPEN_PARENTHESIS,
T_CLOSE_PARENTHESIS => T_CLOSE_PARENTHESIS,
T_SEMICOLON => T_SEMICOLON,
Expand All @@ -59,9 +53,11 @@ final class Analyzer

/**
* B3. Nesting increments
*
* @var int[]|string[]
*/
private const nestingIncrements = [
private const NESTING_INCREMENTS = [
T_CLOSURE => T_CLOSURE,
T_IF => T_IF,
T_INLINE_THEN => T_INLINE_THEN,
T_SWITCH => T_SWITCH,
Expand All @@ -74,19 +70,32 @@ final class Analyzer

/**
* B1. Increments
*
* @var int[]
*/
private const breakingTokens = [
private const BREAKING_TOKENS = [
T_CONTINUE => T_CONTINUE,
T_GOTO => T_GOTO,
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,40 @@ 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']) {
\array_pop($levelStack);
} 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 +147,19 @@ public function computeForFunctionFromTokensAndPosition(array $tokens, int $posi

++$this->cognitiveComplexity;

if (isset(self::breakingTokens[$currentToken['code']])) {
$addNestingIncrement = isset(self::NESTING_INCREMENTS[$currentToken['code']]);
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::NESTING_INCREMENTS);
}));
if ($isNestingToken) {
$measuredNestingLevel--;
}
// B3. Nesting increment
if ($isNestingIncrement && $measuredNestingLevel > 1) {
$this->cognitiveComplexity += $measuredNestingLevel - 1;
if ($measuredNestingLevel > 0) {
$this->cognitiveComplexity += $measuredNestingLevel;
}
}

Expand All @@ -131,18 +169,18 @@ public function computeForFunctionFromTokensAndPosition(array $tokens, int $posi
/**
* Keep track of consecutive matching boolean operators, that don't receive increment.
*
* @param mixed[] $token
* @param mixed[] $token token
*/
private function resolveBooleanOperatorChain(array $token): void
{
// Whenever we cross anything that interrupts possible condition we reset chain.
if ($this->lastBooleanOperator && isset(self::operatorChainBreaks[$token['code']])) {
if ($this->lastBooleanOperator && isset(self::OPERATOR_CHAIN_BREAKS[$token['code']])) {
$this->lastBooleanOperator = 0;

return;
}

if (!isset(self::booleanOperators[$token['code']])) {
if (!isset(self::BOOLEAN_OPERATORS[$token['code']])) {
return;
}

Expand All @@ -156,29 +194,13 @@ private function resolveBooleanOperatorChain(array $token): void
}

/**
* @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
* @param mixed[] $token token to test
* @param mixed[] $tokens all function tokens
* @param int $position token index
*/
private function isIncrementingToken(array $token, array $tokens, int $position): bool
{
if (isset(self::increments[$token['code']])) {
if (isset(self::INCREMENTS[$token['code']])) {
return true;
}

Expand All @@ -188,30 +210,13 @@ 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) {
if (isset(self::BREAKING_TOKENS[$token['code']])) {
$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
47 changes: 17 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,7 +42,7 @@ 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];
Expand All @@ -60,23 +53,17 @@ public function provideTokensAndExpectedCognitiveComplexity(): Iterator
}

/**
* @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;
}
}
Loading