diff --git a/.github/workflows/csqa.yml b/.github/workflows/csqa.yml index ac59f96..641a1a7 100644 --- a/.github/workflows/csqa.yml +++ b/.github/workflows/csqa.yml @@ -32,9 +32,9 @@ jobs: coverage: none tools: cs2pr - # Using PHPCS `master` as an early detection system for bugs upstream. + # Using PHPCS `3.x-dev` as an early detection system for bugs upstream. - name: 'Composer: adjust dependencies' - run: composer require --no-update squizlabs/php_codesniffer:"dev-master" + run: composer require --no-update squizlabs/php_codesniffer:"3.x-dev" # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-composer-dependencies diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6567037..78a03d2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,7 +30,7 @@ jobs: # - PHP 8.0 needs PHPCS 3.5.7+ to run without errors. # - PHP 8.1 needs PHPCS 3.6.1+ to run without errors. php: ['7.1', '7.2', '7.3'] - phpcs_version: ['3.5.6', 'dev-master'] + phpcs_version: ['3.5.6', '3.x-dev'] include: # Make the matrix complete without duplicating builds run in code coverage. @@ -38,16 +38,16 @@ jobs: phpcs_version: '3.6.1' - php: '8.0' - phpcs_version: 'dev-master' + phpcs_version: '3.x-dev' - php: '8.0' phpcs_version: '3.5.7' - php: '7.4' - phpcs_version: 'dev-master' + phpcs_version: '3.x-dev' # Experimental builds. - php: '8.2' # Nightly. - phpcs_version: 'dev-master' + phpcs_version: '3.x-dev' name: "Test: PHP ${{ matrix.php }} on PHPCS ${{ matrix.phpcs_version }}" @@ -62,7 +62,7 @@ jobs: run: | # On stable PHPCS versions, allow for PHP deprecation notices. # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore. - if [ "${{ matrix.phpcs_version }}" != "dev-master" ]; then + if [ "${{ matrix.phpcs_version }}" != "3.x-dev" ]; then echo '::set-output name=PHP_INI::error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On, zend.assertions=1' else echo '::set-output name=PHP_INI::error_reporting=-1, display_errors=On, zend.assertions=1' diff --git a/PhpcsChanged/CacheEntry.php b/PhpcsChanged/CacheEntry.php index 5fbd15b..62b00c2 100644 --- a/PhpcsChanged/CacheEntry.php +++ b/PhpcsChanged/CacheEntry.php @@ -29,6 +29,7 @@ class CacheEntry implements \JsonSerializable { */ public $data; + #[\Override] public function jsonSerialize(): array { return [ 'path' => $this->path, diff --git a/PhpcsChanged/CacheManager.php b/PhpcsChanged/CacheManager.php index 0d037e1..124bd28 100644 --- a/PhpcsChanged/CacheManager.php +++ b/PhpcsChanged/CacheManager.php @@ -49,6 +49,7 @@ public function __construct(CacheInterface $cache, ?callable $debug = null) { $this->cache = $cache; $noopDebug = /** @param string[] $output */ + /** @psalm-suppress UnusedClosureParam, MissingClosureParamType */ function(...$output): void {}; // phpcs:ignore VariableAnalysis $this->debug = $debug ?? $noopDebug; } diff --git a/PhpcsChanged/Cli.php b/PhpcsChanged/Cli.php index f749479..a677746 100644 --- a/PhpcsChanged/Cli.php +++ b/PhpcsChanged/Cli.php @@ -8,6 +8,7 @@ use PhpcsChanged\Reporter; use PhpcsChanged\JsonReporter; use PhpcsChanged\FullReporter; +use PhpcsChanged\JunitReporter; use PhpcsChanged\PhpcsMessages; use PhpcsChanged\ShellException; use PhpcsChanged\ShellOperator; @@ -143,7 +144,7 @@ function printHelp(): void { printTwoColumns([ '--standard ' => 'The phpcs standard to use.', '--extensions ' => 'A comma separated list of extensions to check.', - '--report ' => 'The phpcs reporter to use. One of "full" (default), "json", or "xml".', + '--report ' => 'The phpcs reporter to use. One of "full" (default), "json", "xml", or "junit".', '-s' => 'Show sniff codes for each error when the reporter is "full".', '--ignore ' => 'A comma separated list of patterns to ignore files and directories.', '--warning-severity' => 'The phpcs warning severity to report. See phpcs documentation for usage.', @@ -191,6 +192,8 @@ function getReporter(string $reportType, CliOptions $options, ShellOperator $she return new JsonReporter(); case 'xml': return new XmlReporter($options, $shell); + case 'junit': + return new JunitReporter(); } printErrorAndExit("Unknown Reporter '{$reportType}'"); throw new \Exception("Unknown Reporter '{$reportType}'"); // Just in case we don't exit for some reason. @@ -252,14 +255,18 @@ function runSvnWorkflowForFile(string $svnFile, CliOptions $options, ShellOperat $modifiedFilePhpcsOutput = $cache->getCacheForFile($svnFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); $debug(($modifiedFilePhpcsOutput ? 'Using' : 'Not using') . " cache for modified file '{$svnFile}' at hash '{$modifiedFileHash}', and standard '{$phpcsStandard}'"); } + $modifiedFileTiming = 0.0; if (! $modifiedFilePhpcsOutput) { + $modifiedFileStartTime = microtime(true); $modifiedFilePhpcsOutput = $shell->getPhpcsOutputOfModifiedSvnFile($svnFile); + $modifiedFileTiming = microtime(true) - $modifiedFileStartTime; if (isCachingEnabled($options->toArray())) { $cache->setCacheForFile($svnFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedFilePhpcsOutput); } } $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedFilePhpcsOutput, $fileName); + $modifiedFilePhpcsMessages->setTiming($fileName, $modifiedFileTiming); $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; if (! $hasNewPhpcsMessages) { @@ -279,12 +286,17 @@ function runSvnWorkflowForFile(string $svnFile, CliOptions $options, ShellOperat $unmodifiedFilePhpcsOutput = $cache->getCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); $debug(($unmodifiedFilePhpcsOutput ? 'Using' : 'Not using') . " cache for unmodified file '{$svnFile}' at revision '{$revisionId}', and standard '{$phpcsStandard}'"); } + $unmodifiedFileTiming = 0.0; if (! $unmodifiedFilePhpcsOutput) { + $unmodifiedFileStartTime = microtime(true); $unmodifiedFilePhpcsOutput = $shell->getPhpcsOutputOfUnmodifiedSvnFile($svnFile); + $unmodifiedFileTiming = microtime(true) - $unmodifiedFileStartTime; if (isCachingEnabled($options->toArray())) { $cache->setCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedFilePhpcsOutput); } } + // Add timing for the unmodified scan (accumulated with modified scan time) + $modifiedFileTiming += $unmodifiedFileTiming; } } catch( NoChangesException $err ) { $debug($err->getMessage()); @@ -348,14 +360,18 @@ function runGitWorkflowForFile(string $gitFile, CliOptions $options, ShellOperat $modifiedFilePhpcsOutput = $cache->getCacheForFile($gitFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); $debug(($modifiedFilePhpcsOutput ? 'Using' : 'Not using') . " cache for modified file '{$gitFile}' at hash '{$modifiedFileHash}', and standard '{$phpcsStandard}'"); } + $modifiedFileTiming = 0.0; if (! $modifiedFilePhpcsOutput) { + $modifiedFileStartTime = microtime(true); $modifiedFilePhpcsOutput = $shell->getPhpcsOutputOfModifiedGitFile($gitFile); + $modifiedFileTiming = microtime(true) - $modifiedFileStartTime; if (isCachingEnabled($options->toArray())) { $cache->setCacheForFile($gitFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedFilePhpcsOutput); } } $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedFilePhpcsOutput, $gitFile); + $modifiedFilePhpcsMessages->setTiming($gitFile, $modifiedFileTiming); $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; $unifiedDiff = ''; @@ -378,12 +394,17 @@ function runGitWorkflowForFile(string $gitFile, CliOptions $options, ShellOperat $unmodifiedFilePhpcsOutput = $cache->getCacheForFile($gitFile, 'old', $unmodifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); $debug(($unmodifiedFilePhpcsOutput ? 'Using' : 'Not using') . " cache for unmodified file '{$gitFile}' at hash '{$unmodifiedFileHash}', and standard '{$phpcsStandard}'"); } + $unmodifiedFileTiming = 0.0; if (! $unmodifiedFilePhpcsOutput) { + $unmodifiedFileStartTime = microtime(true); $unmodifiedFilePhpcsOutput = $shell->getPhpcsOutputOfUnmodifiedGitFile($gitFile); + $unmodifiedFileTiming = microtime(true) - $unmodifiedFileStartTime; if (isCachingEnabled($options->toArray())) { $cache->setCacheForFile($gitFile, 'old', $unmodifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedFilePhpcsOutput); } } + // Add timing for the unmodified scan (accumulated with modified scan time) + $modifiedFileTiming += $unmodifiedFileTiming; } } catch( NoChangesException $err ) { $debug($err->getMessage()); diff --git a/PhpcsChanged/FileCache.php b/PhpcsChanged/FileCache.php index e158e1b..701f19a 100644 --- a/PhpcsChanged/FileCache.php +++ b/PhpcsChanged/FileCache.php @@ -15,6 +15,7 @@ class FileCache implements CacheInterface { */ public $cacheFilePath = DEFAULT_CACHE_FILE; + #[\Override] public function load(): CacheObject { if (! file_exists($this->cacheFilePath)) { return new CacheObject(); @@ -39,12 +40,17 @@ public function load(): CacheObject { return $cacheObject; } + #[\Override] public function save(CacheObject $cacheObject): void { $data = [ 'cacheVersion' => $cacheObject->cacheVersion, 'entries' => $cacheObject->entries, ]; - $result = file_put_contents($this->cacheFilePath, json_encode($data)); + $encodedData = json_encode($data); + if ($encodedData === false) { + throw new \Exception('Failed to write cache file; encoding failed'); + } + $result = file_put_contents($this->cacheFilePath, $encodedData); if ($result === false) { throw new \Exception('Failed to write cache file'); } diff --git a/PhpcsChanged/FullReporter.php b/PhpcsChanged/FullReporter.php index dd33991..a47a318 100644 --- a/PhpcsChanged/FullReporter.php +++ b/PhpcsChanged/FullReporter.php @@ -9,6 +9,7 @@ use function PhpcsChanged\getLongestString; class FullReporter implements Reporter { + #[\Override] public function getFormattedMessages(PhpcsMessages $messages, array $options): string { $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; @@ -68,6 +69,7 @@ private function getFormattedMessagesForFile(array $messages, string $file, arra EOF; } + #[\Override] public function getExitCode(PhpcsMessages $messages): int { return (count($messages->getMessages()) > 0) ? 1 : 0; } diff --git a/PhpcsChanged/JsonReporter.php b/PhpcsChanged/JsonReporter.php index 56d8e06..cc35977 100644 --- a/PhpcsChanged/JsonReporter.php +++ b/PhpcsChanged/JsonReporter.php @@ -9,6 +9,7 @@ use PhpcsChanged\LintMessage; class JsonReporter implements Reporter { + #[\Override] public function getFormattedMessages(PhpcsMessages $messages, array $options): string { //phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; @@ -30,9 +31,6 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s $warnings = array_values(array_filter($messages->getMessages(), function($message) { return $message->getType() === 'WARNING'; })); - $messages = array_map(function($message) { - return PhpcsMessagesHelpers::messageToPhpcsArray($message); - }, $messages->getMessages()); $dataForJson = [ 'totals' => [ 'errors' => count($errors), @@ -42,7 +40,7 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s 'files' => array_merge([], ...$outputByFile), ]; $output = json_encode($dataForJson, JSON_UNESCAPED_SLASHES); - if (! boolval($output)) { + if ($output === false) { throw new \Exception('Failed to JSON-encode result messages'); } return $output; @@ -68,6 +66,7 @@ private function getFormattedMessagesForFile(array $messages, string $file): arr return $dataForJson; } + #[\Override] public function getExitCode(PhpcsMessages $messages): int { return (count($messages->getMessages()) > 0) ? 1 : 0; } diff --git a/PhpcsChanged/JunitReporter.php b/PhpcsChanged/JunitReporter.php new file mode 100644 index 0000000..50dec1a --- /dev/null +++ b/PhpcsChanged/JunitReporter.php @@ -0,0 +1,95 @@ +getFile() ?? 'STDIN'; + }, $messages->getMessages())); + if (count($files) === 0) { + $files = ['STDIN']; + } + + $totalTests = count($messages->getMessages()); + $totalFailures = count(array_filter($messages->getMessages(), function(LintMessage $message): bool { + return $message->getType() === 'WARNING'; + })); + $totalErrors = count(array_filter($messages->getMessages(), function(LintMessage $message): bool { + return $message->getType() === 'ERROR'; + })); + + // Calculate total time from all files + $totalTime = array_sum($messages->getAllTiming()); + + $outputByFile = array_reduce($files, function(string $output, string $file) use ($messages): string { + $messagesForFile = array_values(array_filter($messages->getMessages(), static function(LintMessage $message) use ($file): bool { + return ($message->getFile() ?? 'STDIN') === $file; + })); + $output .= $this->getFormattedMessagesForFile($messagesForFile, $file, $messages); + return $output; + }, ''); + + $output = "\n"; + $output .= sprintf("\n", $totalTests, $totalFailures, $totalErrors, $totalTime); + $output .= $outputByFile; + $output .= "\n"; + + return $output; + } + + private function getFormattedMessagesForFile(array $messages, string $file, PhpcsMessages $allMessages): string { + $testCount = count($messages); + $errorCount = count(array_values(array_filter($messages, function(LintMessage $message) { + return $message->getType() === 'ERROR'; + }))); + $failureCount = count(array_values(array_filter($messages, function(LintMessage $message) { + return $message->getType() === 'WARNING'; + }))); + + // Get timing for this specific file + $fileTime = $allMessages->getTiming($file); + + $xmlOutputForFile = sprintf("\t\n", + $file, $testCount, $failureCount, $errorCount, $fileTime); + $xmlOutputForFile .= array_reduce($messages, function(string $output, LintMessage $message): string { + $line = $message->getLineNumber(); + $column = $message->getColumn(); + $source = $this->escapeXml($message->getSource()); + $messageText = $this->escapeXml($message->getMessage()); + $type = $message->getType(); + $severity = $message->getSeverity(); + + // Create a unique test case name using line:column and source + $testCaseName = "line {$line}, column {$column}"; + $output .= "\t\t\n"; + + if ($type === 'ERROR') { + $output .= "\t\t\tLine {$line}, Column {$column}: {$messageText} (Severity: {$severity})\n"; + } else { + $output .= "\t\t\tLine {$line}, Column {$column}: {$messageText} (Severity: {$severity})\n"; + } + + $output .= "\t\t\n"; + return $output; + }, ''); + $xmlOutputForFile .= "\t\n"; + + return $xmlOutputForFile; + } + + private function escapeXml(string $string): string { + return htmlspecialchars($string, ENT_XML1 | ENT_QUOTES, 'UTF-8'); + } + + #[\Override] + public function getExitCode(PhpcsMessages $messages): int { + return (count($messages->getMessages()) > 0) ? 1 : 0; + } +} diff --git a/PhpcsChanged/LintMessage.php b/PhpcsChanged/LintMessage.php index c07d563..84c22a6 100644 --- a/PhpcsChanged/LintMessage.php +++ b/PhpcsChanged/LintMessage.php @@ -4,9 +4,24 @@ namespace PhpcsChanged; class LintMessage { + /** + * @var int Line number where the message occurs + */ private $line; + + /** + * @var string|null File path where the message occurs + */ private $file; + + /** + * @var string Message type (e.g., 'ERROR', 'WARNING') + */ private $type; + + /** + * @var array Additional message properties (message, source, column, severity, fixable, etc.) + */ private $otherProperties; public function __construct(int $line, ?string $file, string $type, array $otherProperties) { diff --git a/PhpcsChanged/LintMessages.php b/PhpcsChanged/LintMessages.php index 859226d..180f2e1 100644 --- a/PhpcsChanged/LintMessages.php +++ b/PhpcsChanged/LintMessages.php @@ -12,6 +12,11 @@ class LintMessages { */ private $messages = []; + /** + * @var array Per-file execution timing in seconds + */ + private $timingData = []; + final public function __construct(array $messages) { foreach($messages as $message) { if (! $message instanceof LintMessage) { @@ -25,9 +30,18 @@ final public function __construct(array $messages) { * @return static */ public static function merge(array $messages) { - return self::fromLintMessages(array_merge([], ...array_map(function(self $message) { + $merged = self::fromLintMessages(array_merge([], ...array_map(function(self $message) { return $message->getMessages(); }, $messages))); + + // Merge timing data from all message sets + $mergedTiming = []; + foreach ($messages as $messageSet) { + $mergedTiming = array_merge($mergedTiming, $messageSet->getAllTiming()); + } + $merged->setAllTiming($mergedTiming); + + return $merged; } /** @@ -64,7 +78,7 @@ public function getLineNumbers(): array { public static function getNewMessages(string $unifiedDiff, self $unmodifiedMessages, self $modifiedMessages) { $map = DiffLineMap::fromUnifiedDiff($unifiedDiff); $fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff); - return self::fromLintMessages(array_values(array_filter($modifiedMessages->getMessages(), function($newMessage) use ($unmodifiedMessages, $map) { + $newMessages = self::fromLintMessages(array_values(array_filter($modifiedMessages->getMessages(), function($newMessage) use ($unmodifiedMessages, $map) { $lineNumber = $newMessage->getLineNumber(); if (! $lineNumber) { return true; @@ -73,7 +87,50 @@ public static function getNewMessages(string $unifiedDiff, self $unmodifiedMessa $unmodifiedMessagesContainingUnmodifiedLineNumber = array_values(array_filter($unmodifiedMessages->getMessages(), function($unmodifiedMessage) use ($unmodifiedLineNumber) { return $unmodifiedMessage->getLineNumber() === $unmodifiedLineNumber; })); - return ! count($unmodifiedMessagesContainingUnmodifiedLineNumber) > 0; + return ! (count($unmodifiedMessagesContainingUnmodifiedLineNumber) > 0); })), $fileName); + + // Preserve timing data from the modified messages + $newMessages->setAllTiming($modifiedMessages->getAllTiming()); + + return $newMessages; + } + + /** + * Set timing data for a file + * + * @param string $fileName The file name or path + * @param float $duration Duration in seconds + */ + public function setTiming(string $fileName, float $duration): void { + $this->timingData[$fileName] = $duration; + } + + /** + * Get timing data for a specific file + * + * @param string $fileName The file name or path + * @return float Duration in seconds, or 0.0 if not set + */ + public function getTiming(string $fileName): float { + return $this->timingData[$fileName] ?? 0.0; + } + + /** + * Get all timing data + * + * @return array Array mapping file names to durations in seconds + */ + public function getAllTiming(): array { + return $this->timingData; + } + + /** + * Set all timing data at once + * + * @param array $timingData Array mapping file names to durations + */ + public function setAllTiming(array $timingData): void { + $this->timingData = $timingData; } } diff --git a/PhpcsChanged/UnixShell.php b/PhpcsChanged/UnixShell.php index cc10aeb..4ed4937 100644 --- a/PhpcsChanged/UnixShell.php +++ b/PhpcsChanged/UnixShell.php @@ -36,11 +36,13 @@ public function __construct(CliOptions $options) { $this->options = $options; } + #[\Override] public function clearCaches(): void { $this->fullPaths = []; $this->svnInfo = []; } + #[\Override] public function validateShellIsReady(): void { if ($this->options->mode === Modes::MANUAL) { $phpcs = $this->getPhpcsExecutable(); @@ -100,6 +102,7 @@ protected function executeCommand(string $command, ?int &$return_val = null): st return implode(PHP_EOL, $output) . PHP_EOL; } + #[\Override] public function getPhpcsStandards(): string { $phpcs = $this->getPhpcsExecutable(); $installedCodingStandardsPhpcsOutputCommand = "{$phpcs} -i"; @@ -141,6 +144,7 @@ private function isFileStagedForAdding(string $fileName): bool { return isset($gitStatusOutput[0]) && $gitStatusOutput[0] === 'A'; } + #[\Override] public function doesUnmodifiedFileExistInGit(string $fileName): bool { if ($this->options->mode === Modes::GIT_BASE) { return $this->doesFileExistInGitBase($fileName); @@ -208,6 +212,7 @@ private function getUnmodifiedFileContentsCommand(string $fileName): string { return "{$git} show {$rev}:" . escapeshellarg($fullPath); } + #[\Override] public function getGitHashOfModifiedFile(string $fileName): string { $debug = getDebug($this->options->debug); $git = $this->options->getExecutablePath('git'); @@ -222,6 +227,7 @@ public function getGitHashOfModifiedFile(string $fileName): string { return $hash; } + #[\Override] public function getGitHashOfUnmodifiedFile(string $fileName): string { $debug = getDebug($this->options->debug); $git = $this->options->getExecutablePath('git'); @@ -252,6 +258,7 @@ private function getPhpcsExtensionsOption(): string { return $phpcsExtensionsOption; } + #[\Override] public function getPhpcsOutputOfModifiedGitFile(string $fileName): string { $debug = getDebug($this->options->debug); $fileContentsCommand = $this->getModifiedFileContentsCommand($fileName); @@ -261,6 +268,7 @@ public function getPhpcsOutputOfModifiedGitFile(string $fileName): string { return $this->processPhpcsOutput($fileName, 'modified', $modifiedFilePhpcsOutput); } + #[\Override] public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string { $debug = getDebug($this->options->debug); $unmodifiedFileContentsCommand = $this->getUnmodifiedFileContentsCommand($fileName); @@ -270,6 +278,7 @@ public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string { return $this->processPhpcsOutput($fileName, 'unmodified', $unmodifiedFilePhpcsOutput); } + #[\Override] public function getGitUnifiedDiff(string $fileName): string { $debug = getDebug($this->options->debug); $git = $this->options->getExecutablePath('git'); @@ -285,6 +294,7 @@ public function getGitUnifiedDiff(string $fileName): string { return $unifiedDiff; } + #[\Override] public function getGitMergeBase(): string { if ($this->options->mode !== Modes::GIT_BASE) { return ''; @@ -302,6 +312,7 @@ public function getGitMergeBase(): string { return trim($mergeBase); } + #[\Override] public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string { $debug = getDebug($this->options->debug); $cat = $this->options->getExecutablePath('cat'); @@ -311,6 +322,7 @@ public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string { return $this->processPhpcsOutput($fileName, 'modified', $modifiedFilePhpcsOutput); } + #[\Override] public function getPhpcsOutputOfUnmodifiedSvnFile(string $fileName): string { $debug = getDebug($this->options->debug); $svn = $this->options->getExecutablePath('svn'); @@ -338,11 +350,13 @@ private function processPhpcsOutput(string $fileName, string $modifiedOrUnmodifi return $phpcsOutput; } + #[\Override] public function doesUnmodifiedFileExistInSvn(string $fileName): bool { $svnFileInfo = $this->getSvnFileInfo($fileName); return (false !== strpos($svnFileInfo, 'Schedule: add')); } + #[\Override] public function getSvnRevisionId(string $fileName): string { $svnFileInfo = $this->getSvnFileInfo($fileName); preg_match('/\bLast Changed Rev:\s([^\n]+)/', $svnFileInfo, $matches); @@ -369,6 +383,7 @@ private function getSvnFileInfo(string $fileName): string { return $svnStatusOutput; } + #[\Override] public function getSvnUnifiedDiff(string $fileName): string { $debug = getDebug($this->options->debug); $svn = $this->options->getExecutablePath('svn'); @@ -382,10 +397,12 @@ public function getSvnUnifiedDiff(string $fileName): string { return $unifiedDiff; } + #[\Override] public function isReadable(string $fileName): bool { return is_readable($fileName); } + #[\Override] public function getFileHash(string $fileName): string { $result = md5_file($fileName); if ($result === false) { @@ -394,19 +411,23 @@ public function getFileHash(string $fileName): string { return $result; } + #[\Override] public function exitWithCode(int $code): void { exit($code); } + #[\Override] public function printError(string $message): void { printError($message); } + #[\Override] public function getFileNameFromPath(string $path): string { $parts = explode('/', $path); return end($parts); } + #[\Override] public function getPhpcsVersion(): string { $phpcs = $this->getPhpcsExecutable(); diff --git a/PhpcsChanged/XmlReporter.php b/PhpcsChanged/XmlReporter.php index 2c9a92a..d264a38 100644 --- a/PhpcsChanged/XmlReporter.php +++ b/PhpcsChanged/XmlReporter.php @@ -27,6 +27,7 @@ public function __construct(CliOptions $options, ShellOperator $shell) { $this->shell = $shell; } + #[\Override] public function getFormattedMessages(PhpcsMessages $messages, array $options): string { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; @@ -80,6 +81,7 @@ private function getFormattedMessagesForFile(array $messages, string $file): str return $xmlOutputForFile; } + #[\Override] public function getExitCode(PhpcsMessages $messages): int { return (count($messages->getMessages()) > 0) ? 1 : 0; } diff --git a/README.md b/README.md index 6583b5c..15f5a1d 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ More than one file can be specified after a version control option, including gl You can use `--ignore` to ignore any directory, file, or paths matching provided pattern(s). For example.: `--ignore=bin/*,vendor/*` would ignore any files in bin directory, as well as in vendor. -You can use `--report` to customize the output type. `full` (the default) is human-readable, `json` prints a JSON object as shown above, and 'xml' can be used by IDEs. These match the phpcs reporters of the same names. +You can use `--report` to customize the output type. `full` (the default) is human-readable, `json` prints a JSON object as shown above, and 'xml' can be used by IDEs. These match the phpcs reporters of the same names. `junit` can also be used for [JUnit XML](https://github.com/testmoapp/junitxml) which can be helpful for test runners. You can use `--standard` to specify a specific phpcs standard to run. This matches the phpcs option of the same name. diff --git a/bin/phpcs-changed b/bin/phpcs-changed index f1f1705..231a3a4 100755 --- a/bin/phpcs-changed +++ b/bin/phpcs-changed @@ -67,6 +67,10 @@ $options = getopt( $optind ); +if ($options === false) { + $options = []; +} + $fileNames = array_slice($argv, $optind); $fileNamesExpanded = []; foreach( $fileNames as $file ) { @@ -134,7 +138,10 @@ $debug = getDebug(isset($options['debug'])); run($options, $debug); function run(array $rawOptions, callable $debug): void { - $debug('Options: ' . json_encode($rawOptions)); + $encodedOptions = json_encode($rawOptions); + if ($encodedOptions !== false) { + $debug('Options: ' . $encodedOptions); + } try { $options = CliOptions::fromArray($rawOptions); } catch ( InvalidOptionException $err ) { diff --git a/composer.json b/composer.json index 1d98a4c..45b1367 100644 --- a/composer.json +++ b/composer.json @@ -36,9 +36,9 @@ }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.1 || ^1.0.0", - "phpunit/phpunit": "^6.4 || ^9.5", + "phpunit/phpunit": "^6.4 || ^7.0 || ^8.0 || ^9.5", "squizlabs/php_codesniffer": "^3.2.1", "sirbrillig/phpcs-variable-analysis": "^2.1.3", - "vimeo/psalm": "^0.2 || ^0.3 || ^1.1 || ^4.24 || ^5.0@beta" + "vimeo/psalm": "^4.24 || ^5.0 || ^6.0" } } diff --git a/index.php b/index.php index ab77a36..48486b9 100644 --- a/index.php +++ b/index.php @@ -18,6 +18,7 @@ require_once __DIR__ . '/PhpcsChanged/JsonReporter.php'; require_once __DIR__ . '/PhpcsChanged/FullReporter.php'; require_once __DIR__ . '/PhpcsChanged/XmlReporter.php'; +require_once __DIR__ . '/PhpcsChanged/JunitReporter.php'; require_once __DIR__ . '/PhpcsChanged/NoChangesException.php'; require_once __DIR__ . '/PhpcsChanged/ShellException.php'; require_once __DIR__ . '/PhpcsChanged/ShellOperator.php'; diff --git a/tests/JunitReporterTest.php b/tests/JunitReporterTest.php new file mode 100644 index 0000000..af79511 --- /dev/null +++ b/tests/JunitReporterTest.php @@ -0,0 +1,281 @@ + 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Foo.', + ], + ], 'fileA.php'); + $expected = << + + + + Line 15, Column 5: Found unused symbol Foo. (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testSingleError() { + $messages = PhpcsMessages::fromArrays([ + [ + 'type' => 'ERROR', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Foo.', + ], + ], 'fileA.php'); + $expected = << + + + + Line 15, Column 5: Found unused symbol Foo. (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testMultipleWarningsWithLongLineNumber() { + $messages = PhpcsMessages::fromArrays([ + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 133825, + 'message' => 'Found unused symbol Foo.', + ], + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Bar.', + ], + ], 'fileA.php'); + $expected = << + + + + Line 133825, Column 5: Found unused symbol Foo. (Severity: 5) + + + Line 15, Column 5: Found unused symbol Bar. (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testMultipleWarningsErrorsAndFiles() { + $messagesA = PhpcsMessages::fromArrays([ + [ + 'type' => 'ERROR', + 'severity' => 5, + 'fixable' => true, + 'column' => 2, + 'source' => 'ImportDetection.Imports.RequireImports.Something', + 'line' => 12, + 'message' => 'Found unused symbol Faa.', + ], + [ + 'type' => 'ERROR', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Foo.', + ], + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 8, + 'source' => 'ImportDetection.Imports.RequireImports.Boom', + 'line' => 18, + 'message' => 'Found unused symbol Bar.', + ], + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 22, + 'message' => 'Found unused symbol Foo.', + ], + ], 'fileA.php'); + $messagesB = PhpcsMessages::fromArrays([ + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Zoop', + 'line' => 30, + 'message' => 'Found unused symbol Hi.', + ], + ], 'fileB.php'); + $messages = PhpcsMessages::merge([$messagesA, $messagesB]); + $expected = << + + + + Line 12, Column 2: Found unused symbol Faa. (Severity: 5) + + + Line 15, Column 5: Found unused symbol Foo. (Severity: 5) + + + Line 18, Column 8: Found unused symbol Bar. (Severity: 5) + + + Line 22, Column 5: Found unused symbol Foo. (Severity: 5) + + + + + Line 30, Column 5: Found unused symbol Hi. (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, ['s' => 1]); + $this->assertEquals($expected, $result); + } + + public function testNoWarnings() { + $messages = PhpcsMessages::fromArrays([]); + $expected = << + + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testSingleWarningWithNoFilename() { + $messages = PhpcsMessages::fromArrays([ + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Foo.', + ], + ]); + $expected = << + + + + Line 15, Column 5: Found unused symbol Foo. (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testXmlEscaping() { + $messages = PhpcsMessages::fromArrays([ + [ + 'type' => 'ERROR', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'Test.Source<>&"', + 'line' => 15, + 'message' => 'Message with & "quotes".', + ], + ], 'fileA.php'); + $expected = << + + + + Line 15, Column 5: Message with <xml> & "quotes". (Severity: 5) + + + + +EOF; + $reporter = new JunitReporter(); + $result = $reporter->getFormattedMessages($messages, []); + $this->assertEquals($expected, $result); + } + + public function testGetExitCodeWithMessages() { + $messages = PhpcsMessages::fromArrays([ + [ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 5, + 'source' => 'ImportDetection.Imports.RequireImports.Import', + 'line' => 15, + 'message' => 'Found unused symbol Foo.', + ], + ], 'fileA.php'); + $reporter = new JunitReporter(); + $this->assertEquals(1, $reporter->getExitCode($messages)); + } + + public function testGetExitCodeWithNoMessages() { + $messages = PhpcsMessages::fromArrays([], 'fileA.php'); + $reporter = new JunitReporter(); + $this->assertEquals(0, $reporter->getExitCode($messages)); + } +}