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/.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/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) { 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/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/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/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 78134bda71..98bb8a82a7 100644 --- a/src/Reporter.php +++ b/src/Reporter.php @@ -14,7 +14,14 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Reports\Report; 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 { @@ -47,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. @@ -103,7 +124,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 +153,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(); @@ -159,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. * @@ -219,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, @@ -306,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/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) { 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..35877b0623 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; @@ -117,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(); @@ -139,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() @@ -234,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() @@ -278,7 +253,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 +284,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() @@ -318,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 */ @@ -348,7 +324,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 +356,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. @@ -452,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 = []; @@ -491,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 = '; @@ -538,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() @@ -615,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(); @@ -694,7 +655,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 @@ -756,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; @@ -783,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 new file mode 100644 index 0000000000..b4ede3f114 --- /dev/null +++ b/src/Util/ExitCode.php @@ -0,0 +1,145 @@ +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 @@ + + + +