diff --git a/.gitattributes b/.gitattributes index 78fb299413..c0cb1a4d8b 100644 --- a/.gitattributes +++ b/.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 diff --git a/src/Fixer.php b/src/Fixer.php index e64f89ba6c..b3e933355b 100644 --- a/src/Fixer.php +++ b/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 ''; } diff --git a/tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.diff b/tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.diff new file mode 100644 index 0000000000..b9a217ed23 --- /dev/null +++ b/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'; + } +- +- +- +- diff --git a/tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc b/tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc new file mode 100644 index 0000000000..2e65b2b505 --- /dev/null +++ b/tests/Core/Fixer/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc @@ -0,0 +1,11 @@ + + * @copyright 2024 Juliette Reinders Folmer. All rights reserved. + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Fixer; + +use PHP_CodeSniffer\Files\LocalFile; +use PHP_CodeSniffer\Ruleset; +use PHP_CodeSniffer\Tests\ConfigDouble; +use PHPUnit\Framework\TestCase; + +/** + * Tests for diff generation. + * + * Note: these tests are specifically about the Fixer::generateDiff() method and do not + * test running the fixer itself, nor generating a diff based on a fixer run. + * + * @covers PHP_CodeSniffer\Fixer::generateDiff + */ +final class GenerateDiffTest extends TestCase +{ + + /** + * A \PHP_CodeSniffer\Files\File object to compare the files against. + * + * @var \PHP_CodeSniffer\Files\LocalFile + */ + private static $phpcsFile; + + + /** + * Initialize an \PHP_CodeSniffer\Files\File object with code. + * + * Things to take note of in the code snippet used for these tests: + * - Line endings are \n. + * - Tab indent. + * - Trailing whitespace. + * + * Also note that the Config object is deliberately created without a `tabWidth` setting to + * prevent doing tab replacement when parsing the file. This is to allow for testing a + * diff with tabs vs spaces (which wouldn't yield a diff if tabs had already been replaced). + * + * @beforeClass + * + * @return void + */ + public static function initializeFile() + { + $config = new ConfigDouble(); + $ruleset = new Ruleset($config); + + self::$phpcsFile = new LocalFile(__DIR__.'/Fixtures/GenerateDiffTest.inc', $ruleset, $config); + self::$phpcsFile->parse(); + self::$phpcsFile->fixer->startFile(self::$phpcsFile); + + }//end initializeFile() + + + /** + * Test generating a diff on the file object itself. + * + * @return void + */ + public function testGenerateDiffNoFile() + { + $diff = self::$phpcsFile->fixer->generateDiff(null, false); + + $this->assertSame('', $diff); + + }//end testGenerateDiffNoFile() + + + /** + * Test generating a diff between a PHPCS File object and a file on disk. + * + * @param string $filePath The path to the file to compare the File object against. + * + * @dataProvider dataGenerateDiff + * + * @return void + */ + public function testGenerateDiff($filePath) + { + $diff = self::$phpcsFile->fixer->generateDiff($filePath, false); + + // Allow for the tests to pass on Windows too. + $diff = str_replace('--- tests\Core\Fixer/', '--- tests/Core/Fixer/', $diff); + + $expectedDiffFile = str_replace('.inc', '.diff', $filePath); + + $this->assertStringEqualsFile($expectedDiffFile, $diff); + + }//end testGenerateDiff() + + + /** + * Data provider. + * + * @see testGenerateDiff() + * + * @return array> + */ + public static function dataGenerateDiff() + { + return [ + 'no difference' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-NoDiff.inc', + ], + 'line removed' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-LineRemoved.inc', + ], + 'line added' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-LineAdded.inc', + ], + 'var name changed' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-VarNameChanged.inc', + ], + 'trailing whitespace removed' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-NoTrailingWhitespace.inc', + ], + 'tab replaced with spaces' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-TabsToSpaces.inc', + ], + 'blank lines at start of file' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-BlankLinesAtStart.inc', + ], + 'whitespace diff at start of file' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-WhiteSpaceAtStart.inc', + ], + 'blank lines at end of file' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-BlankLinesAtEnd.inc', + ], + 'whitespace diff at end of file' => [ + 'filePath' => __DIR__.'/Fixtures/GenerateDiffTest-WhiteSpaceAtEnd.inc', + ], + ]; + + }//end dataGenerateDiff() + + + /** + * Test generating a diff between a PHPCS File object and a file on disk and colourizing the output. + * + * @return void + */ + public function testGenerateDiffColoured() + { + $expected = "\033[31m--- tests/Core/Fixer/Fixtures/GenerateDiffTest-VarNameChanged.inc\033[0m".PHP_EOL; + $expected .= "\033[32m+++ PHP_CodeSniffer\033[0m".PHP_EOL; + $expected .= '@@ -1,7 +1,7 @@'.PHP_EOL; + $expected .= ' fixer->generateDiff($filePath); + + // Allow for the tests to pass on Windows too. + $diff = str_replace('--- tests\Core\Fixer/', '--- tests/Core/Fixer/', $diff); + + $this->assertSame($expected, $diff); + + }//end testGenerateDiffColoured() + + + /** + * Test generating a diff between a PHPCS File object using *nix line endings and a file on disk + * using Windows line endings. + * + * The point of this test is to verify that all lines are marked as having a difference. + * The actual lines endings used in the diff shown to the end-user are not relevant for this + * test. + * As the "diff" command is finicky with what type of line endings are used when the only + * difference on a line is the line ending, the test normalizes the line endings of the + * received diff before testing it. + * + * @return void + */ + public function testGenerateDiffDifferentLineEndings() + { + // By the looks of it, if the only diff between two files is line endings, the + // diff generated by the *nix "diff" command will always contain *nix line endings. + $expected = '--- tests/Core/Fixer/Fixtures/GenerateDiffTest-WindowsLineEndings.inc'."\n"; + $expected .= '+++ PHP_CodeSniffer'."\n"; + $expected .= '@@ -1,7 +1,7 @@'."\n"; + $expected .= '-fixer->generateDiff($filePath, false); + + // Allow for the tests to pass on Windows too. + $diff = str_replace('--- tests\Core\Fixer/', '--- tests/Core/Fixer/', $diff); + + // Normalize line endings of the diff. + $diff = preg_replace('`\R`', "\n", $diff); + + $this->assertSame($expected, $diff); + + }//end testGenerateDiffDifferentLineEndings() + + +}//end class