Skip to content

Commit 4dc78b2

Browse files
committed
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.
1 parent 4d9aef0 commit 4dc78b2

File tree

2 files changed

+80
-23
lines changed

2 files changed

+80
-23
lines changed

src/Ruleset.php

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,10 @@ public function explain()
240240
$sniffs = array_keys($this->sniffCodes);
241241
sort($sniffs);
242242

243-
ob_start();
243+
$sniffCount = count($sniffs);
244244

245-
$lastStandard = null;
246-
$lastCount = 0;
247-
$sniffCount = count($sniffs);
248-
249-
// Add a dummy entry to the end so we loop
250-
// one last time and clear the output buffer.
245+
// Add a dummy entry to the end so we loop one last time
246+
// and echo out the collected info about the last standard.
251247
$sniffs[] = '';
252248

253249
$summaryLine = PHP_EOL."The $this->name standard contains 1 sniff".PHP_EOL;
@@ -257,7 +253,9 @@ public function explain()
257253

258254
echo $summaryLine;
259255

260-
ob_start();
256+
$lastStandard = null;
257+
$lastCount = 0;
258+
$sniffsInStandard = [];
261259

262260
foreach ($sniffs as $i => $sniff) {
263261
if ($i === $sniffCount) {
@@ -269,32 +267,31 @@ public function explain()
269267
}
270268
}
271269

270+
// Reached the first item in the next standard.
271+
// Echo out the info collected from the previous standard.
272272
if ($currentStandard !== $lastStandard) {
273-
$sniffList = ob_get_contents();
274-
ob_end_clean();
275-
276-
echo PHP_EOL.$lastStandard.' ('.$lastCount.' sniff';
273+
$subTitle = $lastStandard.' ('.$lastCount.' sniff';
277274
if ($lastCount > 1) {
278-
echo 's';
275+
$subTitle .= 's';
279276
}
280277

281-
echo ')'.PHP_EOL;
282-
echo str_repeat('-', (strlen($lastStandard.$lastCount) + 10));
283-
echo PHP_EOL;
284-
echo $sniffList;
278+
$subTitle .= ')';
285279

286-
$lastStandard = $currentStandard;
287-
$lastCount = 0;
280+
echo PHP_EOL.$subTitle.PHP_EOL;
281+
echo str_repeat('-', strlen($subTitle)).PHP_EOL;
282+
echo ' '.implode(PHP_EOL.' ', $sniffsInStandard).PHP_EOL;
283+
284+
$lastStandard = $currentStandard;
285+
$lastCount = 0;
286+
$sniffsInStandard = [];
288287

289288
if ($currentStandard === null) {
290289
break;
291290
}
292-
293-
ob_start();
294291
}//end if
295292

296-
echo ' '.$sniff.PHP_EOL;
297-
$lastCount++;
293+
$sniffsInStandard[] = $sniff;
294+
++$lastCount;
298295
}//end foreach
299296

300297
}//end explain()

tests/Core/Ruleset/ExplainTest.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
/**
3+
* Tests to verify that the "explain" command functions as expected.
4+
*
5+
* @author Juliette Reinders Folmer <phpcs_nospam@adviesenzo.nl>
6+
* @copyright 2023 Juliette Reinders Folmer. All rights reserved.
7+
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
8+
*/
9+
10+
namespace PHP_CodeSniffer\Tests\Core\Ruleset;
11+
12+
use PHP_CodeSniffer\Config;
13+
use PHP_CodeSniffer\Ruleset;
14+
use PHPUnit\Framework\TestCase;
15+
16+
/**
17+
* Test the Ruleset::explain() function.
18+
*
19+
* @covers \PHP_CodeSniffer\Ruleset::explain
20+
*/
21+
class ExplainTest extends TestCase
22+
{
23+
24+
25+
/**
26+
* Test the output of the "explain" command.
27+
*
28+
* @return void
29+
*/
30+
public function testExplain()
31+
{
32+
// Set up the ruleset.
33+
$config = new Config(['--standard=PSR1', '-e']);
34+
$ruleset = new Ruleset($config);
35+
36+
$expected = PHP_EOL;
37+
$expected .= 'The PSR1 standard contains 8 sniffs'.PHP_EOL.PHP_EOL;
38+
$expected .= 'Generic (4 sniffs)'.PHP_EOL;
39+
$expected .= '------------------'.PHP_EOL;
40+
$expected .= ' Generic.Files.ByteOrderMark'.PHP_EOL;
41+
$expected .= ' Generic.NamingConventions.UpperCaseConstantName'.PHP_EOL;
42+
$expected .= ' Generic.PHP.DisallowAlternativePHPTags'.PHP_EOL;
43+
$expected .= ' Generic.PHP.DisallowShortOpenTag'.PHP_EOL.PHP_EOL;
44+
$expected .= 'PSR1 (3 sniffs)'.PHP_EOL;
45+
$expected .= '---------------'.PHP_EOL;
46+
$expected .= ' PSR1.Classes.ClassDeclaration'.PHP_EOL;
47+
$expected .= ' PSR1.Files.SideEffects'.PHP_EOL;
48+
$expected .= ' PSR1.Methods.CamelCapsMethodName'.PHP_EOL.PHP_EOL;
49+
$expected .= 'Squiz (1 sniff)'.PHP_EOL;
50+
$expected .= '---------------'.PHP_EOL;
51+
$expected .= ' Squiz.Classes.ValidClassName'.PHP_EOL;
52+
53+
$this->expectOutputString($expected);
54+
55+
$ruleset->explain();
56+
57+
}//end testExplain()
58+
59+
60+
}//end class

0 commit comments

Comments
 (0)