From a7020af2a52d1ea43b1daed52e85003dd95c2689 Mon Sep 17 00:00:00 2001 From: bradley kent Date: Tue, 26 Oct 2021 11:47:16 -0500 Subject: [PATCH 1/6] refactor MaximumComplexity Sniff & Analyzer.php to utilize "$phpcsfile" and how nesting level increment is calculated --- .gitignore | 3 +- phpunit.xml | 25 ++-- src/CognitiveComplexity/Analyzer.php | 141 +++++++++--------- .../Complexity/MaximumComplexitySniff.php | 8 +- tests/AnalyzerTest.php | 47 +++--- tests/Data/function3.php.inc | 39 +++-- 6 files changed, 136 insertions(+), 127 deletions(-) diff --git a/.gitignore b/.gitignore index 49ce3c1..d8a745c 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ -/vendor \ No newline at end of file +.phpunit.result.cache +/vendor diff --git a/phpunit.xml b/phpunit.xml index 8d40677..afa0e20 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,12 +1,13 @@ - - - - tests - - - - - src - - - \ No newline at end of file + + + + + src + + + + + tests + + + diff --git a/src/CognitiveComplexity/Analyzer.php b/src/CognitiveComplexity/Analyzer.php index 9b0510f..31ed5fe 100644 --- a/src/CognitiveComplexity/Analyzer.php +++ b/src/CognitiveComplexity/Analyzer.php @@ -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 * @@ -14,15 +17,6 @@ */ final class Analyzer { - /** @var int */ - private $cognitiveComplexity = 0; - - /** @var bool */ - private $isInTryConstruction = false; - - /** @var int */ - private $lastBooleanOperator = 0; - /** * B1. Increments * @@ -30,7 +24,7 @@ final class Analyzer * * @var int[]|string[] */ - private const increments = [ + private const INCREMENTS = [ T_IF => T_IF, T_ELSE => T_ELSE, T_ELSEIF => T_ELSEIF, @@ -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, @@ -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, @@ -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; @@ -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)) { @@ -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; } } @@ -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; } @@ -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; } @@ -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; - } } diff --git a/src/CognitiveComplexity/Sniffs/Complexity/MaximumComplexitySniff.php b/src/CognitiveComplexity/Sniffs/Complexity/MaximumComplexitySniff.php index 2430e7b..f87eb86 100644 --- a/src/CognitiveComplexity/Sniffs/Complexity/MaximumComplexitySniff.php +++ b/src/CognitiveComplexity/Sniffs/Complexity/MaximumComplexitySniff.php @@ -34,10 +34,8 @@ public function register(): array */ public function process(File $phpcsFile, $stackPtr): void { - $tokens = $phpcsFile->getTokens(); - $cognitiveComplexity = $this->analyzer->computeForFunctionFromTokensAndPosition( - $tokens, + $phpcsFile, $stackPtr ); @@ -45,7 +43,7 @@ public function process(File $phpcsFile, $stackPtr): void 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.', @@ -54,7 +52,7 @@ public function process(File $phpcsFile, $stackPtr): void [ $name, $cognitiveComplexity, - $this->maxCognitiveComplexity + $this->maxCognitiveComplexity, ] ); } diff --git a/tests/AnalyzerTest.php b/tests/AnalyzerTest.php index 3fa13a3..cae377b 100644 --- a/tests/AnalyzerTest.php +++ b/tests/AnalyzerTest.php @@ -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; @@ -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); } @@ -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]; @@ -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; } } diff --git a/tests/Data/function3.php.inc b/tests/Data/function3.php.inc index 1a8e61c..49c8124 100644 --- a/tests/Data/function3.php.inc +++ b/tests/Data/function3.php.inc @@ -1,16 +1,33 @@ Date: Mon, 1 Nov 2021 11:43:39 -0500 Subject: [PATCH 2/6] remove unimportant changes --- src/CognitiveComplexity/Analyzer.php | 31 +++++++++++++--------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/CognitiveComplexity/Analyzer.php b/src/CognitiveComplexity/Analyzer.php index 31ed5fe..522ccab 100644 --- a/src/CognitiveComplexity/Analyzer.php +++ b/src/CognitiveComplexity/Analyzer.php @@ -24,7 +24,7 @@ final class Analyzer * * @var int[]|string[] */ - private const INCREMENTS = [ + private const increments = [ T_IF => T_IF, T_ELSE => T_ELSE, T_ELSEIF => T_ELSEIF, @@ -37,13 +37,13 @@ final class Analyzer ]; /** @var int[]|string[] */ - private const BOOLEAN_OPERATORS = [ + private const booleanOperators = [ T_BOOLEAN_AND => T_BOOLEAN_AND, // && T_BOOLEAN_OR => T_BOOLEAN_OR, // || ]; /** @var int[]|string[] */ - private const OPERATOR_CHAIN_BREAKS = [ + private const operatorChainBreaks = [ T_OPEN_PARENTHESIS => T_OPEN_PARENTHESIS, T_CLOSE_PARENTHESIS => T_CLOSE_PARENTHESIS, T_SEMICOLON => T_SEMICOLON, @@ -53,10 +53,9 @@ final class Analyzer /** * B3. Nesting increments - * * @var int[]|string[] */ - private const NESTING_INCREMENTS = [ + private const nestingIncrements = [ T_CLOSURE => T_CLOSURE, T_IF => T_IF, T_INLINE_THEN => T_INLINE_THEN, @@ -70,10 +69,9 @@ final class Analyzer /** * B1. Increments - * * @var int[] */ - private const BREAKING_TOKENS = [ + private const breakingTokens = [ T_CONTINUE => T_CONTINUE, T_GOTO => T_GOTO, T_BREAK => T_BREAK, @@ -147,12 +145,12 @@ public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $po ++$this->cognitiveComplexity; - $addNestingIncrement = isset(self::NESTING_INCREMENTS[$currentToken['code']]); + $addNestingIncrement = isset(self::nestingIncrements[$currentToken['code']]); if (!$addNestingIncrement) { continue; } $measuredNestingLevel = \count(\array_filter($levelStack, function ($token) { - return \in_array($token['code'], self::NESTING_INCREMENTS); + return \in_array($token['code'], self::nestingIncrements); })); if ($isNestingToken) { $measuredNestingLevel--; @@ -169,18 +167,18 @@ public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $po /** * Keep track of consecutive matching boolean operators, that don't receive increment. * - * @param mixed[] $token token + * @param mixed[] $token */ private function resolveBooleanOperatorChain(array $token): void { // Whenever we cross anything that interrupts possible condition we reset chain. - if ($this->lastBooleanOperator && isset(self::OPERATOR_CHAIN_BREAKS[$token['code']])) { + if ($this->lastBooleanOperator && isset(self::operatorChainBreaks[$token['code']])) { $this->lastBooleanOperator = 0; return; } - if (!isset(self::BOOLEAN_OPERATORS[$token['code']])) { + if (!isset(self::booleanOperators[$token['code']])) { return; } @@ -194,13 +192,12 @@ private function resolveBooleanOperatorChain(array $token): void } /** - * @param mixed[] $token token to test - * @param mixed[] $tokens all function tokens - * @param int $position token index + * @param mixed[] $token + * @param mixed[] $tokens */ private function isIncrementingToken(array $token, array $tokens, int $position): bool { - if (isset(self::INCREMENTS[$token['code']])) { + if (isset(self::increments[$token['code']])) { return true; } @@ -210,7 +207,7 @@ private function isIncrementingToken(array $token, array $tokens, int $position) } // B1. goto LABEL, break LABEL, continue LABEL - if (isset(self::BREAKING_TOKENS[$token['code']])) { + if (isset(self::breakingTokens[$token['code']])) { $nextToken = $this->phpcsFile->findNext(Tokens::$emptyTokens, $position + 1, null, true); if ($nextToken === false || $tokens[$nextToken]['code'] !== T_SEMICOLON) { return true; From 925982a7944d1734812ba9c86b086ed6cc754bba Mon Sep 17 00:00:00 2001 From: bradley kent Date: Mon, 1 Nov 2021 12:40:38 -0500 Subject: [PATCH 3/6] revert phpunit.xml update --- phpunit.xml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index afa0e20..4a41d51 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,13 +1,13 @@ - - - - src - - - - - tests - - + + + + tests + + + + + src + + From 15c953d590c81b3c97d6bf4c37a05dd2014375bc Mon Sep 17 00:00:00 2001 From: bradley kent Date: Mon, 1 Nov 2021 13:05:08 -0500 Subject: [PATCH 4/6] tests/bootstrap.php : define PHP_CODESNIFFER_CBF --- tests/bootstrap.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 6c9f249..d64a29f 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -12,4 +12,9 @@ define('PHP_CODESNIFFER_VERBOSITY', 0); // initialize custom T_* token constants used by PHP_CodeSniffer parser new Tokens(); +} + +// The below two defines are needed for PHPCS 3.x. +if (defined('PHP_CODESNIFFER_CBF') === false) { + define('PHP_CODESNIFFER_CBF', false); } \ No newline at end of file From b17469a3982189912dc7c4e244b8e0748df49144 Mon Sep 17 00:00:00 2001 From: bradley kent Date: Thu, 4 Nov 2021 09:24:03 -0500 Subject: [PATCH 5/6] Handle level changing by more than 1 at once. --- src/CognitiveComplexity/Analyzer.php | 3 ++- tests/AnalyzerTest.php | 1 + tests/Data/function11.php.inc | 30 ++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/Data/function11.php.inc diff --git a/src/CognitiveComplexity/Analyzer.php b/src/CognitiveComplexity/Analyzer.php index 522ccab..adb491e 100644 --- a/src/CognitiveComplexity/Analyzer.php +++ b/src/CognitiveComplexity/Analyzer.php @@ -132,7 +132,8 @@ public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $po $levelStack[] = $currentToken; $levelIncreased = false; } elseif (isset($tokens[$i - 1]) && $currentToken['level'] < $tokens[$i - 1]['level']) { - \array_pop($levelStack); + $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; } diff --git a/tests/AnalyzerTest.php b/tests/AnalyzerTest.php index cae377b..696ab01 100644 --- a/tests/AnalyzerTest.php +++ b/tests/AnalyzerTest.php @@ -50,6 +50,7 @@ public function provideTokensAndExpectedCognitiveComplexity(): Iterator 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]; } /** diff --git a/tests/Data/function11.php.inc b/tests/Data/function11.php.inc new file mode 100644 index 0000000..ce3887a --- /dev/null +++ b/tests/Data/function11.php.inc @@ -0,0 +1,30 @@ + Date: Sun, 14 Nov 2021 22:03:52 -0600 Subject: [PATCH 6/6] elseif & else should contribute to nesting increment, but not receive nesting increment --- src/CognitiveComplexity/Analyzer.php | 5 ++++- tests/AnalyzerTest.php | 1 + tests/Data/function12.php.inc | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tests/Data/function12.php.inc diff --git a/src/CognitiveComplexity/Analyzer.php b/src/CognitiveComplexity/Analyzer.php index adb491e..7cde61c 100644 --- a/src/CognitiveComplexity/Analyzer.php +++ b/src/CognitiveComplexity/Analyzer.php @@ -57,6 +57,8 @@ final class Analyzer */ 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, @@ -146,7 +148,8 @@ public function computeForFunctionFromTokensAndPosition(File $phpcsFile, int $po ++$this->cognitiveComplexity; - $addNestingIncrement = isset(self::nestingIncrements[$currentToken['code']]); + $addNestingIncrement = isset(self::nestingIncrements[$currentToken['code']]) + && !\in_array($currentToken['code'], array(T_ELSEIF, T_ELSE)); if (!$addNestingIncrement) { continue; } diff --git a/tests/AnalyzerTest.php b/tests/AnalyzerTest.php index 696ab01..daa78f5 100644 --- a/tests/AnalyzerTest.php +++ b/tests/AnalyzerTest.php @@ -51,6 +51,7 @@ public function provideTokensAndExpectedCognitiveComplexity(): Iterator 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]; } /** diff --git a/tests/Data/function12.php.inc b/tests/Data/function12.php.inc new file mode 100644 index 0000000..4716619 --- /dev/null +++ b/tests/Data/function12.php.inc @@ -0,0 +1,18 @@ +