From 4dc78b23c6991d1f2dc96c59f43cbae2cf37bddc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 15 Aug 2023 01:12:16 +0200 Subject: [PATCH] Ruleset::explain(): refactor method to remove use of output buffers + add test As pointed out in 3876 / 84, the `Ruleset::explain()` method was using output buffers incorrectly - it contained three `ob_start()` calls and only one `ob_end*()` call, which meant that at the end, there would be at least one unclosed output buffer. The (incorrect) use of the output buffers also meant that any test calling this method would be marked as risky with a "Test code or tested code did not (only) close its own output buffers" notice. In my opinion, the use of output buffers is unnecessary for the "explain" command, so I've refactored the code to remove the use of output buffering completely. The actual function output remains the same, with only a tiny difference - where previously, the separator marker line for a standard of which only 1 sniff was used would be 1 dash too long, the separator marker line will now match the length of the title line for the standard (tiny bug fix). This change now allows for the method to be tested, so a test has been added as well. The test uses the PSR1 standard as: 1. That standard is not expected to change anymore, which should make this test quite stable. 2. That standard has "sub-standards" with both 1 as well as "more than 1" sniffs, meaning the output of the subtitles can be tested. While it may seem redundant to have a test for this code as this is not code which changes often, the deprecation notice fixed in 3876 / 84 would have been caught by this test, while now it was only by the luck of the draw of me running `phpcs -e` while on PHP 8.3 (early) that we found that deprecation. --- src/Ruleset.php | 43 ++++++++++----------- tests/Core/Ruleset/ExplainTest.php | 60 ++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 23 deletions(-) create mode 100644 tests/Core/Ruleset/ExplainTest.php diff --git a/src/Ruleset.php b/src/Ruleset.php index aa80f73643..82facade1e 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -240,14 +240,10 @@ public function explain() $sniffs = array_keys($this->sniffCodes); sort($sniffs); - ob_start(); + $sniffCount = count($sniffs); - $lastStandard = null; - $lastCount = 0; - $sniffCount = count($sniffs); - - // Add a dummy entry to the end so we loop - // one last time and clear the output buffer. + // Add a dummy entry to the end so we loop one last time + // and echo out the collected info about the last standard. $sniffs[] = ''; $summaryLine = PHP_EOL."The $this->name standard contains 1 sniff".PHP_EOL; @@ -257,7 +253,9 @@ public function explain() echo $summaryLine; - ob_start(); + $lastStandard = null; + $lastCount = 0; + $sniffsInStandard = []; foreach ($sniffs as $i => $sniff) { if ($i === $sniffCount) { @@ -269,32 +267,31 @@ public function explain() } } + // Reached the first item in the next standard. + // Echo out the info collected from the previous standard. if ($currentStandard !== $lastStandard) { - $sniffList = ob_get_contents(); - ob_end_clean(); - - echo PHP_EOL.$lastStandard.' ('.$lastCount.' sniff'; + $subTitle = $lastStandard.' ('.$lastCount.' sniff'; if ($lastCount > 1) { - echo 's'; + $subTitle .= 's'; } - echo ')'.PHP_EOL; - echo str_repeat('-', (strlen($lastStandard.$lastCount) + 10)); - echo PHP_EOL; - echo $sniffList; + $subTitle .= ')'; - $lastStandard = $currentStandard; - $lastCount = 0; + echo PHP_EOL.$subTitle.PHP_EOL; + echo str_repeat('-', strlen($subTitle)).PHP_EOL; + echo ' '.implode(PHP_EOL.' ', $sniffsInStandard).PHP_EOL; + + $lastStandard = $currentStandard; + $lastCount = 0; + $sniffsInStandard = []; if ($currentStandard === null) { break; } - - ob_start(); }//end if - echo ' '.$sniff.PHP_EOL; - $lastCount++; + $sniffsInStandard[] = $sniff; + ++$lastCount; }//end foreach }//end explain() diff --git a/tests/Core/Ruleset/ExplainTest.php b/tests/Core/Ruleset/ExplainTest.php new file mode 100644 index 0000000000..934801b674 --- /dev/null +++ b/tests/Core/Ruleset/ExplainTest.php @@ -0,0 +1,60 @@ + + * @copyright 2023 Juliette Reinders Folmer. All rights reserved. + * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Ruleset; + +use PHP_CodeSniffer\Config; +use PHP_CodeSniffer\Ruleset; +use PHPUnit\Framework\TestCase; + +/** + * Test the Ruleset::explain() function. + * + * @covers \PHP_CodeSniffer\Ruleset::explain + */ +class ExplainTest extends TestCase +{ + + + /** + * Test the output of the "explain" command. + * + * @return void + */ + public function testExplain() + { + // Set up the ruleset. + $config = new Config(['--standard=PSR1', '-e']); + $ruleset = new Ruleset($config); + + $expected = PHP_EOL; + $expected .= 'The PSR1 standard contains 8 sniffs'.PHP_EOL.PHP_EOL; + $expected .= 'Generic (4 sniffs)'.PHP_EOL; + $expected .= '------------------'.PHP_EOL; + $expected .= ' Generic.Files.ByteOrderMark'.PHP_EOL; + $expected .= ' Generic.NamingConventions.UpperCaseConstantName'.PHP_EOL; + $expected .= ' Generic.PHP.DisallowAlternativePHPTags'.PHP_EOL; + $expected .= ' Generic.PHP.DisallowShortOpenTag'.PHP_EOL.PHP_EOL; + $expected .= 'PSR1 (3 sniffs)'.PHP_EOL; + $expected .= '---------------'.PHP_EOL; + $expected .= ' PSR1.Classes.ClassDeclaration'.PHP_EOL; + $expected .= ' PSR1.Files.SideEffects'.PHP_EOL; + $expected .= ' PSR1.Methods.CamelCapsMethodName'.PHP_EOL.PHP_EOL; + $expected .= 'Squiz (1 sniff)'.PHP_EOL; + $expected .= '---------------'.PHP_EOL; + $expected .= ' Squiz.Classes.ValidClassName'.PHP_EOL; + + $this->expectOutputString($expected); + + $ruleset->explain(); + + }//end testExplain() + + +}//end class