From 25ad6e21c50d2c853ae51bb2ceddf5f83aebd0b9 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 31 Mar 2025 16:14:48 -0300 Subject: [PATCH 1/4] Generic/Syntax: add support for inspecting code passed via STDIN This commit improves the Generic.PHP.Syntax sniff to make it work when code is passed via STDIN. Before, any code passed this way would cause a false negative as the sniff was not taking into account that `$phpcsFile->getFilename()` might return `STDIN` instead of an actual file name. --- .../Generic/Sniffs/PHP/SyntaxSniff.php | 28 ++++- .../Generic/Tests/PHP/SyntaxUnitTest.php | 104 ++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) diff --git a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php index 7297ebc134..3be0095d1d 100644 --- a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php +++ b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php @@ -56,10 +56,9 @@ public function process(File $phpcsFile, $stackPtr) $this->phpPath = Config::getExecutablePath('php'); } - $fileName = escapeshellarg($phpcsFile->getFilename()); - $cmd = Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' $fileName 2>&1"; - $output = shell_exec($cmd); - $matches = []; + $cmd = $this->getPhpLintCommand($phpcsFile); + $output = shell_exec($cmd); + $matches = []; if (preg_match('/^.*error:(.*) in .* on line ([0-9]+)/m', trim($output), $matches) === 1) { $error = trim($matches[1]); $line = (int) $matches[2]; @@ -72,4 +71,25 @@ public function process(File $phpcsFile, $stackPtr) }//end process() + /** + * Returns the command used to lint PHP code. Uses a different command when the content is + * provided via STDIN. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The File object. + * + * @return string The command used to lint PHP code. + */ + private function getPhpLintCommand(File $phpcsFile) + { + if ($phpcsFile->getFilename() === 'STDIN') { + $content = $phpcsFile->getTokensAsString(0, $phpcsFile->numTokens); + return "echo ".escapeshellarg($content)." | ".Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' 2>&1"; + } + + $fileName = escapeshellarg($phpcsFile->getFilename()); + return Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' $fileName 2>&1"; + + }//end getPhpLintCommand() + + }//end class diff --git a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php index 58433eb1d1..86cc752e2d 100644 --- a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php @@ -10,6 +10,9 @@ namespace PHP_CodeSniffer\Standards\Generic\Tests\PHP; +use PHP_CodeSniffer\Files\DummyFile; +use PHP_CodeSniffer\Ruleset; +use PHP_CodeSniffer\Tests\ConfigDouble; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** @@ -60,4 +63,105 @@ public function getWarningList() }//end getWarningList() + /** + * Test the sniff checks syntax when file contents are passed via STDIN. + * + * @param string $content The content to test. + * @param int $errorCount The expected number of errors. + * @param array $expectedErrors The expected errors. + * + * @dataProvider dataStdIn + * + * @return void + */ + public function testStdIn($content, $errorCount, $expectedErrors) + { + $config = new ConfigDouble(); + $config->standards = ['Generic']; + $config->sniffs = ['Generic.PHP.Syntax']; + + $ruleset = new Ruleset($config); + + $file = new DummyFile($content, $ruleset, $config); + $file->process(); + + $this->assertSame( + $errorCount, + $file->getErrorCount(), + 'Error count does not match expected value' + ); + $this->assertSame( + 0, + $file->getWarningCount(), + 'Warning count does not match expected value' + ); + $this->assertSame( + $expectedErrors, + $file->getErrors(), + 'Error list does not match expected errors' + ); + + }//end testStdIn() + + + /** + * Data provider for testStdIn(). + * + * @return array[] + */ + public function dataStdIn() + { + // The error message changed in PHP 8+. + if (PHP_VERSION_ID >= 80000) { + $errorMessage = 'PHP syntax error: syntax error, unexpected token ";", expecting "]"'; + } else { + $errorMessage = 'PHP syntax error: syntax error, unexpected \';\', expecting \']\''; + } + + return [ + 'No syntax errors' => [ + ' [ + ' [ + 1 => [ + 0 => [ + 'message' => $errorMessage, + 'source' => 'Generic.PHP.Syntax.PHPSyntax', + 'listener' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\PHP\\SyntaxSniff', + 'severity' => 5, + 'fixable' => false, + ], + ], + ], + ], + ], + 'Single error reported even when there is more than syntax error in the file' => [ + ' [ + 1 => [ + 0 => [ + 'message' => $errorMessage, + 'source' => 'Generic.PHP.Syntax.PHPSyntax', + 'listener' => 'PHP_CodeSniffer\\Standards\\Generic\\Sniffs\\PHP\\SyntaxSniff', + 'severity' => 5, + 'fixable' => false, + ], + ], + ], + ], + ], + ]; + + }//end dataStdIn() + + }//end class From d19a93985b4ba5a7d6f1bfc1d2b7a10fd8ca740e Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 2 Jul 2025 14:46:17 -0300 Subject: [PATCH 2/4] Doesn't run the STDIN test on Windows --- src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php index 86cc752e2d..22441271a2 100644 --- a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php @@ -64,13 +64,15 @@ public function getWarningList() /** - * Test the sniff checks syntax when file contents are passed via STDIN. + * Test the sniff checks syntax when file contents are passed via STDIN. Doesn't run on Windows + * as PHPCS currently doesn't support STDIN on this OS. * * @param string $content The content to test. * @param int $errorCount The expected number of errors. * @param array $expectedErrors The expected errors. * * @dataProvider dataStdIn + * @requires OS ^(?!WIN).* * * @return void */ From 9a2f67d565724968cd016e6327bc6dffa9b75231 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 4 Jul 2025 09:51:42 -0300 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php | 5 +++-- src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php index 3be0095d1d..183245b306 100644 --- a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php +++ b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php @@ -72,8 +72,9 @@ public function process(File $phpcsFile, $stackPtr) /** - * Returns the command used to lint PHP code. Uses a different command when the content is - * provided via STDIN. + * Returns the command used to lint PHP code. + * + * Uses a different command when the content is provided via STDIN. * * @param \PHP_CodeSniffer\Files\File $phpcsFile The File object. * diff --git a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php index 22441271a2..115677ae98 100644 --- a/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/SyntaxUnitTest.php @@ -64,8 +64,9 @@ public function getWarningList() /** - * Test the sniff checks syntax when file contents are passed via STDIN. Doesn't run on Windows - * as PHPCS currently doesn't support STDIN on this OS. + * Test the sniff checks syntax when file contents are passed via STDIN. + * + * Note: this test doesn't run on Windows as PHPCS currently doesn't support STDIN on this OS. * * @param string $content The content to test. * @param int $errorCount The expected number of errors. @@ -121,12 +122,12 @@ public function dataStdIn() } return [ - 'No syntax errors' => [ + 'No syntax errors' => [ ' [ + 'One syntax error' => [ ' [ + 'Single error reported even when there is more than one syntax error in the file' => [ ' Date: Fri, 4 Jul 2025 09:59:19 -0300 Subject: [PATCH 4/4] Better formatting for the two commands to check for syntax errors --- src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php index 183245b306..803e3802db 100644 --- a/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php +++ b/src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php @@ -84,11 +84,19 @@ private function getPhpLintCommand(File $phpcsFile) { if ($phpcsFile->getFilename() === 'STDIN') { $content = $phpcsFile->getTokensAsString(0, $phpcsFile->numTokens); - return "echo ".escapeshellarg($content)." | ".Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' 2>&1"; + return sprintf( + "echo %s | %s -l -d display_errors=1 -d error_prepend_string='' 2>&1", + escapeshellarg($content), + Common::escapeshellcmd($this->phpPath) + ); } $fileName = escapeshellarg($phpcsFile->getFilename()); - return Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' $fileName 2>&1"; + return sprintf( + "%s -l -d display_errors=1 -d error_prepend_string='' %s 2>&1", + Common::escapeshellcmd($this->phpPath), + $fileName + ); }//end getPhpLintCommand()