Skip to content

Commit

Permalink
Fixer: improve performance of generateDiff()
Browse files Browse the repository at this point in the history
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%.
  • Loading branch information
jrfnl committed Feb 21, 2024
1 parent 10e6502 commit de1c3f2
Showing 1 changed file with 42 additions and 2 deletions.
44 changes: 42 additions & 2 deletions src/Fixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

namespace PHP_CodeSniffer;

use PHP_CodeSniffer\Exceptions\RuntimeException;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Common;

Expand Down Expand Up @@ -226,6 +227,8 @@ public function fixFile()
* @param boolean $colors Print coloured output or not.
*
* @return string
*
* @throws \PHP_CodeSniffer\Exceptions\RuntimeException when the diff command fails.
*/
public function generateDiff($filePath=null, $colors=true)
{
Expand All @@ -246,12 +249,49 @@ public function generateDiff($filePath=null, $colors=true)
$fixedFile = fopen($tempName, 'w');
fwrite($fixedFile, $contents);

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

$diff = shell_exec($cmd);
// Stream 0 = STDIN, 1 = STDOUT, 2 = STDERR.
$descriptorspec = [
0 => [
'pipe',
'r',
],
1 => [
'pipe',
'w',
],
2 => [
'pipe',
'w',
],
];

$options = null;
if (stripos(PHP_OS, 'WIN') === 0) {
$options = ['bypass_shell' => true];
}

$process = proc_open($cmd, $descriptorspec, $pipes, $cwd, null, $options);
if (is_resource($process) === false) {
throw new RuntimeException('Could not obtain a resource to execute the diff command.');
}

// We don't need these.
fclose($pipes[0]);
fclose($pipes[2]);

// Stdout will contain the actual diff.
$diff = stream_get_contents($pipes[1]);
fclose($pipes[1]);

proc_close($process);

fclose($fixedFile);
if (is_file($tempName) === true) {
Expand Down

0 comments on commit de1c3f2

Please sign in to comment.