Skip to content

Commit 22d496a

Browse files
committed
Fixer: improve performance of generateDiff()
Similar to 351 and 354, this is a change intended to make the test suite faster, in particular when running on Windows, as a quality of life improvement for contributing developers. However, this change has the added benefit that the performance improvement will also be noticeable for users running PHPCS with the `diff` report, which also uses the `Fixer::generateDiff()` method. On its own, this change makes running the test suite ~50% faster on Windows. In combination with the other two changes, the difference is smaller, but still ~10%.
1 parent 7e248a9 commit 22d496a

File tree

1 file changed

+43
-3
lines changed

1 file changed

+43
-3
lines changed

src/Fixer.php

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
namespace PHP_CodeSniffer;
1414

15+
use PHP_CodeSniffer\Exceptions\RuntimeException;
1516
use PHP_CodeSniffer\Files\File;
1617
use PHP_CodeSniffer\Util\Common;
1718

@@ -223,6 +224,8 @@ public function fixFile()
223224
* @param boolean $colors Print coloured output or not.
224225
*
225226
* @return string
227+
*
228+
* @throws \PHP_CodeSniffer\Exceptions\RuntimeException when the diff command fails.
226229
*/
227230
public function generateDiff($filePath=null, $colors=true)
228231
{
@@ -243,19 +246,56 @@ public function generateDiff($filePath=null, $colors=true)
243246
$fixedFile = fopen($tempName, 'w');
244247
fwrite($fixedFile, $contents);
245248

246-
// We must use something like shell_exec() because whitespace at the end
249+
// We must use something like shell_exec() or proc_open() because whitespace at the end
247250
// of lines is critical to diff files.
251+
// Using proc_open() instead of shell_exec improves performance on Windows significantly,
252+
// while the results are the same (though more code is needed to get the results).
253+
// This is specifically due to proc_open allowing to set the "bypass_shell" option.
248254
$filename = escapeshellarg($filename);
249255
$cmd = "diff -u -L$filename -LPHP_CodeSniffer $filename \"$tempName\"";
250256

251-
$diff = shell_exec($cmd);
257+
// Stream 0 = STDIN, 1 = STDOUT, 2 = STDERR.
258+
$descriptorspec = [
259+
0 => [
260+
'pipe',
261+
'r',
262+
],
263+
1 => [
264+
'pipe',
265+
'w',
266+
],
267+
2 => [
268+
'pipe',
269+
'w',
270+
],
271+
];
272+
273+
$options = null;
274+
if (stripos(PHP_OS, 'WIN') === 0) {
275+
$options = ['bypass_shell' => true];
276+
}
277+
278+
$process = proc_open($cmd, $descriptorspec, $pipes, $cwd, null, $options);
279+
if (is_resource($process) === false) {
280+
throw new RuntimeException('Could not obtain a resource to execute the diff command.');
281+
}
282+
283+
// We don't need these.
284+
fclose($pipes[0]);
285+
fclose($pipes[2]);
286+
287+
// Stdout will contain the actual diff.
288+
$diff = stream_get_contents($pipes[1]);
289+
fclose($pipes[1]);
290+
291+
proc_close($process);
252292

253293
fclose($fixedFile);
254294
if (is_file($tempName) === true) {
255295
unlink($tempName);
256296
}
257297

258-
if ($diff === null) {
298+
if ($diff === false || $diff === '') {
259299
return '';
260300
}
261301

0 commit comments

Comments
 (0)