From 547890bffa7b6e64499bb795d68748e50184b77d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 21 Apr 2025 07:11:35 +0200 Subject: [PATCH 1/5] Introduce new `ExitCode` class This class contains class constants for the new exit codes, so code referencing the constants becomes more self-documenting. Closes squizlabs/PHP_CodeSniffer 3696 --- src/Util/ExitCode.php | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 src/Util/ExitCode.php diff --git a/src/Util/ExitCode.php b/src/Util/ExitCode.php new file mode 100644 index 0000000000..62cee53b42 --- /dev/null +++ b/src/Util/ExitCode.php @@ -0,0 +1,79 @@ + Date: Mon, 21 Apr 2025 07:11:59 +0200 Subject: [PATCH 2/5] Requirements check: use new exit code Includes safeguarding the exit code via the `test-requirements-check.yml` workflow. --- .github/workflows/test-requirements-check.yml | 34 +++++++++++++++++-- requirements.php | 3 +- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-requirements-check.yml b/.github/workflows/test-requirements-check.yml index db58a4e52d..e6cd09a059 100644 --- a/.github/workflows/test-requirements-check.yml +++ b/.github/workflows/test-requirements-check.yml @@ -142,7 +142,14 @@ jobs: - name: Run the test id: check continue-on-error: true - run: php "bin/${{ matrix.cmd }}" --version + shell: bash + run: | + set +e + php "bin/${{ matrix.cmd }}" --version + exitcode="$?" + echo "EXITCODE=$exitcode" >> "$GITHUB_OUTPUT" + echo "Exitcode is: $exitcode" + exit "$exitcode" - name: Check the result of a successful test against expectation if: ${{ steps.check.outcome == 'success' && matrix.expect == 'fail' }} @@ -152,6 +159,14 @@ jobs: if: ${{ steps.check.outcome != 'success' && matrix.expect == 'success' }} run: exit 1 + - name: Verify the exit code is 0 when requirements are met + if: ${{ matrix.expect == 'success' && steps.check.outputs.EXITCODE != 0 }} + run: exit 1 + + - name: Verify the exit code is 64 when requirements are not met + if: ${{ matrix.expect == 'fail' && steps.check.outputs.EXITCODE != 64 }} + run: exit 1 + build-phars: needs: lint @@ -199,7 +214,14 @@ jobs: - name: Run the test id: check continue-on-error: true - run: php ${{ matrix.cmd }}.phar --version + shell: bash + run: | + set +e + php ${{ matrix.cmd }}.phar --version + exitcode="$?" + echo "EXITCODE=$exitcode" >> "$GITHUB_OUTPUT" + echo "Exitcode is: $exitcode" + exit "$exitcode" - name: Check the result of a successful test against expectation if: ${{ steps.check.outcome == 'success' && matrix.expect == 'fail' }} @@ -208,3 +230,11 @@ jobs: - name: Check the result of a failed test against expectation if: ${{ steps.check.outcome != 'success' && matrix.expect == 'success' }} run: exit 1 + + - name: Verify the exit code is 0 when requirements are met + if: ${{ matrix.expect == 'success' && steps.check.outputs.EXITCODE != 0 }} + run: exit 1 + + - name: Verify the exit code is 64 when requirements are not met + if: ${{ matrix.expect == 'fail' && steps.check.outputs.EXITCODE != 64 }} + run: exit 1 diff --git a/requirements.php b/requirements.php index 9b27e11f2d..39b95df6aa 100644 --- a/requirements.php +++ b/requirements.php @@ -29,7 +29,8 @@ */ function checkRequirements() { - $exitCode = 3; + // IMPORTANT: Must stay in sync with the value of the `PHP_CodeSniffer\Util\ExitCode::REQUIREMENTS_NOT_MET` constant! + $exitCode = 64; // Check the PHP version. if (PHP_VERSION_ID < 70200) { From 7ba0a48cd6f5632a609ee7f4e6179bff5c8daf34 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 29 Apr 2025 01:56:12 +0200 Subject: [PATCH 3/5] Use new exit codes for DeepExitExceptions Includes updating the documentation of the `DeepExitException` class. --- src/Config.php | 61 ++++++++++++++-------------- src/Exceptions/DeepExitException.php | 5 +++ src/Files/FileList.php | 3 +- src/Reporter.php | 5 ++- src/Reports/Gitblame.php | 3 +- src/Reports/Hgblame.php | 5 ++- src/Reports/Svnblame.php | 3 +- src/Runner.php | 12 +++--- 8 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/Config.php b/src/Config.php index 2974121725..d97767fed9 100644 --- a/src/Config.php +++ b/src/Config.php @@ -17,6 +17,7 @@ use PHP_CodeSniffer\Exceptions\DeepExitException; use PHP_CodeSniffer\Exceptions\RuntimeException; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\ExitCode; use PHP_CodeSniffer\Util\Help; use PHP_CodeSniffer\Util\Standards; @@ -678,10 +679,10 @@ public function processShortArgument($arg, $pos) case 'h': case '?': $this->printUsage(); - throw new DeepExitException('', 0); + throw new DeepExitException('', ExitCode::OKAY); case 'i' : $output = Standards::prepareInstalledStandardsForDisplay().PHP_EOL; - throw new DeepExitException($output, 0); + throw new DeepExitException($output, ExitCode::OKAY); case 'v' : if ($this->quiet === true) { // Ignore when quiet mode is enabled. @@ -747,7 +748,7 @@ public function processShortArgument($arg, $pos) if ($changed === false && ini_get($ini[0]) !== $ini[1]) { $error = sprintf('ERROR: Ini option "%s" cannot be changed at runtime.', $ini[0]).PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } break; case 'n' : @@ -789,11 +790,11 @@ public function processLongArgument($arg, $pos) switch ($arg) { case 'help': $this->printUsage(); - throw new DeepExitException('', 0); + throw new DeepExitException('', ExitCode::OKAY); case 'version': $output = 'PHP_CodeSniffer version '.self::VERSION.' ('.self::STABILITY.') '; $output .= 'by Squiz and PHPCSStandards'.PHP_EOL; - throw new DeepExitException($output, 0); + throw new DeepExitException($output, ExitCode::OKAY); case 'colors': if (isset($this->overriddenDefaults['colors']) === true) { break; @@ -840,7 +841,7 @@ public function processLongArgument($arg, $pos) ) { $error = 'ERROR: Setting a config option requires a name and value'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $key = $this->cliArgs[($pos + 1)]; @@ -850,7 +851,7 @@ public function processLongArgument($arg, $pos) try { $this->setConfigData($key, $value); } catch (Exception $e) { - throw new DeepExitException($e->getMessage().PHP_EOL, 3); + throw new DeepExitException($e->getMessage().PHP_EOL, ExitCode::PROCESS_ERROR); } $output = 'Using config file: '.self::$configDataFile.PHP_EOL.PHP_EOL; @@ -860,12 +861,12 @@ public function processLongArgument($arg, $pos) } else { $output .= "Config value \"$key\" updated successfully; old value was \"$current\"".PHP_EOL; } - throw new DeepExitException($output, 0); + throw new DeepExitException($output, ExitCode::OKAY); case 'config-delete': if (isset($this->cliArgs[($pos + 1)]) === false) { $error = 'ERROR: Deleting a config option requires the name of the option'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $output = 'Using config file: '.self::$configDataFile.PHP_EOL.PHP_EOL; @@ -878,24 +879,24 @@ public function processLongArgument($arg, $pos) try { $this->setConfigData($key, null); } catch (Exception $e) { - throw new DeepExitException($e->getMessage().PHP_EOL, 3); + throw new DeepExitException($e->getMessage().PHP_EOL, ExitCode::PROCESS_ERROR); } $output .= "Config value \"$key\" removed successfully; old value was \"$current\"".PHP_EOL; } - throw new DeepExitException($output, 0); + throw new DeepExitException($output, ExitCode::OKAY); case 'config-show': $data = self::getAllConfigData(); $output = 'Using config file: '.self::$configDataFile.PHP_EOL.PHP_EOL; $output .= $this->prepareConfigDataForDisplay($data); - throw new DeepExitException($output, 0); + throw new DeepExitException($output, ExitCode::OKAY); case 'runtime-set': if (isset($this->cliArgs[($pos + 1)]) === false || isset($this->cliArgs[($pos + 2)]) === false ) { $error = 'ERROR: Setting a runtime config option requires a name and value'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $key = $this->cliArgs[($pos + 1)]; @@ -946,7 +947,7 @@ public function processLongArgument($arg, $pos) if (is_dir($dir) === false) { $error = 'ERROR: The specified cache file path "'.$this->cacheFile.'" points to a non-existent directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } if ($dir === '.') { @@ -972,7 +973,7 @@ public function processLongArgument($arg, $pos) if (is_dir($this->cacheFile) === true) { $error = 'ERROR: The specified cache file path "'.$this->cacheFile.'" is a directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } } else if (substr($arg, 0, 10) === 'bootstrap=') { $files = explode(',', substr($arg, 10)); @@ -982,7 +983,7 @@ public function processLongArgument($arg, $pos) if ($path === false) { $error = 'ERROR: The specified bootstrap file "'.$file.'" does not exist'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $bootstrap[] = $path; @@ -996,7 +997,7 @@ public function processLongArgument($arg, $pos) if ($path === false) { $error = 'ERROR: The specified file list "'.$fileList.'" does not exist'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $files = file($path); @@ -1038,7 +1039,7 @@ public function processLongArgument($arg, $pos) if (is_dir($dir) === false) { $error = 'ERROR: The specified report file path "'.$this->reportFile.'" points to a non-existent directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $this->reportFile = $dir.'/'.basename($this->reportFile); @@ -1049,7 +1050,7 @@ public function processLongArgument($arg, $pos) if (is_dir($this->reportFile) === true) { $error = 'ERROR: The specified report file path "'.$this->reportFile.'" is a directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } } else if (substr($arg, 0, 13) === 'report-width=') { if (isset($this->overriddenDefaults['reportWidth']) === true) { @@ -1080,7 +1081,7 @@ public function processLongArgument($arg, $pos) if (is_dir($this->basepath) === false) { $error = 'ERROR: The specified basepath "'.$this->basepath.'" points to a non-existent directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } } else if ((substr($arg, 0, 7) === 'report=' || substr($arg, 0, 7) === 'report-')) { $reports = []; @@ -1101,7 +1102,7 @@ public function processLongArgument($arg, $pos) if (is_dir($dir) === false) { $error = 'ERROR: The specified '.$report.' report file path "'.$output.'" points to a non-existent directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $output = $dir.'/'.basename($output); @@ -1109,7 +1110,7 @@ public function processLongArgument($arg, $pos) if (is_dir($output) === true) { $error = 'ERROR: The specified '.$report.' report file path "'.$output.'" is a directory'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } }//end if }//end if @@ -1167,7 +1168,7 @@ public function processLongArgument($arg, $pos) $error .= 'PHP_CodeSniffer >= 4.0 only supports scanning PHP files.'.PHP_EOL; $error .= 'Received: '.substr($arg, 11).PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } } @@ -1258,7 +1259,7 @@ public function processLongArgument($arg, $pos) $validOptions ); $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $this->generator = $this->validGenerators[$lowerCaseGeneratorName]; @@ -1384,7 +1385,7 @@ static function ($carry, $item) { $error .= PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException(ltrim($error), 3); + throw new DeepExitException(ltrim($error), ExitCode::PROCESS_ERROR); } return $sniffs; @@ -1413,7 +1414,7 @@ public function processUnknownArgument($arg, $pos) $error = "ERROR: option \"$arg\" not known".PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $this->processFilePath($arg); @@ -1444,7 +1445,7 @@ public function processFilePath($path) $error = 'ERROR: The file "'.$path.'" does not exist.'.PHP_EOL.PHP_EOL; $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } else { // Can't modify the files array directly because it's not a real // class member, so need to use this little get/modify/set trick. @@ -1652,7 +1653,7 @@ public function setConfigData($key, $value, $temp=false) && is_writable($configFile) === false ) { $error = 'ERROR: Config file '.$configFile.' is not writable'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } }//end if @@ -1673,7 +1674,7 @@ public function setConfigData($key, $value, $temp=false) if (file_put_contents($configFile, $output) === false) { $error = 'ERROR: Config file '.$configFile.' could not be written'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } self::$configDataFile = $configFile; @@ -1731,7 +1732,7 @@ public static function getAllConfigData() if (Common::isReadable($configFile) === false) { $error = 'ERROR: Config file '.$configFile.' is not readable'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } include $configFile; diff --git a/src/Exceptions/DeepExitException.php b/src/Exceptions/DeepExitException.php index 6943e033ed..4b81305cc6 100644 --- a/src/Exceptions/DeepExitException.php +++ b/src/Exceptions/DeepExitException.php @@ -5,8 +5,13 @@ * Allows the runner to return an exit code instead of putting exit codes elsewhere * in the source code. * + * Exit codes passed to this exception (as the `$code` parameter) MUST be one of the + * predefined exit code constants per the `PHP_CodeSniffer\Util\ExitCode` class; or a bitmask sum of those. + * * @author Greg Sherwood + * @author Juliette Reinders Folmer * @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600) + * @copyright 2025 PHPCSStandards and contributors * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence */ diff --git a/src/Files/FileList.php b/src/Files/FileList.php index ab52e33888..7c5ceadfdd 100644 --- a/src/Files/FileList.php +++ b/src/Files/FileList.php @@ -19,6 +19,7 @@ use PHP_CodeSniffer\Exceptions\DeepExitException; use PHP_CodeSniffer\Ruleset; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\ExitCode; use RecursiveArrayIterator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -157,7 +158,7 @@ private function getFilterClass() $filename = realpath($filterType); if ($filename === false) { $error = "ERROR: Custom filter \"$filterType\" not found".PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $filterClass = Autoload::loadFile($filename); diff --git a/src/Reporter.php b/src/Reporter.php index 78134bda71..8f230d635f 100644 --- a/src/Reporter.php +++ b/src/Reporter.php @@ -14,6 +14,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Reports\Report; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\ExitCode; class Reporter { @@ -103,7 +104,7 @@ public function __construct(Config $config) $filename = realpath($type); if ($filename === false) { $error = "ERROR: Custom report \"$type\" not found".PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $reportClassName = Autoload::loadFile($filename); @@ -132,7 +133,7 @@ public function __construct(Config $config) if ($reportClassName === '') { $error = "ERROR: Class file for report \"$type\" not found".PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $reportClass = new $reportClassName(); diff --git a/src/Reports/Gitblame.php b/src/Reports/Gitblame.php index 3beb34524a..c035ba75bb 100644 --- a/src/Reports/Gitblame.php +++ b/src/Reports/Gitblame.php @@ -11,6 +11,7 @@ namespace PHP_CodeSniffer\Reports; use PHP_CodeSniffer\Exceptions\DeepExitException; +use PHP_CodeSniffer\Util\ExitCode; class Gitblame extends VersionControl { @@ -74,7 +75,7 @@ protected function getBlameContent($filename) $handle = popen($command, 'r'); if ($handle === false) { $error = 'ERROR: Could not execute "'.$command.'"'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $rawContent = stream_get_contents($handle); diff --git a/src/Reports/Hgblame.php b/src/Reports/Hgblame.php index 2d98c46db6..caf47e111b 100644 --- a/src/Reports/Hgblame.php +++ b/src/Reports/Hgblame.php @@ -11,6 +11,7 @@ namespace PHP_CodeSniffer\Reports; use PHP_CodeSniffer\Exceptions\DeepExitException; +use PHP_CodeSniffer\Util\ExitCode; class Hgblame extends VersionControl { @@ -86,14 +87,14 @@ protected function getBlameContent($filename) chdir($location); } else { $error = 'ERROR: Could not locate .hg directory '.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $command = 'hg blame -u -d -v "'.$filename.'" 2>&1'; $handle = popen($command, 'r'); if ($handle === false) { $error = 'ERROR: Could not execute "'.$command.'"'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $rawContent = stream_get_contents($handle); diff --git a/src/Reports/Svnblame.php b/src/Reports/Svnblame.php index b44e83d627..51e4a760f6 100644 --- a/src/Reports/Svnblame.php +++ b/src/Reports/Svnblame.php @@ -10,6 +10,7 @@ namespace PHP_CodeSniffer\Reports; use PHP_CodeSniffer\Exceptions\DeepExitException; +use PHP_CodeSniffer\Util\ExitCode; class Svnblame extends VersionControl { @@ -57,7 +58,7 @@ protected function getBlameContent($filename) $handle = popen($command, 'r'); if ($handle === false) { $error = 'ERROR: Could not execute "'.$command.'"'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } $rawContent = stream_get_contents($handle); diff --git a/src/Runner.php b/src/Runner.php index bf5d85561d..169fe30ab0 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -21,6 +21,7 @@ use PHP_CodeSniffer\Files\FileList; use PHP_CodeSniffer\Util\Cache; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\ExitCode; use PHP_CodeSniffer\Util\Standards; use PHP_CodeSniffer\Util\Timing; use PHP_CodeSniffer\Util\Tokens; @@ -278,7 +279,7 @@ public function init() // out by letting them know which standards are installed. $error = 'ERROR: the "'.$standard.'" coding standard is not installed.'.PHP_EOL.PHP_EOL; $error .= Standards::prepareInstalledStandardsForDisplay().PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } } @@ -309,7 +310,7 @@ public function init() } catch (RuntimeException $e) { $error = rtrim($e->getMessage(), "\r\n").PHP_EOL.PHP_EOL; $error .= $this->config->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } }//end init() @@ -348,7 +349,7 @@ private function run() if (empty($this->config->files) === true) { $error = 'ERROR: You must supply at least one file or directory to process.'.PHP_EOL.PHP_EOL; $error .= $this->config->printShortUsage(true); - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } if (PHP_CODESNIFFER_VERBOSITY > 0) { @@ -380,7 +381,7 @@ private function run() if ($numFiles === 0) { $error = 'ERROR: No files were checked.'.PHP_EOL; $error .= 'All specified files were excluded or did not match filtering rules.'.PHP_EOL.PHP_EOL; - throw new DeepExitException($error, 3); + throw new DeepExitException($error, ExitCode::PROCESS_ERROR); } // Turn all sniff errors into exceptions. @@ -694,7 +695,8 @@ public function processFile($file) case 's': break(2); case 'q': - throw new DeepExitException('', 0); + // User request to "quit": exit code should be 0. + throw new DeepExitException('', ExitCode::OKAY); default: // Repopulate the sniffs because some of them save their state // and only clear it when the file changes, but we are rechecking From 7dcdab22b29fd39c7e78f47daeebf7dc39608641 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 6 May 2025 08:26:08 +0200 Subject: [PATCH 4/5] Change exit codes for scan runs This commit implements the change in the exit codes for scan runs, both `phpcs` as well as `phpcbf`, as previously outlined and extensively discussed in 184. It also introduces a new `ignore_non_auto_fixable_on_exit` config flag to influence the exit code. To allow for determining whether there are fixable and/or non-fixable issues left at the end of a run, while also supporting the previously already supported `ignore_errors_on_exit` and `ignore_warnings_on_exit` config flags, this required more extensive changes than originally anticipated. Originally, the `File` class and the `Reporter` class tracked (single file/all files) totals for: * Errors found * Warnings found * Fixable violations found (errors and warnings combined) * Violations fixed (errors and warnings combined) Additionally, the total for "fixed" could be higher than the "fixable" count as it totals the fixes applied in all loops, including fixes for issues _created by fixers_ and not in the original scan, which made the "fixed" total unusable for the new exit code determination. To allow for correctly determining the new exit codes, the "fixable" total had to be split up into "fixable errors" and "fixable warnings", both in the `File` class, as well as in the `Reporter` class. Secondly, the "fixed" total also had to be split up, as well as being based on the _effective_ number of fixes made instead of the _actual_ number of fixes made. These changes also affect the way scan results are cached, as the cache now also needs to store the split up numbers (for "fixable") and how parallel processing accumulates the totals of the child processes. And as the logic for the exit code determination for the new exit codes is largely the same for both `phpcs` as well as `phpcbf`, a new `ExitCode::calculate()` method has been introduced to transparently handle this for both. Notable API changes: * The signature of the `DummyFile::setErrorCounts()` method has changed: ```diff -public function setErrorCounts($errorCount, $warningCount, $fixableCount, $fixedCount) +public function setErrorCounts($errorCount, $warningCount, $fixableErrorCount, $fixableWarningCount, $fixedErrorCount, $fixedWarningCount) ``` * The `Reporter::$totalFixable` and `Reporter::$totalFixed` properties are now deprecated. They will still be supported as readonly properties for the 4.x branch (via magic methods), but support will be removed in PHPCS 5.0. * The return value of the `private` `Runner::run()` method has changed from `int` to ` void` as the return value was no longer used. Includes mentioning the new `ignore_non_auto_fixable_on_exit` config flag in the CLI help text. Includes extensive integration tests for the exit code calculations and the changes made in support of the new exit codes. Includes git-ignoring temporary files which will be created during the test run in case the test run would bork out before the tear down methods are run. Includes tests for the new magic methods in the `Reporter` class to safeguard that the deprecated properties are still accessible, but not writeable. Includes improving the documentation for `Fixer::$numFixes`/`Fixer::getFixCount()` to mention that these numbers are _actual_ number fixes, not _effective_ number of fixes. Fixes 184 Fixes squizlabs/PHP_CodeSniffer 2898 --- .gitignore | 2 + src/Files/DummyFile.php | 22 +- src/Files/File.php | 143 +++- src/Files/LocalFile.php | 35 +- src/Fixer.php | 9 +- src/Reporter.php | 115 ++- src/Runner.php | 104 +-- src/Util/ExitCode.php | 66 ++ src/Util/Help.php | 2 +- tests/Core/Reporter/MagicMethodsTest.php | 150 ++++ tests/Core/Util/ExitCode/ExitCodeTest.php | 673 ++++++++++++++++++ tests/Core/Util/ExitCode/ExitCodeTest.xml | 8 + .../ExitCodeTest/mix-errors-warnings.inc | 6 + .../Fixtures/ExitCodeTest/only-errors.inc | 5 + .../ExitCodeTest/only-fixable-errors.inc | 3 + .../ExitCodeTest/only-fixable-warnings.inc | 3 + .../Fixtures/ExitCodeTest/only-warnings.inc | 4 + .../Sniffs/ExitCodes/ErrorSniff.php | 28 + .../Sniffs/ExitCodes/FailToFixSniff.php | 38 + .../Sniffs/ExitCodes/FixableErrorSniff.php | 33 + .../Sniffs/ExitCodes/FixableWarningSniff.php | 31 + .../Sniffs/ExitCodes/NoIssuesSniff.php | 25 + .../Sniffs/ExitCodes/WarningSniff.php | 25 + .../Fixtures/TestStandard/ruleset.xml | 4 + 24 files changed, 1408 insertions(+), 126 deletions(-) create mode 100644 tests/Core/Reporter/MagicMethodsTest.php create mode 100644 tests/Core/Util/ExitCode/ExitCodeTest.php create mode 100644 tests/Core/Util/ExitCode/ExitCodeTest.xml create mode 100644 tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/mix-errors-warnings.inc create mode 100644 tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/only-errors.inc create mode 100644 tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/only-fixable-errors.inc create mode 100644 tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/only-fixable-warnings.inc create mode 100644 tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/only-warnings.inc create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/ErrorSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FailToFixSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableErrorSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableWarningSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/NoIssuesSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/WarningSniff.php create mode 100644 tests/Core/Util/ExitCode/Fixtures/TestStandard/ruleset.xml diff --git a/.gitignore b/.gitignore index dedd53df79..96b464dec4 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,5 @@ composer.lock phpstan.neon /node_modules/ /tests/Standards/sniffStnd.xml +/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/*.fixed +/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/phpcs.cache diff --git a/src/Files/DummyFile.php b/src/Files/DummyFile.php index f5dc7cc7ac..b10a18086a 100644 --- a/src/Files/DummyFile.php +++ b/src/Files/DummyFile.php @@ -62,19 +62,23 @@ public function __construct($content, Ruleset $ruleset, Config $config) /** * Set the error, warning, and fixable counts for the file. * - * @param int $errorCount The number of errors found. - * @param int $warningCount The number of warnings found. - * @param int $fixableCount The number of fixable errors found. - * @param int $fixedCount The number of errors that were fixed. + * @param int $errorCount The number of errors found. + * @param int $warningCount The number of warnings found. + * @param int $fixableErrorCount The number of fixable errors found. + * @param int $fixableWarningCount The number of fixable warning found. + * @param int $fixedErrorCount The number of errors that were fixed. + * @param int $fixedWarningCount The number of warning that were fixed. * * @return void */ - public function setErrorCounts($errorCount, $warningCount, $fixableCount, $fixedCount) + public function setErrorCounts($errorCount, $warningCount, $fixableErrorCount, $fixableWarningCount, $fixedErrorCount, $fixedWarningCount) { - $this->errorCount = $errorCount; - $this->warningCount = $warningCount; - $this->fixableCount = $fixableCount; - $this->fixedCount = $fixedCount; + $this->errorCount = $errorCount; + $this->warningCount = $warningCount; + $this->fixableErrorCount = $fixableErrorCount; + $this->fixableWarningCount = $fixableWarningCount; + $this->fixedErrorCount = $fixedErrorCount; + $this->fixedWarningCount = $fixedWarningCount; }//end setErrorCounts() diff --git a/src/Files/File.php b/src/Files/File.php index be3d8d4c0f..eddbb3a218 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -153,19 +153,67 @@ class File protected $warningCount = 0; /** - * The total number of errors and warnings that can be fixed. + * The original total number of errors that can be fixed (first run on a file). + * + * {@internal This should be regarded as an immutable property.} * * @var integer */ - protected $fixableCount = 0; + private $fixableErrorCountFirstRun; /** - * The total number of errors and warnings that were fixed. + * The original total number of warnings that can be fixed (first run on a file). + * + * {@internal This should be regarded as an immutable property.} + * + * @var integer + */ + private $fixableWarningCountFirstRun; + + /** + * The current total number of errors that can be fixed. + * + * @var integer + */ + protected $fixableErrorCount = 0; + + /** + * The current total number of warnings that can be fixed. + * + * @var integer + */ + protected $fixableWarningCount = 0; + + /** + * The actual number of errors and warnings that were fixed. + * + * I.e. how many fixes were applied. This may be higher than the originally found + * issues if the fixer from one sniff causes other sniffs to come into play in follow-up loops. + * Example: if a brace is moved to a new line, the `ScopeIndent` sniff may need to ensure + * the brace is indented correctly in the next loop. * * @var integer */ protected $fixedCount = 0; + /** + * The effective number of errors that were fixed. + * + * I.e. how many of the originally found errors were fixed. + * + * @var integer + */ + protected $fixedErrorCount = 0; + + /** + * The effective number of warnings that were fixed. + * + * I.e. how many of the originally found warnings were fixed. + * + * @var integer + */ + protected $fixedWarningCount = 0; + /** * TRUE if errors are being replayed from the cache. * @@ -309,11 +357,12 @@ public function process() return; } - $this->errors = []; - $this->warnings = []; - $this->errorCount = 0; - $this->warningCount = 0; - $this->fixableCount = 0; + $this->errors = []; + $this->warnings = []; + $this->errorCount = 0; + $this->warningCount = 0; + $this->fixableErrorCount = 0; + $this->fixableWarningCount = 0; $this->parse(); @@ -351,11 +400,12 @@ public function process() || substr($commentTextLower, 0, 17) === '@phpcs:ignorefile' ) { // Ignoring the whole file, just a little late. - $this->errors = []; - $this->warnings = []; - $this->errorCount = 0; - $this->warningCount = 0; - $this->fixableCount = 0; + $this->errors = []; + $this->warnings = []; + $this->errorCount = 0; + $this->warningCount = 0; + $this->fixableErrorCount = 0; + $this->fixableWarningCount = 0; return; } else if (substr($commentTextLower, 0, 9) === 'phpcs:set' || substr($commentTextLower, 0, 10) === '@phpcs:set' @@ -505,7 +555,14 @@ public function process() StatusWriter::write('*** END SNIFF PROCESSING REPORT ***', 1); } - $this->fixedCount += $this->fixer->getFixCount(); + if (isset($this->fixableErrorCountFirstRun, $this->fixableWarningCountFirstRun) === false) { + $this->fixableErrorCountFirstRun = $this->fixableErrorCount; + $this->fixableWarningCountFirstRun = $this->fixableWarningCount; + } + + $this->fixedCount += $this->fixer->getFixCount(); + $this->fixedErrorCount = ($this->fixableErrorCountFirstRun - $this->fixableErrorCount); + $this->fixedWarningCount = ($this->fixableWarningCountFirstRun - $this->fixableWarningCount); }//end process() @@ -1004,7 +1061,11 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s $messageCount++; if ($fixable === true) { - $this->fixableCount++; + if ($error === true) { + $this->fixableErrorCount++; + } else { + $this->fixableWarningCount++; + } } if ($this->configCache['recordErrors'] === false @@ -1114,13 +1175,37 @@ public function getWarningCount() */ public function getFixableCount() { - return $this->fixableCount; + return ($this->fixableErrorCount + $this->fixableWarningCount); }//end getFixableCount() /** - * Returns the number of fixed errors/warnings. + * Returns the number of fixable errors raised. + * + * @return int + */ + public function getFixableErrorCount() + { + return $this->fixableErrorCount; + + }//end getFixableErrorCount() + + + /** + * Returns the number of fixable warnings raised. + * + * @return int + */ + public function getFixableWarningCount() + { + return $this->fixableWarningCount; + + }//end getFixableWarningCount() + + + /** + * Returns the actual number of fixed errors/warnings. * * @return int */ @@ -1131,6 +1216,30 @@ public function getFixedCount() }//end getFixedCount() + /** + * Returns the effective number of fixed errors. + * + * @return int + */ + public function getFixedErrorCount() + { + return $this->fixedErrorCount; + + }//end getFixedErrorCount() + + + /** + * Returns the effective number of fixed warnings. + * + * @return int + */ + public function getFixedWarningCount() + { + return $this->fixedWarningCount; + + }//end getFixedWarningCount() + + /** * Returns the list of ignored lines. * diff --git a/src/Files/LocalFile.php b/src/Files/LocalFile.php index 140d6be910..db93ac1f13 100644 --- a/src/Files/LocalFile.php +++ b/src/Files/LocalFile.php @@ -106,9 +106,10 @@ public function process() $this->replayErrors($cache['errors'], $cache['warnings']); $this->configCache['cache'] = true; } else { - $this->errorCount = $cache['errorCount']; - $this->warningCount = $cache['warningCount']; - $this->fixableCount = $cache['fixableCount']; + $this->errorCount = $cache['errorCount']; + $this->warningCount = $cache['warningCount']; + $this->fixableErrorCount = $cache['fixableErrorCount']; + $this->fixableWarningCount = $cache['fixableWarningCount']; } if (PHP_CODESNIFFER_VERBOSITY > 0 @@ -129,14 +130,15 @@ public function process() parent::process(); $cache = [ - 'hash' => $hash, - 'errors' => $this->errors, - 'warnings' => $this->warnings, - 'metrics' => $this->metrics, - 'errorCount' => $this->errorCount, - 'warningCount' => $this->warningCount, - 'fixableCount' => $this->fixableCount, - 'numTokens' => $this->numTokens, + 'hash' => $hash, + 'errors' => $this->errors, + 'warnings' => $this->warnings, + 'metrics' => $this->metrics, + 'errorCount' => $this->errorCount, + 'warningCount' => $this->warningCount, + 'fixableErrorCount' => $this->fixableErrorCount, + 'fixableWarningCount' => $this->fixableWarningCount, + 'numTokens' => $this->numTokens, ]; Cache::set($this->path, $cache); @@ -166,11 +168,12 @@ public function process() */ private function replayErrors($errors, $warnings) { - $this->errors = []; - $this->warnings = []; - $this->errorCount = 0; - $this->warningCount = 0; - $this->fixableCount = 0; + $this->errors = []; + $this->warnings = []; + $this->errorCount = 0; + $this->warningCount = 0; + $this->fixableErrorCount = 0; + $this->fixableWarningCount = 0; $this->replayingErrors = true; diff --git a/src/Fixer.php b/src/Fixer.php index 6e804c0a20..4a47c8cf67 100644 --- a/src/Fixer.php +++ b/src/Fixer.php @@ -102,7 +102,12 @@ class Fixer private $inConflict = false; /** - * The number of fixes that have been performed. + * The actual number of fixes that have been performed. + * + * I.e. how many fixes were applied. This may be higher than the originally found + * issues if the fixer from one sniff causes other sniffs to come into play in follow-up loops. + * Example: if a brace is moved to a new line, the `ScopeIndent` sniff may need to ensure + * the brace is indented correctly in the next loop. * * @var integer */ @@ -336,7 +341,7 @@ public function generateDiff($filePath=null, $colors=true) /** - * Get a count of fixes that have been performed on the file. + * Get a count of the actual number of fixes that have been performed on the file. * * This value is reset every time a new file is started, or an existing * file is restarted. diff --git a/src/Reporter.php b/src/Reporter.php index 8f230d635f..98bb8a82a7 100644 --- a/src/Reporter.php +++ b/src/Reporter.php @@ -16,6 +16,12 @@ use PHP_CodeSniffer\Util\Common; use PHP_CodeSniffer\Util\ExitCode; +/** + * Manages reporting of errors and warnings. + * + * @property-read int $totalFixable Total number of errors/warnings that can be fixed. + * @property-read int $totalFixed Total number of errors/warnings that were fixed. + */ class Reporter { @@ -48,18 +54,32 @@ class Reporter public $totalWarnings = 0; /** - * Total number of errors/warnings that can be fixed. + * Total number of errors that can be fixed. * * @var integer */ - public $totalFixable = 0; + public $totalFixableErrors = 0; /** - * Total number of errors/warnings that were fixed. + * Total number of warnings that can be fixed. * * @var integer */ - public $totalFixed = 0; + public $totalFixableWarnings = 0; + + /** + * Total number of errors that were fixed. + * + * @var integer + */ + public $totalFixedErrors = 0; + + /** + * Total number of warnings that were fixed. + * + * @var integer + */ + public $totalFixedWarnings = 0; /** * A cache of report objects. @@ -160,6 +180,81 @@ public function __construct(Config $config) }//end __construct() + /** + * Check whether a (virtual) property is set. + * + * @param string $name Property name. + * + * @return bool + */ + public function __isset($name) + { + return ($name === 'totalFixable' || $name === 'totalFixed'); + + }//end __isset() + + + /** + * Get the value of an inaccessible property. + * + * The properties supported via this method are both deprecated since PHP_CodeSniffer 4.0. + * - For $totalFixable, use `($reporter->totalFixableErrors + $reporter->totalFixableWarnings)` instead. + * - For $totalFixed, use `($reporter->totalFixedErrors + $reporter->totalFixedWarnings)` instead. + * + * @param string $name The name of the property. + * + * @return int + * + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException If the setting name is invalid. + */ + public function __get($name) + { + if ($name === 'totalFixable') { + return ($this->totalFixableErrors + $this->totalFixableWarnings); + } + + if ($name === 'totalFixed') { + return ($this->totalFixedErrors + $this->totalFixedWarnings); + } + + throw new RuntimeException("ERROR: access requested to unknown property \"Reporter::\${$name}\""); + + }//end __get() + + + /** + * Setting a dynamic/virtual property on this class is not allowed. + * + * @param string $name Property name. + * @param mixed $value Property value. + * + * @return bool + * + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException + */ + public function __set($name, $value) + { + throw new RuntimeException("ERROR: setting property \"Reporter::\${$name}\" is not allowed"); + + }//end __set() + + + /** + * Unsetting a dynamic/virtual property on this class is not allowed. + * + * @param string $name Property name. + * + * @return bool + * + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException + */ + public function __unset($name) + { + throw new RuntimeException("ERROR: unsetting property \"Reporter::\${$name}\" is not allowed"); + + }//end __unset() + + /** * Generates and prints final versions of all reports. * @@ -220,7 +315,7 @@ public function printReport($report) $this->totalFiles, $this->totalErrors, $this->totalWarnings, - $this->totalFixable, + ($this->totalFixableErrors + $this->totalFixableWarnings), $this->config->showSources, $this->config->reportWidth, $this->config->interactive, @@ -307,12 +402,10 @@ public function cacheFileReport(File $phpcsFile) // When PHPCBF is running, we need to use the fixable error values // after the report has run and fixed what it can. - if (PHP_CODESNIFFER_CBF === true) { - $this->totalFixable += $phpcsFile->getFixableCount(); - $this->totalFixed += $phpcsFile->getFixedCount(); - } else { - $this->totalFixable += $reportData['fixable']; - } + $this->totalFixableErrors += $phpcsFile->getFixableErrorCount(); + $this->totalFixableWarnings += $phpcsFile->getFixableWarningCount(); + $this->totalFixedErrors += $phpcsFile->getFixedErrorCount(); + $this->totalFixedWarnings += $phpcsFile->getFixedWarningCount(); } }//end cacheFileReport() diff --git a/src/Runner.php b/src/Runner.php index 169fe30ab0..35877b0623 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -118,7 +118,7 @@ public function runPHPCS() $this->config->cache = false; } - $numErrors = $this->run(); + $this->run(); // Print all the reports for this run. $this->reporter->printReports(); @@ -140,16 +140,7 @@ public function runPHPCS() return $exitCode; }//end try - if ($numErrors === 0) { - // No errors found. - return 0; - } else if ($this->reporter->totalFixable === 0) { - // Errors found, but none of them can be fixed by PHPCBF. - return 1; - } else { - // Errors found, and some can be fixed by PHPCBF. - return 2; - } + return ExitCode::calculate($this->reporter); }//end runPHPCS() @@ -235,24 +226,7 @@ public function runPHPCBF() return $exitCode; }//end try - if ($this->reporter->totalFixed === 0) { - // Nothing was fixed by PHPCBF. - if ($this->reporter->totalFixable === 0) { - // Nothing found that could be fixed. - return 0; - } else { - // Something failed to fix. - return 2; - } - } - - if ($this->reporter->totalFixable === 0) { - // PHPCBF fixed all fixable errors. - return 1; - } - - // PHPCBF fixed some fixable errors, but others failed to fix. - return 2; + return ExitCode::calculate($this->reporter); }//end runPHPCBF() @@ -319,7 +293,8 @@ public function init() /** * Performs the run. * - * @return int The number of errors and warnings found. + * @return void + * * @throws \PHP_CodeSniffer\Exceptions\DeepExitException * @throws \PHP_CodeSniffer\Exceptions\RuntimeException */ @@ -453,11 +428,13 @@ private function run() // Reset the reporter to make sure only figures from this // file batch are recorded. - $this->reporter->totalFiles = 0; - $this->reporter->totalErrors = 0; - $this->reporter->totalWarnings = 0; - $this->reporter->totalFixable = 0; - $this->reporter->totalFixed = 0; + $this->reporter->totalFiles = 0; + $this->reporter->totalErrors = 0; + $this->reporter->totalWarnings = 0; + $this->reporter->totalFixableErrors = 0; + $this->reporter->totalFixableWarnings = 0; + $this->reporter->totalFixedErrors = 0; + $this->reporter->totalFixedWarnings = 0; // Process the files. $pathsProcessed = []; @@ -492,11 +469,13 @@ private function run() // Write information about the run to the filesystem // so it can be picked up by the main process. $childOutput = [ - 'totalFiles' => $this->reporter->totalFiles, - 'totalErrors' => $this->reporter->totalErrors, - 'totalWarnings' => $this->reporter->totalWarnings, - 'totalFixable' => $this->reporter->totalFixable, - 'totalFixed' => $this->reporter->totalFixed, + 'totalFiles' => $this->reporter->totalFiles, + 'totalErrors' => $this->reporter->totalErrors, + 'totalWarnings' => $this->reporter->totalWarnings, + 'totalFixableErrors' => $this->reporter->totalFixableErrors, + 'totalFixableWarnings' => $this->reporter->totalFixableWarnings, + 'totalFixedErrors' => $this->reporter->totalFixedErrors, + 'totalFixedWarnings' => $this->reporter->totalFixedWarnings, ]; $output = '<'.'?php'."\n".' $childOutput = '; @@ -539,26 +518,6 @@ private function run() Cache::save(); } - $ignoreWarnings = Config::getConfigData('ignore_warnings_on_exit'); - $ignoreErrors = Config::getConfigData('ignore_errors_on_exit'); - - $return = ($this->reporter->totalErrors + $this->reporter->totalWarnings); - if ($ignoreErrors !== null) { - $ignoreErrors = (bool) $ignoreErrors; - if ($ignoreErrors === true) { - $return -= $this->reporter->totalErrors; - } - } - - if ($ignoreWarnings !== null) { - $ignoreWarnings = (bool) $ignoreWarnings; - if ($ignoreWarnings === true) { - $return -= $this->reporter->totalWarnings; - } - } - - return $return; - }//end run() @@ -616,8 +575,9 @@ public function processFile($file) StatusWriter::write('DONE in '.Timing::getHumanReadableDuration(Timing::getDurationSince($startTime)), 0, 0); if (PHP_CODESNIFFER_CBF === true) { - $errors = $file->getFixableCount(); - StatusWriter::write(" ($errors fixable violations)"); + $errors = $file->getFixableErrorCount(); + $warnings = $file->getFixableWarningCount(); + StatusWriter::write(" ($errors fixable errors, $warnings fixable warnings)"); } else { $errors = $file->getErrorCount(); $warnings = $file->getWarningCount(); @@ -758,17 +718,19 @@ private function processChildProcs($childProcs) if (isset($childOutput) === false) { // The child process died, so the run has failed. $file = new DummyFile('', $this->ruleset, $this->config); - $file->setErrorCounts(1, 0, 0, 0); + $file->setErrorCounts(1, 0, 0, 0, 0, 0); $this->printProgress($file, $totalBatches, $numProcessed); $success = false; continue; } - $this->reporter->totalFiles += $childOutput['totalFiles']; - $this->reporter->totalErrors += $childOutput['totalErrors']; - $this->reporter->totalWarnings += $childOutput['totalWarnings']; - $this->reporter->totalFixable += $childOutput['totalFixable']; - $this->reporter->totalFixed += $childOutput['totalFixed']; + $this->reporter->totalFiles += $childOutput['totalFiles']; + $this->reporter->totalErrors += $childOutput['totalErrors']; + $this->reporter->totalWarnings += $childOutput['totalWarnings']; + $this->reporter->totalFixableErrors += $childOutput['totalFixableErrors']; + $this->reporter->totalFixableWarnings += $childOutput['totalFixableWarnings']; + $this->reporter->totalFixedErrors += $childOutput['totalFixedErrors']; + $this->reporter->totalFixedWarnings += $childOutput['totalFixedWarnings']; if (isset($debugOutput) === true) { echo $debugOutput; @@ -785,8 +747,10 @@ private function processChildProcs($childProcs) $file->setErrorCounts( $childOutput['totalErrors'], $childOutput['totalWarnings'], - $childOutput['totalFixable'], - $childOutput['totalFixed'] + $childOutput['totalFixableErrors'], + $childOutput['totalFixableWarnings'], + $childOutput['totalFixedErrors'], + $childOutput['totalFixedWarnings'] ); $this->printProgress($file, $totalBatches, $numProcessed); }//end while diff --git a/src/Util/ExitCode.php b/src/Util/ExitCode.php index 62cee53b42..b4ede3f114 100644 --- a/src/Util/ExitCode.php +++ b/src/Util/ExitCode.php @@ -17,6 +17,9 @@ namespace PHP_CodeSniffer\Util; +use PHP_CodeSniffer\Config; +use PHP_CodeSniffer\Reporter; + final class ExitCode { @@ -76,4 +79,67 @@ final class ExitCode public const REQUIREMENTS_NOT_MET = 64; + /** + * Calculate the exit code based on the results of the run as recorded in the Reporter object. + * + * @param \PHP_CodeSniffer\Reporter $reporter Reporter object for the run. + * + * @return int + */ + public static function calculate(Reporter $reporter) + { + // First figure out what the relevant numbers are on which we need to base the exit code. + $ignoreNonAutofixable = (bool) (Config::getConfigData('ignore_non_auto_fixable_on_exit') ?? false); + $totalRelevantErrors = $reporter->totalErrors; + $totalRelevantWarnings = $reporter->totalWarnings; + + if ($ignoreNonAutofixable === true) { + $totalRelevantErrors = $reporter->totalFixableErrors; + $totalRelevantWarnings = $reporter->totalFixableWarnings; + } + + $ignoreErrors = (bool) (Config::getConfigData('ignore_errors_on_exit') ?? false); + $ignoreWarnings = (bool) (Config::getConfigData('ignore_warnings_on_exit') ?? false); + + $totalRelevantIssues = 0; + $totalRelevantFixableIssues = 0; + $totalRelevantFixedIssues = 0; + + if ($ignoreErrors === false) { + $totalRelevantIssues += $totalRelevantErrors; + $totalRelevantFixableIssues += $reporter->totalFixableErrors; + $totalRelevantFixedIssues += $reporter->totalFixedErrors; + } + + if ($ignoreWarnings === false) { + $totalRelevantIssues += $totalRelevantWarnings; + $totalRelevantFixableIssues += $reporter->totalFixableWarnings; + $totalRelevantFixedIssues += $reporter->totalFixedWarnings; + } + + // Next figure out what the exit code should be. + $exitCode = self::OKAY; + + if (PHP_CODESNIFFER_CBF === true + && ($reporter->totalFixableErrors + $reporter->totalFixableWarnings) > 0 + ) { + // Something failed to fix. + $exitCode |= self::FAILED_TO_FIX; + } + + // Are there issues which PHPCBF could fix ? + if ($totalRelevantFixableIssues > 0) { + $exitCode |= self::FIXABLE; + } + + // Are there issues which PHPCBF cannot fix ? + if (($totalRelevantIssues - $totalRelevantFixableIssues - $totalRelevantFixedIssues) > 0) { + $exitCode |= self::NON_FIXABLE; + } + + return $exitCode; + + }//end calculate() + + }//end class diff --git a/src/Util/Help.php b/src/Util/Help.php index 8605dcd361..f58daa349d 100644 --- a/src/Util/Help.php +++ b/src/Util/Help.php @@ -573,7 +573,7 @@ private function getAllOptions() 'config-explain' => [ 'text' => 'Default values for a selection of options can be stored in a user-specific CodeSniffer.conf configuration file.'."\n" - .'This applies to the following options: "default_standard", "report_format", "tab_width", "encoding", "severity", "error_severity", "warning_severity", "show_warnings", "report_width", "show_progress", "quiet", "colors", "cache", "parallel", "installed_paths", "php_version", "ignore_errors_on_exit", "ignore_warnings_on_exit".', + .'This applies to the following options: "default_standard", "report_format", "tab_width", "encoding", "severity", "error_severity", "warning_severity", "show_warnings", "report_width", "show_progress", "quiet", "colors", "cache", "parallel", "installed_paths", "php_version", "ignore_errors_on_exit", "ignore_warnings_on_exit", "ignore_non_auto_fixable_on_exit".', ], 'config-show' => [ 'argument' => '--config-show', diff --git a/tests/Core/Reporter/MagicMethodsTest.php b/tests/Core/Reporter/MagicMethodsTest.php new file mode 100644 index 0000000000..d0d3f0a471 --- /dev/null +++ b/tests/Core/Reporter/MagicMethodsTest.php @@ -0,0 +1,150 @@ +assertFalse(isset($reporter->unknown)); + + }//end testMagicIssetReturnsFalseForUnknownProperty() + + + /** + * Verify the magic __get() method rejects requests for anything but the explicitly supported properties. + * + * @return void + */ + public function testMagicGetThrowsExceptionForUnsupportedProperty() + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('ERROR: access requested to unknown property "Reporter::$invalid"'); + + (new Reporter(new ConfigDouble()))->invalid; + + }//end testMagicGetThrowsExceptionForUnsupportedProperty() + + + /** + * Test the magic __get() method handles supported properties. + * + * @param string $propertyName The name of the property to request. + * @param int $set Value to set for the properties comprising the virtual ones. + * @param int $expectedValue The expected value for the property. + * + * @dataProvider dataMagicGetReturnsValueForSupportedProperty + * + * @return void + */ + public function testMagicGetReturnsValueForSupportedProperty($propertyName, $set, $expectedValue) + { + $reporter = new Reporter(new ConfigDouble()); + + if ($set !== 0) { + $reporter->totalFixableErrors = $set; + $reporter->totalFixableWarnings = $set; + $reporter->totalFixedErrors = $set; + $reporter->totalFixedWarnings = $set; + } + + $this->assertTrue(isset($reporter->$propertyName)); + $this->assertSame($expectedValue, $reporter->$propertyName); + + }//end testMagicGetReturnsValueForSupportedProperty() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataMagicGetReturnsValueForSupportedProperty() + { + return [ + 'Property: totalFixable - 0' => [ + 'propertyName' => 'totalFixable', + 'set' => 0, + 'expectedValue' => 0, + ], + 'Property: totalFixed - 0' => [ + 'propertyName' => 'totalFixed', + 'set' => 0, + 'expectedValue' => 0, + ], + 'Property: totalFixable - 2' => [ + 'propertyName' => 'totalFixable', + 'set' => 1, + 'expectedValue' => 2, + ], + 'Property: totalFixed - 4' => [ + 'propertyName' => 'totalFixed', + 'set' => 2, + 'expectedValue' => 4, + ], + ]; + + }//end dataMagicGetReturnsValueForSupportedProperty() + + + /** + * Verify the magic __set() method rejects everything. + * + * @return void + */ + public function testMagicSetThrowsException() + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('ERROR: setting property "Reporter::$totalFixable" is not allowed'); + + $reporter = new Reporter(new ConfigDouble()); + $reporter->totalFixable = 10; + + }//end testMagicSetThrowsException() + + + /** + * Verify the magic __unset() method rejects everything. + * + * @return void + */ + public function testMagicUnsetThrowsException() + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('ERROR: unsetting property "Reporter::$totalFixed" is not allowed'); + + $reporter = new Reporter(new ConfigDouble()); + unset($reporter->totalFixed); + + }//end testMagicUnsetThrowsException() + + +}//end class diff --git a/tests/Core/Util/ExitCode/ExitCodeTest.php b/tests/Core/Util/ExitCode/ExitCodeTest.php new file mode 100644 index 0000000000..ea9ef05309 --- /dev/null +++ b/tests/Core/Util/ExitCode/ExitCodeTest.php @@ -0,0 +1,673 @@ +redirectStatusWriterOutputToStream(); + + // Reset static properties on the Config class. + AbstractRunnerTestCase::setUp(); + + }//end setUp() + + + /** + * Clean up after the test. + * + * @return void + */ + protected function tearDown(): void + { + // Reset all static properties on the StatusWriter class. + $this->resetStatusWriterProperties(); + + // Reset $_SERVER['argv'] between tests. + AbstractRunnerTestCase::tearDown(); + + // Delete the cache file between tests to prevent a cache from an earlier test run influencing the results of the tests. + @unlink(self::CACHE_FILE); + + }//end tearDown() + + + /** + * Clean up after the tests. + * + * @return void + */ + public static function tearDownAfterClass(): void + { + $globPattern = self::SOURCE_DIR.'/*.inc.fixed'; + $globPattern = str_replace('/', DIRECTORY_SEPARATOR, $globPattern); + + $fixedFiles = glob($globPattern, GLOB_NOESCAPE); + + if (is_array($fixedFiles) === true) { + foreach ($fixedFiles as $file) { + @unlink($file); + } + } + + AbstractRunnerTestCase::tearDownAfterClass(); + + }//end tearDownAfterClass() + + + /** + * Verify generated exit codes (PHPCS). + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @dataProvider dataPhpcs + * + * @return void + */ + public function testPhpcsNoParallel($extraArgs, $expected) + { + $extraArgs[] = self::SOURCE_DIR.'mix-errors-warnings.inc'; + + $this->runPhpcsAndCheckExitCode($extraArgs, $expected); + + }//end testPhpcsNoParallel() + + + /** + * Verify generated exit codes (PHPCS) when using parallel processing. + * + * Note: there tests are skipped on Windows as the PCNTL extension needed for parallel processing + * isn't available on Windows. + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @dataProvider dataPhpcs + * @requires OS ^(?!WIN).* + * + * @return void + */ + public function testPhpcsParallel($extraArgs, $expected) + { + // Deliberately using `parallel=3` to scan 5 files to make sure that the results + // from multiple batches get recorded and added correctly. + $extraArgs[] = self::SOURCE_DIR; + $extraArgs[] = '--parallel=3'; + + $this->runPhpcsAndCheckExitCode($extraArgs, $expected); + + }//end testPhpcsParallel() + + + /** + * Verify generated exit codes (PHPCS) when caching of results is used. + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @dataProvider dataPhpcs + * + * @return void + */ + public function testPhpcsWithCache($extraArgs, $expected) + { + $extraArgs[] = self::SOURCE_DIR.'mix-errors-warnings.inc'; + $extraArgs[] = '--cache='.self::CACHE_FILE; + + // Make sure we start without a cache. + if (method_exists($this, 'assertFileDoesNotExist') === true) { + $this->assertFileDoesNotExist(self::CACHE_FILE); + } else { + // PHPUnit < 9.1.0. + $this->assertFileNotExists(self::CACHE_FILE); + } + + // First run with these arguments to create the cache. + $this->runPhpcsAndCheckExitCode($extraArgs, $expected); + + // Check that the cache file was created. + $this->assertFileExists(self::CACHE_FILE); + + // Second run to verify the exit code is the same when the results are taking from the cache. + $this->runPhpcsAndCheckExitCode($extraArgs, $expected); + + }//end testPhpcsWithCache() + + + /** + * Test Helper: run PHPCS and verify the generated exit code. + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @return void + */ + private function runPhpcsAndCheckExitCode($extraArgs, $expected) + { + if (PHP_CODESNIFFER_CBF === true) { + $this->markTestSkipped('This test needs CS mode to run'); + } + + $this->setServerArgs('phpcs', $extraArgs); + + // Catch & discard the screen output. That's not what we're interested in for this test. + ob_start(); + $runner = new Runner(); + $actual = $runner->runPHPCS(); + ob_end_clean(); + + $this->assertSame($expected, $actual); + + }//end runPhpcsAndCheckExitCode() + + + /** + * Data provider. + * + * @return array>> + */ + public static function dataPhpcs() + { + return [ + 'No issues' => [ + 'extraArgs' => ['--sniffs=TestStandard.ExitCodes.NoIssues'], + 'expected' => 0, + ], + 'Only auto-fixable issues' => [ + 'extraArgs' => ['--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.FixableWarning'], + 'expected' => 1, + ], + 'Only non-fixable issues' => [ + 'extraArgs' => ['--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.Warning'], + 'expected' => 2, + ], + 'Both auto-fixable + non-fixable issues' => [ + 'extraArgs' => [], + 'expected' => 3, + ], + + // In both the below cases, we still have both fixable and non-fixable issues, so exit code = 3. + 'Only errors' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.FixableWarning,TestStandard.ExitCodes.Warning'], + 'expected' => 3, + ], + 'Only warnings' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Error'], + 'expected' => 3, + ], + + // In both the below cases, we still have 1 fixable and 1 non-fixable issue which we need to take into account, so exit code = 3. + 'Mix of errors and warnings, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 3, + ], + 'Mix of errors and warnings, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 3, + ], + + 'Mix of errors and warnings, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 1, + ], + 'Mix of errors and warnings, ignoring errors + non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_errors_on_exit', + '1', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 1, + ], + 'Mix of errors and warnings, ignoring warnings + non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 1, + ], + 'Mix of errors and warnings, ignoring errors + warnings for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_errors_on_exit', + '1', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Mix of errors and warnings, explicitly ignoring nothing for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_errors_on_exit', + '0', + '--runtime-set', + 'ignore_warnings_on_exit', + '0', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '0', + ], + 'expected' => 3, + ], + + 'Fixable error and non-fixable warning, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 2, + ], + 'Non-fixable error and fixable warning, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 1, + ], + + 'Fixable error and non-fixable warning, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 1, + ], + 'Non-fixable error and fixable warning, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 2, + ], + + 'Fixable error and non-fixable warning, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 1, + ], + 'Non-fixable error and fixable warning, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 1, + ], + ]; + + }//end dataPhpcs() + + + /** + * Verify generated exit codes (PHPCBF). + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @dataProvider dataPhpcbf + * @group CBF + * + * @return void + */ + public function testPhpcbfNoParallel($extraArgs, $expected) + { + $extraArgs[] = self::SOURCE_DIR.'mix-errors-warnings.inc'; + + $this->runPhpcbfAndCheckExitCode($extraArgs, $expected); + + }//end testPhpcbfNoParallel() + + + /** + * Verify generated exit codes (PHPCBF) when using parallel processing. + * + * Note: there tests are skipped on Windows as the PCNTL extension needed for parallel processing + * isn't available on Windows. + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @dataProvider dataPhpcbf + * @group CBF + * @requires OS ^(?!WIN).* + * + * @return void + */ + public function testPhpcbfParallel($extraArgs, $expected) + { + // Deliberately using `parallel=3` to scan 5 files to make sure that the results + // from multiple batches get recorded and added correctly. + $extraArgs[] = self::SOURCE_DIR; + $extraArgs[] = '--parallel=3'; + + $this->runPhpcbfAndCheckExitCode($extraArgs, $expected); + + }//end testPhpcbfParallel() + + + /** + * Test Helper: run PHPCBF and verify the generated exit code. + * + * @param array $extraArgs Any extra arguments to pass on the command line. + * @param int $expected The expected exit code. + * + * @return void + */ + private function runPhpcbfAndCheckExitCode($extraArgs, $expected) + { + if (PHP_CODESNIFFER_CBF === false) { + $this->markTestSkipped('This test needs CBF mode to run'); + } + + $this->setServerArgs('phpcbf', $extraArgs); + + // Catch & discard the screen output. That's not what we're interested in for this test. + ob_start(); + $runner = new Runner(); + $actual = $runner->runPHPCBF(); + ob_end_clean(); + + $this->assertSame($expected, $actual); + + }//end runPhpcbfAndCheckExitCode() + + + /** + * Data provider. + * + * @return array>> + */ + public static function dataPhpcbf() + { + return [ + 'No issues' => [ + 'extraArgs' => ['--sniffs=TestStandard.ExitCodes.NoIssues'], + 'expected' => 0, + ], + 'Fixed all auto-fixable issues, no issues left' => [ + 'extraArgs' => ['--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.FixableWarning'], + 'expected' => 0, + ], + 'Fixed all auto-fixable issues, has non-autofixable issues left' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.FailToFix'], + 'expected' => 2, + ], + 'Fixed all auto-fixable issues, has non-autofixable issues left, ignoring those for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Failed to fix, only fixable issues remaining' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.Warning'], + 'expected' => 5, + ], + 'Failed to fix, both fixable + non-fixable issues remaining' => [ + 'extraArgs' => [], + 'expected' => 7, + ], + 'Failed to fix, both fixable + non-fixable issues remaining, ignoring non-fixable for exit code' => [ + 'extraArgs' => [ + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 5, + ], + + // In both the below cases, we still have 1 non-fixable issue which we need to take into account, so exit code = 2. + 'Only errors' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.FailToFix,TestStandard.ExitCodes.FixableWarning,TestStandard.ExitCodes.Warning'], + 'expected' => 2, + ], + 'Only warnings' => [ + 'extraArgs' => ['--exclude=TestStandard.ExitCodes.FailToFix,TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Error'], + 'expected' => 2, + ], + + // In both the below cases, we still have 1 non-fixable issue which we need to take into account, so exit code = 2. + 'Mix of errors and warnings, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 2, + ], + 'Mix of errors and warnings, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 2, + ], + + 'Mix of errors and warnings, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Mix of errors and warnings, ignoring errors + non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Mix of errors and warnings, ignoring warnings + non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Mix of errors and warnings, ignoring errors + warnings for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Mix of errors and warnings, explicitly ignoring nothing for exit code' => [ + 'extraArgs' => [ + '--exclude=TestStandard.ExitCodes.FailToFix', + '--runtime-set', + 'ignore_errors_on_exit', + '0', + '--runtime-set', + 'ignore_warnings_on_exit', + '0', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '0', + ], + 'expected' => 2, + ], + + 'Fixable error and non-fixable warning, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 2, + ], + 'Non-fixable error and fixable warning, ignoring errors for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_errors_on_exit', + '1', + ], + 'expected' => 0, + ], + + 'Fixable error and non-fixable warning, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Non-fixable error and fixable warning, ignoring warnings for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_warnings_on_exit', + '1', + ], + 'expected' => 2, + ], + + 'Fixable error and non-fixable warning, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.FixableError,TestStandard.ExitCodes.Warning', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + 'Non-fixable error and fixable warning, ignoring non-auto-fixable for exit code' => [ + 'extraArgs' => [ + '--sniffs=TestStandard.ExitCodes.Error,TestStandard.ExitCodes.FixableWarning', + '--runtime-set', + 'ignore_non_auto_fixable_on_exit', + '1', + ], + 'expected' => 0, + ], + ]; + + }//end dataPhpcbf() + + + /** + * Helper method to prepare the CLI arguments for a test. + * + * @param string $cmd The command. Either 'phpcs' or 'phpcbf'. + * @param array $extraArgs Any extra arguments to pass on the command line. + * + * @return void + */ + private function setServerArgs($cmd, $extraArgs) + { + $standard = __DIR__.'/ExitCodeTest.xml'; + + $_SERVER['argv'] = [ + $cmd, + "--standard=$standard", + '--report-width=80', + '--suffix=.fixed', + ]; + + foreach ($extraArgs as $arg) { + $_SERVER['argv'][] = $arg; + } + + }//end setServerArgs() + + +}//end class diff --git a/tests/Core/Util/ExitCode/ExitCodeTest.xml b/tests/Core/Util/ExitCode/ExitCodeTest.xml new file mode 100644 index 0000000000..6a9a347830 --- /dev/null +++ b/tests/Core/Util/ExitCode/ExitCodeTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/mix-errors-warnings.inc b/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/mix-errors-warnings.inc new file mode 100644 index 0000000000..8e5025a909 --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/mix-errors-warnings.inc @@ -0,0 +1,6 @@ +getTokens(); + if ($tokens[$stackPtr]['content'] === '$var') { + $phpcsFile->addError('Variables should have descriptive names. Found: $var', $stackPtr, 'VarFound'); + } + } +} diff --git a/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FailToFixSniff.php b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FailToFixSniff.php new file mode 100644 index 0000000000..7c3ee78134 --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FailToFixSniff.php @@ -0,0 +1,38 @@ +getTokens(); + + if ($tokens[($stackPtr + 1)]['code'] !== T_WHITESPACE + || $tokens[($stackPtr + 1)]['length'] > 60 + ) { + return; + } + + $error = 'There should be 60 spaces after an ECHO keyword'; + $fix = $phpcsFile->addFixableError($error, ($stackPtr + 1), 'ShortSpace'); + if ($fix === true) { + // The fixer deliberately only adds one space in each loop to ensure it runs out of loops before the file complies. + $phpcsFile->fixer->addContent($stackPtr, ' '); + } + } +} diff --git a/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableErrorSniff.php b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableErrorSniff.php new file mode 100644 index 0000000000..f3d4ef20b9 --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableErrorSniff.php @@ -0,0 +1,33 @@ +getTokens(); + $contents = $tokens[$stackPtr]['content']; + $contentsLC = strtolower($contents); + if ($contentsLC !== $contents) { + $fix = $phpcsFile->addFixableError('Use lowercase open tag', $stackPtr, 'Found'); + if ($fix === true) { + $phpcsFile->fixer->replaceToken($stackPtr, $contentsLC); + } + } + } +} diff --git a/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableWarningSniff.php b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableWarningSniff.php new file mode 100644 index 0000000000..21522302ad --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/FixableWarningSniff.php @@ -0,0 +1,31 @@ +getTokens(); + if ($tokens[($stackPtr - 1)]['code'] === T_WHITESPACE) { + $fix = $phpcsFile->addFixableWarning('There should be no whitespace before a semicolon', ($stackPtr - 1), 'Found'); + if ($fix === true) { + $phpcsFile->fixer->replaceToken(($stackPtr - 1), ''); + } + } + } +} diff --git a/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/NoIssuesSniff.php b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/NoIssuesSniff.php new file mode 100644 index 0000000000..5f03652652 --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/TestStandard/Sniffs/ExitCodes/NoIssuesSniff.php @@ -0,0 +1,25 @@ +addWarning('Commments are not allowed', $stackPtr, 'Found'); + } +} diff --git a/tests/Core/Util/ExitCode/Fixtures/TestStandard/ruleset.xml b/tests/Core/Util/ExitCode/Fixtures/TestStandard/ruleset.xml new file mode 100644 index 0000000000..56afad6e29 --- /dev/null +++ b/tests/Core/Util/ExitCode/Fixtures/TestStandard/ruleset.xml @@ -0,0 +1,4 @@ + + + + From de26471137f077b40e9dbed43a0f4b886d283e63 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 7 May 2025 05:46:28 +0200 Subject: [PATCH 5/5] Use the new exit code when scanning code from STDIN For now, always return a zero-exit code when the fixer runs on code provided via STDIN. This should be re-evaluated at a later time to see if we can get this to use the `ExitCode::calculate()` method as well. --- src/Reports/Cbf.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Reports/Cbf.php b/src/Reports/Cbf.php index 9d908cf1ed..823ed14a57 100644 --- a/src/Reports/Cbf.php +++ b/src/Reports/Cbf.php @@ -15,6 +15,7 @@ use PHP_CodeSniffer\Exceptions\DeepExitException; use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Util\ExitCode; use PHP_CodeSniffer\Util\Timing; use PHP_CodeSniffer\Util\Writers\StatusWriter; @@ -60,7 +61,7 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, // even if nothing was fixed. Exit here because we // can't process any more than 1 file in this setup. $fixedContent = $phpcsFile->fixer->getContents(); - throw new DeepExitException($fixedContent, 1); + throw new DeepExitException($fixedContent, ExitCode::OKAY); } if ($errors === 0) {