From 0c2d6d4d37cf3f7df269c7d61ded72639ae23728 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 14 Nov 2024 08:52:18 -0300 Subject: [PATCH 1/3] Generic/CyclomaticComplexity: rename test case file Doing this to be able to create tests with syntax errors on separate files. --- ...inc => CyclomaticComplexityUnitTest.1.inc} | 0 .../Metrics/CyclomaticComplexityUnitTest.php | 42 ++++++++++++------- 2 files changed, 28 insertions(+), 14 deletions(-) rename src/Standards/Generic/Tests/Metrics/{CyclomaticComplexityUnitTest.inc => CyclomaticComplexityUnitTest.1.inc} (100%) diff --git a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.inc b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.inc rename to src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.php b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.php index 950ba3eec3..f7c75f79ca 100644 --- a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.php +++ b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.php @@ -26,11 +26,18 @@ final class CyclomaticComplexityUnitTest extends AbstractSniffUnitTest * The key of the array should represent the line number and the value * should represent the number of errors that should occur on that line. * + * @param string $testFile The name of the file being tested. + * * @return array */ - public function getErrorList() + public function getErrorList($testFile='') { - return [118 => 1]; + switch ($testFile) { + case 'CyclomaticComplexityUnitTest.1.inc': + return [118 => 1]; + default: + return []; + } }//end getErrorList() @@ -41,21 +48,28 @@ public function getErrorList() * The key of the array should represent the line number and the value * should represent the number of warnings that should occur on that line. * + * @param string $testFile The name of the file being tested. + * * @return array */ - public function getWarningList() + public function getWarningList($testFile='') { - return [ - 45 => 1, - 72 => 1, - 189 => 1, - 237 => 1, - 285 => 1, - 333 => 1, - 381 => 1, - 417 => 1, - 445 => 1, - ]; + switch ($testFile) { + case 'CyclomaticComplexityUnitTest.1.inc': + return [ + 45 => 1, + 72 => 1, + 189 => 1, + 237 => 1, + 285 => 1, + 333 => 1, + 381 => 1, + 417 => 1, + 445 => 1, + ]; + default: + return []; + } }//end getWarningList() From 2371b0ae53e19e598a55ddaf7da62aa8d15d500f Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 14 Nov 2024 09:02:51 -0300 Subject: [PATCH 2/3] Generic/CyclomaticComplexity: improve code coverage --- .../Metrics/CyclomaticComplexitySniff.php | 2 +- .../CyclomaticComplexityUnitTest.1.inc | 32 +++++++++++-------- .../CyclomaticComplexityUnitTest.2.inc | 7 ++++ .../CyclomaticComplexityUnitTest.3.inc | 7 ++++ 4 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.2.inc create mode 100644 src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.3.inc diff --git a/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php b/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php index 8cba81daf2..c37140a990 100644 --- a/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php +++ b/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php @@ -60,7 +60,7 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - // Ignore abstract methods. + // Ignore abstract and interface methods. Bail early when live coding. if (isset($tokens[$stackPtr]['scope_opener']) === false) { return; } diff --git a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc index 494dcc7694..9ee638b0cb 100644 --- a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc +++ b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc @@ -46,7 +46,7 @@ function complexityEleven() { while ($condition === true) { if ($condition) { - } else if ($cond) { + } elseif ($cond) { } } @@ -61,11 +61,11 @@ function complexityEleven() echo 'hi'; } break; - case '3': - break; default: break; } + + foreach ($array as $element) {} } @@ -136,14 +136,6 @@ function complexityTwentyOne() echo 'hi'; } break; - case '3': - switch ($cond) { - case '1': - break; - case '2': - break; - } - break; case '4': do { if ($condition) { @@ -159,8 +151,16 @@ function complexityTwentyOne() } break; } -} + try { + for ($i = 0; $i < 10; $i++) { + if ($i % 2) { + doSomething(); + } + } + } catch (Exception $e) { + } +} function complexityTenWithTernaries() { @@ -451,4 +451,10 @@ function complexityElevenWithNullSafeOperator() $bits = $object5->getX()?->getY()?->getZ(); } -?> +abstract class AbstractClass { + abstract public function sniffShouldIgnoreAbstractMethods(); +} + +interface MyInterface { + public function sniffShouldIgnoreInterfaceMethods(); +} diff --git a/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.2.inc b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.2.inc new file mode 100644 index 0000000000..9658af3005 --- /dev/null +++ b/src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.2.inc @@ -0,0 +1,7 @@ + Date: Mon, 2 Dec 2024 10:02:06 -0300 Subject: [PATCH 3/3] Generic/CyclomaticComplexity: ensure the sniff bails if `scope_closer` is not set Quoting @jrfnl: "Only checking for the scope_opener, when accessing/using both the scope_opener and scope_closer indexes, is probably fine in practical terms. However, the part of the code base which sets these indexes is not sufficiently covered by tests, nor does it document that those indexes will only be set if both can be set, so there may be edge case exceptions" (https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/684#discussion_r1865313147). The sniff was already working fine before this change. Checking if `scope_closer` is just an extra precaution to err on the side of caution. --- .../Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php b/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php index c37140a990..a6b17d1315 100644 --- a/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php +++ b/src/Standards/Generic/Sniffs/Metrics/CyclomaticComplexitySniff.php @@ -61,7 +61,7 @@ public function process(File $phpcsFile, $stackPtr) $tokens = $phpcsFile->getTokens(); // Ignore abstract and interface methods. Bail early when live coding. - if (isset($tokens[$stackPtr]['scope_opener']) === false) { + if (isset($tokens[$stackPtr]['scope_opener'], $tokens[$stackPtr]['scope_closer']) === false) { return; }