Fixer: improve performance of generateDiff()

.gitattributes

@@ -21,3 +21,4 @@ phpunit.xml.dist  export-ignore
 *WinTest.inc text eol=crlf
 *WinTest.php text eol=crlf
 src/Standards/Generic/Tests/Files/LineEndingsUnitTest*.inc text eol=crlf
+tests/Core/Fixer/Fixtures/GenerateDiffTest-WindowsLineEndings.inc text eol=crlf

src/Fixer.php

@@ -12,6 +12,7 @@
 
 namespace PHP_CodeSniffer;
 
+use PHP_CodeSniffer\Exceptions\RuntimeException;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Util\Common;
 
@@ -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)
     {
@@ -246,19 +249,56 @@ 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) {
             unlink($tempName);
         }
 
-        if ($diff === null) {
+        if ($diff === false || $diff === '') {
             return '';
         }
 

tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.diff

@@ -0,0 +1,10 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc
++++ PHP_CodeSniffer
+@@ -5,7 +5,3 @@
+ if ($var) {
+ 	echo 'This line is tab indented';
+ }
+-
+-
+-
+-

tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc

@@ -0,0 +1,11 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}
+
+
+
+

tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtStart.diff

@@ -0,0 +1,9 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtStart.inc
++++ PHP_CodeSniffer
+@@ -1,6 +1,3 @@
+-
+-
+-
+ <?php
+ // Comment with 2 spaces trailing whitespace.  
+ $var = FALSE;

tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtStart.inc

@@ -0,0 +1,10 @@
+
+
+
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-LineAdded.diff

@@ -0,0 +1,8 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-LineAdded.inc
++++ PHP_CodeSniffer
+@@ -1,4 +1,5 @@
+ <?php
++// Comment with 2 spaces trailing whitespace.  
+ $var = FALSE;
+
+ if ($var) {

tests/Core/Fixer/Fixtures/GenerateDiffTest-LineAdded.inc

@@ -0,0 +1,6 @@
+<?php
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-LineRemoved.diff

@@ -0,0 +1,8 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-LineRemoved.inc
++++ PHP_CodeSniffer
+@@ -4,5 +4,4 @@
+
+ if ($var) {
+ 	echo 'This line is tab indented';
+-    echo 'And this line is not';
+ }

tests/Core/Fixer/Fixtures/GenerateDiffTest-LineRemoved.inc

@@ -0,0 +1,8 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+    echo 'And this line is not';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-NoDiff.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-NoTrailingWhitespace.diff

@@ -0,0 +1,9 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-NoTrailingWhitespace.inc
++++ PHP_CodeSniffer
+@@ -1,5 +1,5 @@
+ <?php
+-// Comment with 2 spaces trailing whitespace.
++// Comment with 2 spaces trailing whitespace.  
+ $var = FALSE;
+
+ if ($var) {

tests/Core/Fixer/Fixtures/GenerateDiffTest-NoTrailingWhitespace.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-TabsToSpaces.diff

@@ -0,0 +1,9 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-TabsToSpaces.inc
++++ PHP_CodeSniffer
+@@ -3,5 +3,5 @@
+ $var = FALSE;
+
+ if ($var) {
+-    echo 'This line is tab indented';
++	echo 'This line is tab indented';
+ }

tests/Core/Fixer/Fixtures/GenerateDiffTest-TabsToSpaces.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+    echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-VarNameChanged.diff

@@ -0,0 +1,12 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-VarNameChanged.inc
++++ PHP_CodeSniffer
+@@ -1,7 +1,7 @@
+ <?php
+ // Comment with 2 spaces trailing whitespace.  
+-$rav = FALSE;
++$var = FALSE;
+
+-if ($rav) {
++if ($var) {
+ 	echo 'This line is tab indented';
+ }

tests/Core/Fixer/Fixtures/GenerateDiffTest-VarNameChanged.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$rav = FALSE;
+
+if ($rav) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtEnd.diff

@@ -0,0 +1,8 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtEnd.inc
++++ PHP_CodeSniffer
+@@ -4,4 +4,4 @@
+
+ if ($var) {
+ 	echo 'This line is tab indented';
+-}     
++}

tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtEnd.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}     

tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtStart.diff

@@ -0,0 +1,8 @@
+--- tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtStart.inc
++++ PHP_CodeSniffer
+@@ -1,4 +1,4 @@
+- <?php
++<?php
+ // Comment with 2 spaces trailing whitespace.  
+ $var = FALSE;
+

tests/Core/Fixer/Fixtures/GenerateDiffTest-WhiteSpaceAtStart.inc

@@ -0,0 +1,7 @@
+ <?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest-WindowsLineEndings.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}

tests/Core/Fixer/Fixtures/GenerateDiffTest.inc

@@ -0,0 +1,7 @@
+<?php
+// Comment with 2 spaces trailing whitespace.  
+$var = FALSE;
+
+if ($var) {
+	echo 'This line is tab indented';
+}