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 @@ getMockBuilder(File::class) - ->disableOriginalConstructor() - ->onlyMethods(['getTokens', 'addError']) - ->getMock(); - - $file->expects($this->once()) - ->method('getTokens') - ->willReturn( - [ - [ - 'scope_opener' => 2, - 'scope_closer' => 5, - 'level' => 0, - ], - [], - [ - 'content' => 'function_name', - 'level' => 0, - ], - [ - 'type' => 'T_IF', - 'code' => T_IF, - 'level' => 1, - ], - [ - 'type' => 'T_BOOLEAN_AND', - 'code' => T_BOOLEAN_AND, - 'level' => 2, - ], - ] - ); - - $file->expects($this->once()) - ->method('addError') - ->with( - $this->isType('string'), - $this->anything(), - $this->equalTo('TooHigh'), - $this->equalTo(['function_name', 2, 1]) - ); - - $sniff = new MaximumComplexitySniff(); - $sniff->maxCognitiveComplexity = 1; - - $this->assertEquals([T_FUNCTION], $sniff->register()); - - $sniff->process($file, 0); - } -} diff --git a/tests/Sniffs/Complexity/MaximumComplexitySniffTest.php b/tests/Sniffs/Complexity/MaximumComplexitySniffTest.php new file mode 100644 index 0000000..f3e9ba6 --- /dev/null +++ b/tests/Sniffs/Complexity/MaximumComplexitySniffTest.php @@ -0,0 +1,24 @@ +getErrorCount()); + self::assertSniffError($report, 3, 'TooHigh'); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php new file mode 100644 index 0000000..62bb941 --- /dev/null +++ b/tests/TestCase.php @@ -0,0 +1,201 @@ +)[] $sniffProperties + * @param string[] $codesToCheck + * @param string[] $cliArgs + * @return File + */ + protected static function checkFile(string $filePath, array $sniffProperties = [], array $codesToCheck = [], array $cliArgs = []): File + { + if (defined('PHP_CODESNIFFER_CBF') === false) { + define('PHP_CODESNIFFER_CBF', false); + } + $codeSniffer = new Runner(); + $codeSniffer->config = new Config(array_merge(['-s'], $cliArgs)); + $codeSniffer->init(); + + if (count($sniffProperties) > 0) { + $codeSniffer->ruleset->ruleset[static::getSniffName()]['properties'] = $sniffProperties; + } + + $sniffClassName = static::getSniffClassName(); + + $codeSniffer->ruleset->sniffs = [$sniffClassName => new $sniffClassName()]; + + if (count($codesToCheck) > 0) { + foreach (static::getSniffClassReflection()->getConstants() as $constantName => $constantValue) { + if (strpos($constantName, 'CODE_') !== 0 || in_array($constantValue, $codesToCheck, true)) { + continue; + } + + $codeSniffer->ruleset->ruleset[sprintf('%s.%s', static::getSniffName(), $constantValue)]['severity'] = 0; + } + } + + $codeSniffer->ruleset->populateTokenListeners(); + + $file = new LocalFile($filePath, $codeSniffer->ruleset, $codeSniffer->config); + $file->process(); + + return $file; + } + + protected static function assertNoSniffErrorInFile(File $phpcsFile): void + { + $errors = $phpcsFile->getErrors(); + self::assertEmpty($errors, sprintf('No errors expected, but %d errors found.', count($errors))); + } + + protected static function assertSniffError(File $phpcsFile, int $line, string $code, ?string $message = null): void + { + $errors = $phpcsFile->getErrors(); + self::assertTrue(isset($errors[$line]), sprintf('Expected error on line %s, but none found.', $line)); + + $sniffCode = sprintf('%s.%s', static::getSniffName(), $code); + + self::assertTrue( + self::hasError($errors[$line], $sniffCode, $message), + sprintf( + 'Expected error %s%s, but none found on line %d.%sErrors found on line %d:%s%s%s', + $sniffCode, + $message !== null + ? sprintf(' with message "%s"', $message) + : '', + $line, + PHP_EOL . PHP_EOL, + $line, + PHP_EOL, + self::getFormattedErrors($errors[$line]), + PHP_EOL + ) + ); + } + + protected static function assertNoSniffError(File $phpcsFile, int $line): void + { + $errors = $phpcsFile->getErrors(); + self::assertFalse( + isset($errors[$line]), + sprintf( + 'Expected no error on line %s, but found:%s%s%s', + $line, + PHP_EOL . PHP_EOL, + isset($errors[$line]) ? self::getFormattedErrors($errors[$line]) : '', + PHP_EOL + ) + ); + } + + protected static function assertAllFixedInFile(File $phpcsFile): void + { + $phpcsFile->disableCaching(); + $phpcsFile->fixer->fixFile(); + self::assertStringEqualsFile(preg_replace('~(\\.php)$~', '.fixed\\1', $phpcsFile->getFilename()), $phpcsFile->fixer->getContents()); + } + + /** + * Customized / different from Slevomat + */ + private static function getSniffName(): string + { + return CommonUtil::getSniffCode(static::getSniffClassName()); + } + + /** + * Customized / different from Slevomat + */ + private static function getSniffClassName(): string + { + $className = \substr(static::class, 0, -\strlen('Test')); + return \preg_replace('#\\\Tests?\\\#i', '\\', $className); + } + + private static function getSniffClassReflection(): ReflectionClass + { + static $reflections = []; + + /** @phpstan-var class-string $className */ + $className = static::getSniffClassName(); + + return $reflections[$className] ?? $reflections[$className] = new ReflectionClass($className); + } + + /** + * @param (string|int)[][][] $errorsOnLine + * @param string $sniffCode + * @param string|null $message + * @return bool + */ + private static function hasError(array $errorsOnLine, string $sniffCode, ?string $message): bool + { + $hasError = false; + + foreach ($errorsOnLine as $errorsOnPosition) { + foreach ($errorsOnPosition as $error) { + /** @var string $errorSource */ + $errorSource = $error['source']; + /** @var string $errorMessage */ + $errorMessage = $error['message']; + + if ( + $errorSource === $sniffCode + && ( + $message === null + || strpos($errorMessage, $message) !== false + ) + ) { + $hasError = true; + break; + } + } + } + + return $hasError; + } + + /** + * @param (string|int|bool)[][][] $errors + */ + private static function getFormattedErrors(array $errors): string + { + return implode(PHP_EOL, array_map(static function (array $errors): string { + return implode(PHP_EOL, array_map(static function (array $error): string { + return sprintf("\t%s: %s", $error['source'], $error['message']); + }, $errors)); + }, $errors)); + } + +} \ No newline at end of file diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 6c9f249..641a1f0 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -5,6 +5,7 @@ use PHP_CodeSniffer\Util\Tokens; require_once __DIR__ . '/../vendor/squizlabs/php_codesniffer/autoload.php'; +require_once __DIR__ . '/TestCase.php'; require_once __DIR__ . '/../src/CognitiveComplexity/Analyzer.php'; require_once __DIR__ . '/../src/CognitiveComplexity/Sniffs/Complexity/MaximumComplexitySniff.php';