From 74ce1ebc09bfe67de9fadf1a645b5850b28feaa2 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 20 Apr 2026 19:07:58 -0400 Subject: [PATCH 01/12] Batch phpcs calls across files into a single invocation Replace the 2N per-file phpcs launches in runGitWorkflow and runSvnWorkflow with a single batch invocation. All modified and unmodified file contents are written to a temp directory and phpcs is run once on all of them, eliminating the startup overhead cost that scaled linearly with the number of changed files. - Add getPhpcsOutputForGitBatch / getPhpcsOutputForSvnBatch to ShellOperator - Implement batch methods in UnixShell using a temp dir layout (new/ and old/) - Override batch methods in TestShell to delegate to existing per-file mocks - Rewrite runGitWorkflow and runSvnWorkflow with pre-batch/batch/filter phases - Add and update tests for the new batch behavior --- PhpcsChanged/Cli.php | 269 ++++++++++++++++++++++++++++- PhpcsChanged/ShellOperator.php | 22 +++ PhpcsChanged/ShellRunner.php | 148 ++++++++++++++++ PhpcsChanged/UnixShell.php | 10 ++ PhpcsChanged/WindowsShell.php | 10 ++ tests/GitWorkflowTest.php | 84 ++++++++- tests/SvnWorkflowTest.php | 60 ++++++- tests/helpers/TestShell.php | 24 +++ tests/helpers/WindowsTestShell.php | 24 +++ 9 files changed, 631 insertions(+), 20 deletions(-) diff --git a/PhpcsChanged/Cli.php b/PhpcsChanged/Cli.php index c2a837f..4795ba5 100644 --- a/PhpcsChanged/Cli.php +++ b/PhpcsChanged/Cli.php @@ -229,12 +229,142 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she loadCache($cache, $shell, $options->toArray()); - $phpcsMessages = array_map(function(string $svnFile) use ($options, $shell, $cache, $debug): PhpcsMessages { - return runSvnWorkflowForFile($svnFile, $options, $shell, $cache, $debug); - }, $svnFiles); + $phpcsStandard = $options->phpcsStandard; + $warningSeverity = $options->warningSeverity; + $errorSeverity = $options->errorSeverity; - saveCache($cache, $shell, $options->toArray()); + // Pre-batch phase: determine which files need phpcs scans + $needsModifiedPhpcs = []; + $needsUnmodifiedPhpcs = []; + $modifiedOutputs = []; + $unmodifiedOutputs = []; + $isNewFileMap = []; + $modifiedHashMap = []; + $revisionIdMap = []; + + foreach ($svnFiles as $svnFile) { + try { + if (! $shell->isReadable($svnFile)) { + throw new ShellException("Cannot read file '{$svnFile}'"); + } + + $modifiedFileHash = ''; + $modifiedCached = null; + if (isCachingEnabled($options->toArray())) { + $modifiedFileHash = $shell->getFileHash($svnFile); + $modifiedCached = $cache->getCacheForFile($svnFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); + $debug(($modifiedCached !== null ? 'Using' : 'Not using') . " cache for modified file '{$svnFile}' at hash '{$modifiedFileHash}', and standard '{$phpcsStandard}'"); + } + $modifiedHashMap[$svnFile] = $modifiedFileHash; + + if ($modifiedCached !== null) { + $modifiedOutputs[$svnFile] = $modifiedCached; + } else { + $needsModifiedPhpcs[] = $svnFile; + } + + $revisionId = $shell->getSvnRevisionId($svnFile); + $isNewFile = $shell->doesUnmodifiedFileExistInSvn($svnFile); + $isNewFileMap[$svnFile] = $isNewFile; + $revisionIdMap[$svnFile] = $revisionId; + if ($isNewFile) { + $debug("File '{$svnFile}' is new; unmodified version will not be scanned."); + } + + if (! $isNewFile) { + $unmodifiedCached = null; + if (isCachingEnabled($options->toArray())) { + $unmodifiedCached = $cache->getCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); + $debug(($unmodifiedCached !== null ? 'Using' : 'Not using') . " cache for unmodified file '{$svnFile}' at revision '{$revisionId}', and standard '{$phpcsStandard}'"); + } + + if ($unmodifiedCached !== null) { + $unmodifiedOutputs[$svnFile] = $unmodifiedCached; + } else { + $needsUnmodifiedPhpcs[] = $svnFile; + } + } + } catch( ShellException $err ) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit, like in tests + } + } + + // Batch phase: single phpcs invocation for all uncached files + $batchTime = 0.0; + $batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs); + if ($batchSize > 0) { + try { + $batchStartTime = microtime(true); + $batchResults = $shell->getPhpcsOutputForSvnBatch($needsModifiedPhpcs, $needsUnmodifiedPhpcs); + $batchTime = microtime(true) - $batchStartTime; + } catch( \Exception $err ) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit, like in tests + } + + foreach ($needsModifiedPhpcs as $svnFile) { + $modifiedOutputs[$svnFile] = $batchResults['new'][$svnFile] ?? ''; + if (isCachingEnabled($options->toArray())) { + $cache->setCacheForFile($svnFile, 'new', $modifiedHashMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$svnFile]); + } + } + + foreach ($needsUnmodifiedPhpcs as $svnFile) { + $unmodifiedOutputs[$svnFile] = $batchResults['old'][$svnFile] ?? ''; + if (isCachingEnabled($options->toArray())) { + $cache->setCacheForFile($svnFile, 'old', $revisionIdMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$svnFile]); + } + } + } + $timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0; + + // Filter phase: compute new messages per file + $phpcsMessages = []; + foreach ($svnFiles as $svnFile) { + $fileName = $shell->getFileNameFromPath($svnFile); + try { + $modifiedOutput = $modifiedOutputs[$svnFile] ?? ''; + $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $fileName); + $modifiedFilePhpcsMessages->setTiming($fileName, $timePerFile); + $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; + + if (! $hasNewPhpcsMessages) { + throw new NoChangesException("Modified file '{$svnFile}' has no PHPCS messages; skipping"); + } + + $unifiedDiff = $shell->getSvnUnifiedDiff($svnFile); + $isNewFile = $isNewFileMap[$svnFile] ?? false; + + if ($isNewFile) { + $debug('Skipping the linting of the unmodified file as it is a new file.'); + $phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson('', $fileName), $modifiedFilePhpcsMessages); + continue; + } + + $unmodifiedOutput = $unmodifiedOutputs[$svnFile] ?? ''; + $phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedOutput, $fileName), $modifiedFilePhpcsMessages); + } catch( NoChangesException $err ) { + $debug($err->getMessage()); + $unifiedDiff = ''; + $unmodifiedFilePhpcsOutput = ''; + $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson(''); + $phpcsMessages[] = getNewPhpcsMessages( + $unifiedDiff, + PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $fileName), + $modifiedFilePhpcsMessages + ); + } catch( \Exception $err ) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit, like in tests + } + } + + saveCache($cache, $shell, $options->toArray()); $shell->clearCaches(); return PhpcsMessages::merge($phpcsMessages); } @@ -336,12 +466,135 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager loadCache($cache, $shell, $options->toArray()); - $phpcsMessages = array_map(function(string $gitFile) use ($options, $shell, $cache, $debug): PhpcsMessages { - return runGitWorkflowForFile($gitFile, $options, $shell, $cache, $debug); - }, $options->files); + $phpcsStandard = $options->phpcsStandard; + $warningSeverity = $options->warningSeverity; + $errorSeverity = $options->errorSeverity; - saveCache($cache, $shell, $options->toArray()); + // Pre-batch phase: determine which files need phpcs scans + $needsModifiedPhpcs = []; + $needsUnmodifiedPhpcs = []; + $modifiedOutputs = []; + $unmodifiedOutputs = []; + $isNewFileMap = []; + $modifiedHashMap = []; + $unmodifiedHashMap = []; + + foreach ($options->files as $gitFile) { + try { + if (! $shell->isReadable($gitFile)) { + throw new ShellException("Cannot read file '{$gitFile}'"); + } + + $modifiedHash = ''; + $modifiedCached = null; + if (isCachingEnabled($options->toArray())) { + $modifiedHash = $shell->getGitHashOfModifiedFile($gitFile); + $modifiedCached = $cache->getCacheForFile($gitFile, 'new', $modifiedHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); + $debug(($modifiedCached !== null ? 'Using' : 'Not using') . " cache for modified file '{$gitFile}' at hash '{$modifiedHash}', and standard '{$phpcsStandard}'"); + } + $modifiedHashMap[$gitFile] = $modifiedHash; + + if ($modifiedCached !== null) { + $modifiedOutputs[$gitFile] = $modifiedCached; + } else { + $needsModifiedPhpcs[] = $gitFile; + } + + $isNewFile = $shell->doesUnmodifiedFileExistInGit($gitFile); + $isNewFileMap[$gitFile] = $isNewFile; + if ($isNewFile) { + $debug("File '{$gitFile}' is new; unmodified version will not be scanned."); + } + + if (! $isNewFile) { + $unmodifiedHash = ''; + $unmodifiedCached = null; + if (isCachingEnabled($options->toArray())) { + $unmodifiedHash = $shell->getGitHashOfUnmodifiedFile($gitFile); + $unmodifiedCached = $cache->getCacheForFile($gitFile, 'old', $unmodifiedHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? ''); + $debug(($unmodifiedCached !== null ? 'Using' : 'Not using') . " cache for unmodified file '{$gitFile}' at hash '{$unmodifiedHash}', and standard '{$phpcsStandard}'"); + } + $unmodifiedHashMap[$gitFile] = $unmodifiedHash; + + if ($unmodifiedCached !== null) { + $unmodifiedOutputs[$gitFile] = $unmodifiedCached; + } else { + $needsUnmodifiedPhpcs[] = $gitFile; + } + } + } catch(ShellException $err) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit + } + } + // Batch phase: single phpcs invocation for all uncached files + $batchTime = 0.0; + $batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs); + if ($batchSize > 0) { + try { + $batchStartTime = microtime(true); + $batchResults = $shell->getPhpcsOutputForGitBatch($needsModifiedPhpcs, $needsUnmodifiedPhpcs); + $batchTime = microtime(true) - $batchStartTime; + } catch(\Exception $err) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit + } + + foreach ($needsModifiedPhpcs as $gitFile) { + $modifiedOutputs[$gitFile] = $batchResults['new'][$gitFile] ?? ''; + if (isCachingEnabled($options->toArray())) { + $cache->setCacheForFile($gitFile, 'new', $modifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$gitFile]); + } + } + + foreach ($needsUnmodifiedPhpcs as $gitFile) { + $unmodifiedOutputs[$gitFile] = $batchResults['old'][$gitFile] ?? ''; + if (isCachingEnabled($options->toArray())) { + $cache->setCacheForFile($gitFile, 'old', $unmodifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$gitFile]); + } + } + } + + $timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0; + + // Filter phase: compute new messages per file + $phpcsMessages = []; + foreach ($options->files as $gitFile) { + try { + $modifiedOutput = $modifiedOutputs[$gitFile] ?? ''; + $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $gitFile); + $modifiedFilePhpcsMessages->setTiming($gitFile, $timePerFile); + + $unifiedDiff = ''; + $unmodifiedFilePhpcsOutput = ''; + if (count($modifiedFilePhpcsMessages->getMessages()) === 0) { + throw new NoChangesException("Modified file '{$gitFile}' has no PHPCS messages; skipping"); + } + + $isNewFile = $isNewFileMap[$gitFile] ?? false; + if (! $isNewFile) { + $debug('Checking the unmodified file with PHPCS since the file is not new and contains some messages.'); + $unifiedDiff = $shell->getGitUnifiedDiff($gitFile); + $unmodifiedFilePhpcsOutput = $unmodifiedOutputs[$gitFile] ?? ''; + } else { + $debug('Skipping the linting of the unmodified file as it is a new file.'); + } + + $phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $gitFile), $modifiedFilePhpcsMessages); + } catch( NoChangesException $err ) { + $debug($err->getMessage()); + $phpcsMessages[] = PhpcsMessages::fromPhpcsJson(''); + } catch(\Exception $err) { + $shell->printError($err->getMessage()); + $shell->exitWithCode(1); + throw $err; // Just in case we do not actually exit + } + } + + saveCache($cache, $shell, $options->toArray()); $shell->clearCaches(); return PhpcsMessages::merge($phpcsMessages); } diff --git a/PhpcsChanged/ShellOperator.php b/PhpcsChanged/ShellOperator.php index 13106b9..c5ddf6d 100644 --- a/PhpcsChanged/ShellOperator.php +++ b/PhpcsChanged/ShellOperator.php @@ -50,4 +50,26 @@ public function getGitMergeBase(): string; public function getSvnRevisionId(string $fileName): string; public function getSvnUnifiedDiff(string $fileName): string; + + /** + * Run phpcs once on all modified and unmodified versions of the given git files. + * New files should not appear in $unmodifiedFileNames. + * + * @param string[] $modifiedFileNames files needing modified (new) phpcs output + * @param string[] $unmodifiedFileNames files needing unmodified (old) phpcs output + * @return array{new: array, old: array} + * Each sub-array maps originalPath => single-file phpcs JSON string + */ + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array; + + /** + * Run phpcs once on all modified and unmodified versions of the given svn files. + * New files should not appear in $unmodifiedFileNames. + * + * @param string[] $modifiedFileNames files needing modified (new) phpcs output + * @param string[] $unmodifiedFileNames files needing unmodified (old) phpcs output + * @return array{new: array, old: array} + * Each sub-array maps originalPath => single-file phpcs JSON string + */ + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array; } diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index 8b0865c..91b3c26 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -388,4 +388,152 @@ public function getPhpcsVersion(): string { return $matches[1]; } + + private function writeTempFile(string $contentCommand, string $tempPath): void { + $dir = dirname($tempPath); + if (! is_dir($dir)) { + mkdir($dir, 0777, true); + } + $content = $this->platform->executeCommand($contentCommand); + file_put_contents($tempPath, $content); + } + + /** + * @param array $tempToOriginal Maps temp file path => original file path + * @return array Maps original file path => single-file phpcs JSON string + */ + private function runBatchPhpcs(array $tempToOriginal): array { + if (empty($tempToOriginal)) { + return []; + } + $phpcs = $this->getPhpcsExecutable(); + $args = implode(' ', array_map('escapeshellarg', array_keys($tempToOriginal))); + $command = "{$phpcs} --report=json -q" . $this->getPhpcsStandardOption() . $this->getPhpcsExtensionsOption() . ' ' . $args; + $phpcsOutput = $this->platform->executeCommand($command); + + if (! $phpcsOutput) { + return []; + } + + $decoded = json_decode($phpcsOutput, true); + if (! is_array($decoded) || ! isset($decoded['files'])) { + return []; + } + + $results = []; + foreach ($tempToOriginal as $tempPath => $originalPath) { + $realTempPath = ($resolved = realpath($tempPath)) !== false ? $resolved : $tempPath; + $fileData = $decoded['files'][$realTempPath] ?? $decoded['files'][$tempPath] ?? null; + if ($fileData === null) { + $results[$originalPath] = ''; + continue; + } + $singleFileJson = json_encode([ + 'totals' => [ + 'errors' => $fileData['errors'] ?? 0, + 'warnings' => $fileData['warnings'] ?? 0, + 'fixable' => $fileData['fixable'] ?? 0, + ], + 'files' => [ + $originalPath => $fileData, + ], + ]); + $results[$originalPath] = $singleFileJson !== false ? $singleFileJson : ''; + } + + return $results; + } + + private function cleanupTempDir(string $dir): void { + if (! is_dir($dir)) { + return; + } + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($dir, \RecursiveDirectoryIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($files as $file) { + if ($file->isDir()) { + rmdir($file->getPathname()); + } else { + unlink($file->getPathname()); + } + } + rmdir($dir); + } + + /** + * @param string[] $modifiedFileNames + * @param string[] $unmodifiedFileNames + * @return array{new: array, old: array} + */ + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + if (empty($modifiedFileNames) && empty($unmodifiedFileNames)) { + return ['new' => [], 'old' => []]; + } + + $tempDir = sys_get_temp_dir() . '/phpcs-changed-' . uniqid(); + mkdir($tempDir); + $modifiedTempToOriginal = []; + $unmodifiedTempToOriginal = []; + + try { + foreach ($modifiedFileNames as $fileName) { + $tempPath = $tempDir . '/new/' . ltrim($fileName, '/'); + $this->writeTempFile($this->getModifiedFileContentsCommand($fileName), $tempPath); + $modifiedTempToOriginal[$tempPath] = $fileName; + } + foreach ($unmodifiedFileNames as $fileName) { + $tempPath = $tempDir . '/old/' . ltrim($fileName, '/'); + $this->writeTempFile($this->getUnmodifiedFileContentsCommand($fileName), $tempPath); + $unmodifiedTempToOriginal[$tempPath] = $fileName; + } + $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; + $allResults = $this->runBatchPhpcs($allTempToOriginal); + return [ + 'new' => array_intersect_key($allResults, array_flip($modifiedFileNames)), + 'old' => array_intersect_key($allResults, array_flip($unmodifiedFileNames)), + ]; + } finally { + $this->cleanupTempDir($tempDir); + } + } + + /** + * @param string[] $modifiedFileNames + * @param string[] $unmodifiedFileNames + * @return array{new: array, old: array} + */ + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + if (empty($modifiedFileNames) && empty($unmodifiedFileNames)) { + return ['new' => [], 'old' => []]; + } + + $tempDir = sys_get_temp_dir() . '/phpcs-changed-' . uniqid(); + mkdir($tempDir); + $modifiedTempToOriginal = []; + $unmodifiedTempToOriginal = []; + + try { + $svn = $this->options->getExecutablePath('svn'); + foreach ($modifiedFileNames as $fileName) { + $tempPath = $tempDir . '/new/' . ltrim($fileName, '/'); + $this->writeTempFile($this->platform->getLocalFileContentsCommand($fileName), $tempPath); + $modifiedTempToOriginal[$tempPath] = $fileName; + } + foreach ($unmodifiedFileNames as $fileName) { + $tempPath = $tempDir . '/old/' . ltrim($fileName, '/'); + $this->writeTempFile("{$svn} cat " . escapeshellarg($fileName), $tempPath); + $unmodifiedTempToOriginal[$tempPath] = $fileName; + } + $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; + $allResults = $this->runBatchPhpcs($allTempToOriginal); + return [ + 'new' => array_intersect_key($allResults, array_flip($modifiedFileNames)), + 'old' => array_intersect_key($allResults, array_flip($unmodifiedFileNames)), + ]; + } finally { + $this->cleanupTempDir($tempDir); + } + } } diff --git a/PhpcsChanged/UnixShell.php b/PhpcsChanged/UnixShell.php index d4efa5d..e8f4033 100644 --- a/PhpcsChanged/UnixShell.php +++ b/PhpcsChanged/UnixShell.php @@ -81,6 +81,16 @@ public function getFileNameFromPath(string $path): string { return end($parts); } + #[\Override] + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + return $this->runner->getPhpcsOutputForGitBatch($modifiedFileNames, $unmodifiedFileNames); + } + + #[\Override] + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + return $this->runner->getPhpcsOutputForSvnBatch($modifiedFileNames, $unmodifiedFileNames); + } + #[\Override] public function isReadable(string $fileName): bool { return is_readable($fileName); diff --git a/PhpcsChanged/WindowsShell.php b/PhpcsChanged/WindowsShell.php index 6008282..fc9fea1 100644 --- a/PhpcsChanged/WindowsShell.php +++ b/PhpcsChanged/WindowsShell.php @@ -212,4 +212,14 @@ public function getSvnUnifiedDiff(string $fileName): string { public function getPhpcsVersion(): string { return $this->runner->getPhpcsVersion(); } + + #[\Override] + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + return $this->runner->getPhpcsOutputForGitBatch($modifiedFileNames, $unmodifiedFileNames); + } + + #[\Override] + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + return $this->runner->getPhpcsOutputForSvnBatch($modifiedFileNames, $unmodifiedFileNames); + } } diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index 509a0ad..c555fa7 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -176,6 +176,8 @@ public function testFullGitWorkflowForOneChangedFileWithoutPhpcsMessagesLintsOnl $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getEmptyResults()->toPhpcsJson()); + // With batch approach, unmodified is always scanned for non-new files even when modified has no messages + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getEmptyResults()->toPhpcsJson()); $cache = new CacheManager( new TestCache(), '\PhpcsChangedTests\Debug' ); $expected = $this->phpcs->getEmptyResults(); @@ -184,7 +186,6 @@ public function testFullGitWorkflowForOneChangedFileWithoutPhpcsMessagesLintsOnl $this->assertEquals($expected->getMessages(), $messages->getMessages()); $this->assertFalse($shell->wasCommandCalled("git diff --no-prefix 'foobar.php'")); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); } public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCache() { @@ -608,23 +609,88 @@ public function testFullGitWorkflowWithUntrackedFileForInterBranchDiff() { $shell = new TestShell($options, [$gitFile]); $shell->registerExecutable('git'); $shell->registerExecutable('phpcs'); - $fixture = $this->fixture->getAltAddedLineDiff('foobar.php', 'use Foobar;'); $shell->registerCommand("git ls-files --full-name 'bin/foobar.php'", ""); $shell->registerCommand("git merge-base 'master' HEAD", "0123456789abcdef0123456789abcdef01234567\n"); - $shell->registerCommand("git diff '0123456789abcdef0123456789abcdef01234567'... --no-prefix 'bin/foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'bin/foobar.php'", ""); - $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php'", ''); - $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | git hash-object --stdin", 'previous-file-hash'); - $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6, 7], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | git hash-object --stdin", 'new-file-hash'); - $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); + // With empty full path (untracked file), cat-file uses '' as the path; non-zero return means file not in base (treated as new) + $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':''", '', 1); $shell->registerCommand("git show HEAD:'' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", '{"totals":{"errors":0,"warnings":0,"fixable":0},"files":{"bin\/foobar.php":{"errors":0,"warnings":0,"messages":[]}}}'); + $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals([], $messages->getMessages()); } + public function testFullGitWorkflowBatchTwoFilesOneNewOneExisting() { + $gitFiles = ['foobar.php', 'newfile.php']; + $options = CliOptions::fromArray(['no-cache-git-root' => false, 'git-staged' => false, 'files' => $gitFiles]); + $shell = new TestShell($options, $gitFiles); + $shell->registerExecutable('git'); + $shell->registerExecutable('phpcs'); + // Existing file + $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); + $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); + $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + // New file (staged for adding) — unmodified should NOT be scanned + $shell->registerCommand("git status --porcelain 'newfile.php'", $this->fixture->getNewFileInfo('newfile.php')); + $shell->registerCommand("git ls-files --full-name 'newfile.php'", "files/newfile.php"); + $shell->registerCommand("git diff --staged --no-prefix 'newfile.php'", $this->fixture->getNewFileDiff('newfile.php')); + $shell->registerCommand("git show :0:'files/newfile.php'", $this->phpcs->getResults('STDIN', [5], 'Found unused symbol New.')->toPhpcsJson()); + $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); + $cache = new CacheManager( new TestCache() ); + $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + // Existing file: line 21 is new; new file: line 5 is new + $this->assertNotEmpty($messages->getMessages()); + // Unmodified scan should not be called for the new file + $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/newfile.php'")); + // Unmodified scan should be called for the existing file + $this->assertTrue($shell->wasCommandCalled("git show HEAD:'files/foobar.php'")); + } + + public function testFullGitWorkflowBatchTwoFilesWithCacheHitsSkipsPhpcs() { + $gitFiles = ['foobar.php', 'baz.php']; + $options = CliOptions::fromArray([ + 'no-cache-git-root' => false, + 'git-staged' => false, + 'cache' => false, // getopt is weird and sets options to false + 'files' => $gitFiles, + ]); + $shell = new TestShell($options, $gitFiles); + $shell->registerExecutable('git'); + $shell->registerExecutable('phpcs'); + $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); + $shell->registerCommand("git status --porcelain 'baz.php'", $this->fixture->getModifiedFileInfo('baz.php')); + $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); + $shell->registerCommand("git ls-files --full-name 'baz.php'", "files/baz.php"); + $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); + $shell->registerCommand("git diff --staged --no-prefix 'baz.php'", $this->fixture->getAddedLineDiff('baz.php', 'use Baz;')); + $shell->registerCommand("git show HEAD:'files/foobar.php' | git hash-object --stdin", 'old-hash-foobar'); + $shell->registerCommand("git show HEAD:'files/baz.php' | git hash-object --stdin", 'old-hash-baz'); + $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'new-hash-foobar'); + $shell->registerCommand("git show :0:'files/baz.php' | git hash-object --stdin", 'new-hash-baz'); + $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); + + // Pre-populate cache for all four versions + $testCache = new TestCache(); + $testCache->setEntry('foobar.php', 'new', 'new-hash-foobar', '', $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $testCache->setEntry('foobar.php', 'old', 'old-hash-foobar', '', $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $testCache->setEntry('baz.php', 'new', 'new-hash-baz', '', $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Baz.')->toPhpcsJson()); + $testCache->setEntry('baz.php', 'old', 'old-hash-baz', '', $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $cache = new CacheManager($testCache, '\PhpcsChangedTests\Debug'); + + $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + + // Both files processed correctly from cache + $this->assertNotEmpty($messages->getMessages()); + // No phpcs invocations needed since all were cached + $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/baz.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/baz.php' | phpcs")); + } + public function testNameDetectionInFullGitWorkflowForInterBranchDiff() { $gitFile = 'test.php'; $options = CliOptions::fromArray(['no-cache-git-root' => false, 'git-base' => 'master', 'files' => [$gitFile]]); diff --git a/tests/SvnWorkflowTest.php b/tests/SvnWorkflowTest.php index 82c0f73..b628626 100644 --- a/tests/SvnWorkflowTest.php +++ b/tests/SvnWorkflowTest.php @@ -161,11 +161,11 @@ public function testFullSvnWorkflowForOneFileWithNoMessages() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getEmptyResults()->toPhpcsJson()); $expected = $this->phpcs->getEmptyResults(); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("svn cat 'foobar.php'")); } public function testFullSvnWorkflowForOneFileWithCachingEnabledButNoCache() { @@ -617,8 +617,6 @@ public function testFullSvnWorkflowForChangedFileWithoutPhpCsMessagesLintsOnlyNe $shell->registerCommand("cat 'foobar.php'", '{"totals":{"errors":0,"warnings":0,"fixable":0},"files":{"STDIN":{"errors":0,"warnings":0,"messages":[]}}}'); runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertFalse($shell->wasCommandCalled("svn diff 'foobar.php'")); - $this->assertFalse($shell->wasCommandCalled("svn cat 'foobar.php'")); - $this->assertFalse($shell->wasCommandCalled("svn info 'foobar.php'")); } public function testFullSvnWorkflowForNonSvnFile() { @@ -679,6 +677,62 @@ public function testFullSvnWorkflowForEmptyNewFile() { $this->assertEquals($expected->getMessages(), $messages->getMessages()); } + public function testFullSvnWorkflowBatchTwoFilesOneNewOneExisting() { + $svnFiles = ['foobar.php', 'newfile.php']; + $options = CliOptions::fromArray([ + 'svn' => false, + 'files' => $svnFiles, + ]); + $shell = new TestShell($options, $svnFiles); + $shell->registerExecutable('svn'); + $shell->registerExecutable('phpcs'); + $shell->registerExecutable('cat'); + // Existing file + $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); + $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + // New file (scheduled for adding) + $shell->registerCommand("svn diff 'newfile.php'", $this->fixture->getNewFileDiff('newfile.php')); + $shell->registerCommand("svn info 'newfile.php'", $this->fixture->getSvnInfoNewFile('newfile.php')); + $shell->registerCommand("cat 'newfile.php'", $this->phpcs->getResults('STDIN', [5, 6])->toPhpcsJson()); + $messages = runSvnWorkflow($svnFiles, $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); + $this->assertNotEmpty($messages->getMessages()); + // Existing file: unmodified scan should be called + $this->assertTrue($shell->wasCommandCalled("svn cat 'foobar.php'")); + // New file: unmodified scan should NOT be called + $this->assertFalse($shell->wasCommandCalled("svn cat 'newfile.php'")); + } + + public function testFullSvnWorkflowBatchTwoFilesWithCacheHitsSkipsPhpcs() { + $svnFiles = ['foobar.php', 'baz.php']; + $options = CliOptions::fromArray([ + 'svn' => false, + 'cache' => false, // getopt is weird and sets options to false + 'files' => $svnFiles, + ]); + $shell = new TestShell($options, $svnFiles); + $shell->registerExecutable('svn'); + $shell->registerExecutable('phpcs'); + $shell->registerExecutable('cat'); + $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); + $shell->registerCommand("svn diff 'baz.php'", $this->fixture->getAddedLineDiff('baz.php', 'use Baz;')); + $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php', '188280')); + $shell->registerCommand("svn info 'baz.php'", $this->fixture->getSvnInfo('baz.php', '188280')); + $testCache = new TestCache(); + $testCache->setEntry('foobar.php', 'new', 'foobar.php', '', $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $testCache->setEntry('foobar.php', 'old', '188280', '', $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $testCache->setEntry('baz.php', 'new', 'baz.php', '', $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Baz.')->toPhpcsJson()); + $testCache->setEntry('baz.php', 'old', '188280', '', $this->phpcs->getResults('STDIN', [20, 99], 'Found unused symbol Baz.')->toPhpcsJson()); + $messages = runSvnWorkflow($svnFiles, $options, $shell, new CacheManager($testCache), '\PhpcsChangedTests\debug'); + $this->assertNotEmpty($messages->getMessages()); + // No phpcs invocations needed since all were cached + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("svn cat 'foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'baz.php'")); + $this->assertFalse($shell->wasCommandCalled("svn cat 'baz.php'")); + } + public function testFullSvnWorkflowForOneFileWithSeveritySetToZero() { $svnFile = 'foobar.php'; $options = CliOptions::fromArray([ diff --git a/tests/helpers/TestShell.php b/tests/helpers/TestShell.php index fb879ec..f704d38 100644 --- a/tests/helpers/TestShell.php +++ b/tests/helpers/TestShell.php @@ -105,4 +105,28 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } + + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + $new = []; + foreach ($modifiedFileNames as $fileName) { + $new[$fileName] = $this->getPhpcsOutputOfModifiedGitFile($fileName); + } + $old = []; + foreach ($unmodifiedFileNames as $fileName) { + $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedGitFile($fileName); + } + return ['new' => $new, 'old' => $old]; + } + + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + $new = []; + foreach ($modifiedFileNames as $fileName) { + $new[$fileName] = $this->getPhpcsOutputOfModifiedSvnFile($fileName); + } + $old = []; + foreach ($unmodifiedFileNames as $fileName) { + $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedSvnFile($fileName); + } + return ['new' => $new, 'old' => $old]; + } } diff --git a/tests/helpers/WindowsTestShell.php b/tests/helpers/WindowsTestShell.php index f8f2a67..b84d739 100644 --- a/tests/helpers/WindowsTestShell.php +++ b/tests/helpers/WindowsTestShell.php @@ -101,4 +101,28 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } + + public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + $new = []; + foreach ($modifiedFileNames as $fileName) { + $new[$fileName] = $this->getPhpcsOutputOfModifiedGitFile($fileName); + } + $old = []; + foreach ($unmodifiedFileNames as $fileName) { + $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedGitFile($fileName); + } + return ['new' => $new, 'old' => $old]; + } + + public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { + $new = []; + foreach ($modifiedFileNames as $fileName) { + $new[$fileName] = $this->getPhpcsOutputOfModifiedSvnFile($fileName); + } + $old = []; + foreach ($unmodifiedFileNames as $fileName) { + $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedSvnFile($fileName); + } + return ['new' => $new, 'old' => $old]; + } } From 4f976a5451a4c6e43f87d616ee9d03abaca9b737 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 11:41:09 -0400 Subject: [PATCH 02/12] Refactor to simplify runXWorkflow functions --- PhpcsChanged/BatchScanResult.php | 48 ++++++++++++ PhpcsChanged/Cli.php | 128 ++++++++++++++++++++++++------- PhpcsChanged/ScanPlan.php | 105 +++++++++++++++++++++++++ index.php | 2 + 4 files changed, 254 insertions(+), 29 deletions(-) create mode 100644 PhpcsChanged/BatchScanResult.php create mode 100644 PhpcsChanged/ScanPlan.php diff --git a/PhpcsChanged/BatchScanResult.php b/PhpcsChanged/BatchScanResult.php new file mode 100644 index 0000000..1e8c8cc --- /dev/null +++ b/PhpcsChanged/BatchScanResult.php @@ -0,0 +1,48 @@ + phpcs output for the modified version, keyed by file + */ + private $modifiedOutputs; + + /** + * @var array phpcs output for the unmodified version, keyed by file + */ + private $unmodifiedOutputs; + + /** + * @var float Wall-clock time attributed to each scanned file + */ + private $timePerFile; + + /** + * @param array $modifiedOutputs + * @param array $unmodifiedOutputs + */ + public function __construct(array $modifiedOutputs, array $unmodifiedOutputs, float $timePerFile) { + $this->modifiedOutputs = $modifiedOutputs; + $this->unmodifiedOutputs = $unmodifiedOutputs; + $this->timePerFile = $timePerFile; + } + + public function getModifiedOutput(string $file): string { + return $this->modifiedOutputs[$file] ?? ''; + } + + public function getUnmodifiedOutput(string $file): string { + return $this->unmodifiedOutputs[$file] ?? ''; + } + + public function getTimePerFile(): float { + return $this->timePerFile; + } +} diff --git a/PhpcsChanged/Cli.php b/PhpcsChanged/Cli.php index 4795ba5..558cdb8 100644 --- a/PhpcsChanged/Cli.php +++ b/PhpcsChanged/Cli.php @@ -11,6 +11,8 @@ use PhpcsChanged\JunitReporter; use PhpcsChanged\CheckstyleReporter; use PhpcsChanged\PhpcsMessages; +use PhpcsChanged\ScanPlan; +use PhpcsChanged\BatchScanResult; use PhpcsChanged\ShellException; use PhpcsChanged\ShellOperator; use PhpcsChanged\XmlReporter; @@ -229,11 +231,25 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she loadCache($cache, $shell, $options->toArray()); + $plan = prepareSvnScanPlan($svnFiles, $options, $shell, $cache, $debug); + $outputs = runSvnBatchScan($plan, $options, $shell, $cache); + $phpcsMessages = getNewSvnMessagesForFiles($svnFiles, $plan, $outputs, $shell, $debug); + + saveCache($cache, $shell, $options->toArray()); + $shell->clearCaches(); + return PhpcsMessages::merge($phpcsMessages); +} + +/** + * Determine which files need fresh phpcs scans and which can be served from the cache. + * + * @param string[] $svnFiles + */ +function prepareSvnScanPlan(array $svnFiles, CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): ScanPlan { $phpcsStandard = $options->phpcsStandard; $warningSeverity = $options->warningSeverity; $errorSeverity = $options->errorSeverity; - // Pre-batch phase: determine which files need phpcs scans $needsModifiedPhpcs = []; $needsUnmodifiedPhpcs = []; $modifiedOutputs = []; @@ -291,7 +307,23 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she } } - // Batch phase: single phpcs invocation for all uncached files + return new ScanPlan($needsModifiedPhpcs, $needsUnmodifiedPhpcs, $modifiedOutputs, $unmodifiedOutputs, $isNewFileMap, $modifiedHashMap, $revisionIdMap); +} + +/** + * Run a single phpcs invocation for all uncached files in the plan and merge the + * results with the outputs already served from the cache. + */ +function runSvnBatchScan(ScanPlan $plan, CliOptions $options, ShellOperator $shell, CacheManager $cache): BatchScanResult { + $phpcsStandard = $options->phpcsStandard; + $warningSeverity = $options->warningSeverity; + $errorSeverity = $options->errorSeverity; + + $needsModifiedPhpcs = $plan->getNeedsModifiedPhpcs(); + $needsUnmodifiedPhpcs = $plan->getNeedsUnmodifiedPhpcs(); + $modifiedOutputs = $plan->getModifiedOutputs(); + $unmodifiedOutputs = $plan->getUnmodifiedOutputs(); + $batchTime = 0.0; $batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs); if ($batchSize > 0) { @@ -308,28 +340,36 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she foreach ($needsModifiedPhpcs as $svnFile) { $modifiedOutputs[$svnFile] = $batchResults['new'][$svnFile] ?? ''; if (isCachingEnabled($options->toArray())) { - $cache->setCacheForFile($svnFile, 'new', $modifiedHashMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$svnFile]); + $cache->setCacheForFile($svnFile, 'new', $plan->getModifiedCacheKey($svnFile), $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$svnFile]); } } foreach ($needsUnmodifiedPhpcs as $svnFile) { $unmodifiedOutputs[$svnFile] = $batchResults['old'][$svnFile] ?? ''; if (isCachingEnabled($options->toArray())) { - $cache->setCacheForFile($svnFile, 'old', $revisionIdMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$svnFile]); + $cache->setCacheForFile($svnFile, 'old', $plan->getUnmodifiedCacheKey($svnFile), $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$svnFile]); } } } - $timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0; + return new BatchScanResult($modifiedOutputs, $unmodifiedOutputs, $batchSize > 0 ? $batchTime / $batchSize : 0.0); +} - // Filter phase: compute new messages per file +/** + * Compute the new phpcs messages for each file from its modified/unmodified phpcs output. + * + * @param string[] $svnFiles + * + * @return PhpcsMessages[] + */ +function getNewSvnMessagesForFiles(array $svnFiles, ScanPlan $plan, BatchScanResult $outputs, ShellOperator $shell, callable $debug): array { $phpcsMessages = []; foreach ($svnFiles as $svnFile) { $fileName = $shell->getFileNameFromPath($svnFile); try { - $modifiedOutput = $modifiedOutputs[$svnFile] ?? ''; + $modifiedOutput = $outputs->getModifiedOutput($svnFile); $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $fileName); - $modifiedFilePhpcsMessages->setTiming($fileName, $timePerFile); + $modifiedFilePhpcsMessages->setTiming($fileName, $outputs->getTimePerFile()); $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; if (! $hasNewPhpcsMessages) { @@ -337,7 +377,7 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she } $unifiedDiff = $shell->getSvnUnifiedDiff($svnFile); - $isNewFile = $isNewFileMap[$svnFile] ?? false; + $isNewFile = $plan->isNewFile($svnFile); if ($isNewFile) { $debug('Skipping the linting of the unmodified file as it is a new file.'); @@ -345,7 +385,7 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she continue; } - $unmodifiedOutput = $unmodifiedOutputs[$svnFile] ?? ''; + $unmodifiedOutput = $outputs->getUnmodifiedOutput($svnFile); $phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedOutput, $fileName), $modifiedFilePhpcsMessages); } catch( NoChangesException $err ) { $debug($err->getMessage()); @@ -363,10 +403,7 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she throw $err; // Just in case we do not actually exit, like in tests } } - - saveCache($cache, $shell, $options->toArray()); - $shell->clearCaches(); - return PhpcsMessages::merge($phpcsMessages); + return $phpcsMessages; } function runSvnWorkflowForFile(string $svnFile, CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): PhpcsMessages { @@ -466,11 +503,23 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager loadCache($cache, $shell, $options->toArray()); + $plan = prepareGitScanPlan($options, $shell, $cache, $debug); + $outputs = runGitBatchScan($plan, $options, $shell, $cache); + $phpcsMessages = getNewGitMessagesForFiles($options->files, $plan, $outputs, $shell, $debug); + + saveCache($cache, $shell, $options->toArray()); + $shell->clearCaches(); + return PhpcsMessages::merge($phpcsMessages); +} + +/** + * Determine which files need fresh phpcs scans and which can be served from the cache. + */ +function prepareGitScanPlan(CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): ScanPlan { $phpcsStandard = $options->phpcsStandard; $warningSeverity = $options->warningSeverity; $errorSeverity = $options->errorSeverity; - // Pre-batch phase: determine which files need phpcs scans $needsModifiedPhpcs = []; $needsUnmodifiedPhpcs = []; $modifiedOutputs = []; @@ -529,7 +578,23 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager } } - // Batch phase: single phpcs invocation for all uncached files + return new ScanPlan($needsModifiedPhpcs, $needsUnmodifiedPhpcs, $modifiedOutputs, $unmodifiedOutputs, $isNewFileMap, $modifiedHashMap, $unmodifiedHashMap); +} + +/** + * Run a single phpcs invocation for all uncached files in the plan and merge the + * results with the outputs already served from the cache. + */ +function runGitBatchScan(ScanPlan $plan, CliOptions $options, ShellOperator $shell, CacheManager $cache): BatchScanResult { + $phpcsStandard = $options->phpcsStandard; + $warningSeverity = $options->warningSeverity; + $errorSeverity = $options->errorSeverity; + + $needsModifiedPhpcs = $plan->getNeedsModifiedPhpcs(); + $needsUnmodifiedPhpcs = $plan->getNeedsUnmodifiedPhpcs(); + $modifiedOutputs = $plan->getModifiedOutputs(); + $unmodifiedOutputs = $plan->getUnmodifiedOutputs(); + $batchTime = 0.0; $batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs); if ($batchSize > 0) { @@ -546,27 +611,35 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager foreach ($needsModifiedPhpcs as $gitFile) { $modifiedOutputs[$gitFile] = $batchResults['new'][$gitFile] ?? ''; if (isCachingEnabled($options->toArray())) { - $cache->setCacheForFile($gitFile, 'new', $modifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$gitFile]); + $cache->setCacheForFile($gitFile, 'new', $plan->getModifiedCacheKey($gitFile), $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$gitFile]); } } foreach ($needsUnmodifiedPhpcs as $gitFile) { $unmodifiedOutputs[$gitFile] = $batchResults['old'][$gitFile] ?? ''; if (isCachingEnabled($options->toArray())) { - $cache->setCacheForFile($gitFile, 'old', $unmodifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$gitFile]); + $cache->setCacheForFile($gitFile, 'old', $plan->getUnmodifiedCacheKey($gitFile), $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$gitFile]); } } } - $timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0; + return new BatchScanResult($modifiedOutputs, $unmodifiedOutputs, $batchSize > 0 ? $batchTime / $batchSize : 0.0); +} - // Filter phase: compute new messages per file +/** + * Compute the new phpcs messages for each file from its modified/unmodified phpcs output. + * + * @param string[] $gitFiles + * + * @return PhpcsMessages[] + */ +function getNewGitMessagesForFiles(array $gitFiles, ScanPlan $plan, BatchScanResult $outputs, ShellOperator $shell, callable $debug): array { $phpcsMessages = []; - foreach ($options->files as $gitFile) { + foreach ($gitFiles as $gitFile) { try { - $modifiedOutput = $modifiedOutputs[$gitFile] ?? ''; + $modifiedOutput = $outputs->getModifiedOutput($gitFile); $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $gitFile); - $modifiedFilePhpcsMessages->setTiming($gitFile, $timePerFile); + $modifiedFilePhpcsMessages->setTiming($gitFile, $outputs->getTimePerFile()); $unifiedDiff = ''; $unmodifiedFilePhpcsOutput = ''; @@ -574,11 +647,11 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager throw new NoChangesException("Modified file '{$gitFile}' has no PHPCS messages; skipping"); } - $isNewFile = $isNewFileMap[$gitFile] ?? false; + $isNewFile = $plan->isNewFile($gitFile); if (! $isNewFile) { $debug('Checking the unmodified file with PHPCS since the file is not new and contains some messages.'); $unifiedDiff = $shell->getGitUnifiedDiff($gitFile); - $unmodifiedFilePhpcsOutput = $unmodifiedOutputs[$gitFile] ?? ''; + $unmodifiedFilePhpcsOutput = $outputs->getUnmodifiedOutput($gitFile); } else { $debug('Skipping the linting of the unmodified file as it is a new file.'); } @@ -593,10 +666,7 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager throw $err; // Just in case we do not actually exit } } - - saveCache($cache, $shell, $options->toArray()); - $shell->clearCaches(); - return PhpcsMessages::merge($phpcsMessages); + return $phpcsMessages; } function runGitWorkflowForFile(string $gitFile, CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): PhpcsMessages { diff --git a/PhpcsChanged/ScanPlan.php b/PhpcsChanged/ScanPlan.php new file mode 100644 index 0000000..0edb10b --- /dev/null +++ b/PhpcsChanged/ScanPlan.php @@ -0,0 +1,105 @@ + Cached phpcs output for the modified version, keyed by file + */ + private $modifiedOutputs; + + /** + * @var array Cached phpcs output for the unmodified version, keyed by file + */ + private $unmodifiedOutputs; + + /** + * @var array Whether each file is new (has no unmodified version), keyed by file + */ + private $isNewFileMap; + + /** + * @var array Cache key (file hash) for the modified version, keyed by file + */ + private $modifiedHashMap; + + /** + * @var array Cache key for the unmodified version (svn revision id or git hash), keyed by file + */ + private $unmodifiedCacheKeyMap; + + /** + * @param string[] $needsModifiedPhpcs + * @param string[] $needsUnmodifiedPhpcs + * @param array $modifiedOutputs + * @param array $unmodifiedOutputs + * @param array $isNewFileMap + * @param array $modifiedHashMap + * @param array $unmodifiedCacheKeyMap + */ + public function __construct(array $needsModifiedPhpcs, array $needsUnmodifiedPhpcs, array $modifiedOutputs, array $unmodifiedOutputs, array $isNewFileMap, array $modifiedHashMap, array $unmodifiedCacheKeyMap) { + $this->needsModifiedPhpcs = $needsModifiedPhpcs; + $this->needsUnmodifiedPhpcs = $needsUnmodifiedPhpcs; + $this->modifiedOutputs = $modifiedOutputs; + $this->unmodifiedOutputs = $unmodifiedOutputs; + $this->isNewFileMap = $isNewFileMap; + $this->modifiedHashMap = $modifiedHashMap; + $this->unmodifiedCacheKeyMap = $unmodifiedCacheKeyMap; + } + + /** + * @return string[] + */ + public function getNeedsModifiedPhpcs(): array { + return $this->needsModifiedPhpcs; + } + + /** + * @return string[] + */ + public function getNeedsUnmodifiedPhpcs(): array { + return $this->needsUnmodifiedPhpcs; + } + + /** + * @return array + */ + public function getModifiedOutputs(): array { + return $this->modifiedOutputs; + } + + /** + * @return array + */ + public function getUnmodifiedOutputs(): array { + return $this->unmodifiedOutputs; + } + + public function isNewFile(string $file): bool { + return $this->isNewFileMap[$file] ?? false; + } + + public function getModifiedCacheKey(string $file): string { + return $this->modifiedHashMap[$file] ?? ''; + } + + public function getUnmodifiedCacheKey(string $file): string { + return $this->unmodifiedCacheKeyMap[$file] ?? ''; + } +} diff --git a/index.php b/index.php index b314991..c997425 100644 --- a/index.php +++ b/index.php @@ -12,6 +12,8 @@ require_once __DIR__ . '/PhpcsChanged/DiffLineMap.php'; require_once __DIR__ . '/PhpcsChanged/LintMessage.php'; require_once __DIR__ . '/PhpcsChanged/LintMessages.php'; +require_once __DIR__ . '/PhpcsChanged/ScanPlan.php'; +require_once __DIR__ . '/PhpcsChanged/BatchScanResult.php'; require_once __DIR__ . '/PhpcsChanged/PhpcsMessages.php'; require_once __DIR__ . '/PhpcsChanged/PhpcsMessagesHelpers.php'; require_once __DIR__ . '/PhpcsChanged/Reporter.php'; From fa57560a7f412e28d8e0c1af4d8272a556dafed2 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 12:12:42 -0400 Subject: [PATCH 03/12] Remove unused runXWorkflowForFile methods --- PhpcsChanged/Cli.php | 159 ------------------------------------------- 1 file changed, 159 deletions(-) diff --git a/PhpcsChanged/Cli.php b/PhpcsChanged/Cli.php index 558cdb8..93e9a24 100644 --- a/PhpcsChanged/Cli.php +++ b/PhpcsChanged/Cli.php @@ -406,87 +406,6 @@ function getNewSvnMessagesForFiles(array $svnFiles, ScanPlan $plan, BatchScanRes return $phpcsMessages; } -function runSvnWorkflowForFile(string $svnFile, CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): PhpcsMessages { - $phpcsStandard = $options->phpcsStandard; - - $warningSeverity = $options->warningSeverity; - $errorSeverity = $options->errorSeverity; - $fileName = $shell->getFileNameFromPath($svnFile); - - try { - if (! $shell->isReadable($svnFile)) { - throw new ShellException("Cannot read file '{$svnFile}'"); - } - - $modifiedFileHash = ''; - $modifiedFilePhpcsOutput = null; - if (isCachingEnabled($options->toArray())) { - $modifiedFileHash = $shell->getFileHash($svnFile); - $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) { - throw new NoChangesException("Modified file '{$svnFile}' has no PHPCS messages; skipping"); - } - - $unifiedDiff = $shell->getSvnUnifiedDiff($svnFile); - - $revisionId = $shell->getSvnRevisionId($svnFile); - $isNewFile = $shell->doesUnmodifiedFileExistInSvn($svnFile); - if ($isNewFile) { - $debug('Skipping the linting of the unmodified file as it is a new file.'); - } - $unmodifiedFilePhpcsOutput = ''; - if (! $isNewFile) { - if (isCachingEnabled($options->toArray())) { - $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()); - $unifiedDiff = ''; - $unmodifiedFilePhpcsOutput = ''; - $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson(''); - } catch( \Exception $err ) { - $shell->printError($err->getMessage()); - $shell->exitWithCode(1); - throw $err; // Just in case we do not actually exit, like in tests - } - - $debug('processing data...'); - return getNewPhpcsMessages( - $unifiedDiff, - PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $fileName), - $modifiedFilePhpcsMessages - ); -} - function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): PhpcsMessages { try { $debug('validating executables'); @@ -669,84 +588,6 @@ function getNewGitMessagesForFiles(array $gitFiles, ScanPlan $plan, BatchScanRes return $phpcsMessages; } -function runGitWorkflowForFile(string $gitFile, CliOptions $options, ShellOperator $shell, CacheManager $cache, callable $debug): PhpcsMessages { - $phpcsStandard = $options->phpcsStandard; - $warningSeverity = $options->warningSeverity; - $errorSeverity = $options->errorSeverity; - - try { - if (! $shell->isReadable($gitFile)) { - throw new ShellException("Cannot read file '{$gitFile}'"); - } - - $modifiedFilePhpcsOutput = null; - $modifiedFileHash = ''; - if (isCachingEnabled($options->toArray())) { - $modifiedFileHash = $shell->getGitHashOfModifiedFile($gitFile); - $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 = ''; - $unmodifiedFilePhpcsOutput = ''; - if (! $hasNewPhpcsMessages) { - throw new NoChangesException("Modified file '{$gitFile}' has no PHPCS messages; skipping"); - } - - $isNewFile = $shell->doesUnmodifiedFileExistInGit($gitFile); - if ($isNewFile) { - $debug('Skipping the linting of the unmodified file as it is a new file.'); - } - if (! $isNewFile) { - $debug('Checking the unmodified file with PHPCS since the file is not new and contains some messages.'); - $unifiedDiff = $shell->getGitUnifiedDiff($gitFile); - $unmodifiedFilePhpcsOutput = null; - $unmodifiedFileHash = ''; - if (isCachingEnabled($options->toArray())) { - $unmodifiedFileHash = $shell->getGitHashOfUnmodifiedFile($gitFile); - $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()); - $unifiedDiff = ''; - $unmodifiedFilePhpcsOutput = ''; - $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson(''); - } catch(\Exception $err) { - $shell->printError($err->getMessage()); - $shell->exitWithCode(1); - throw $err; // Just in case we do not actually exit - } - - $debug('processing data...'); - return getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $gitFile), $modifiedFilePhpcsMessages); -} - function reportMessagesAndExit(PhpcsMessages $messages, CliOptions $options, ShellOperator $shell): void { $reporter = getReporter($options->reporter, $options, $shell); echo $reporter->getFormattedMessages($messages, $options->toArray()); From fee9134760e76fcf9ea1850827acb676e0d23c95 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 13:04:39 -0400 Subject: [PATCH 04/12] Fix batched phpcs returning identical output for modified and unmodified files runBatchPhpcs keyed its results by original file path, but every file is scanned as both a modified (new/) and an unmodified (old/) temp file under the same original path, so one silently overwrote the other. Both 'new' and 'old' then resolved to the same phpcs output, defeating the new-vs-old filtering that the tool exists to perform. Key batch results by temp path instead, and map each side back to its original files through its own temp-to-original map. --- PhpcsChanged/ShellRunner.php | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index 91b3c26..fbc5c6e 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -400,7 +400,7 @@ private function writeTempFile(string $contentCommand, string $tempPath): void { /** * @param array $tempToOriginal Maps temp file path => original file path - * @return array Maps original file path => single-file phpcs JSON string + * @return array Maps temp file path => single-file phpcs JSON string */ private function runBatchPhpcs(array $tempToOriginal): array { if (empty($tempToOriginal)) { @@ -425,7 +425,7 @@ private function runBatchPhpcs(array $tempToOriginal): array { $realTempPath = ($resolved = realpath($tempPath)) !== false ? $resolved : $tempPath; $fileData = $decoded['files'][$realTempPath] ?? $decoded['files'][$tempPath] ?? null; if ($fileData === null) { - $results[$originalPath] = ''; + $results[$tempPath] = ''; continue; } $singleFileJson = json_encode([ @@ -438,7 +438,7 @@ private function runBatchPhpcs(array $tempToOriginal): array { $originalPath => $fileData, ], ]); - $results[$originalPath] = $singleFileJson !== false ? $singleFileJson : ''; + $results[$tempPath] = $singleFileJson !== false ? $singleFileJson : ''; } return $results; @@ -491,8 +491,8 @@ public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmod $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; $allResults = $this->runBatchPhpcs($allTempToOriginal); return [ - 'new' => array_intersect_key($allResults, array_flip($modifiedFileNames)), - 'old' => array_intersect_key($allResults, array_flip($unmodifiedFileNames)), + 'new' => $this->mapBatchResultsToOriginalFiles($allResults, $modifiedTempToOriginal), + 'old' => $this->mapBatchResultsToOriginalFiles($allResults, $unmodifiedTempToOriginal), ]; } finally { $this->cleanupTempDir($tempDir); @@ -529,11 +529,28 @@ public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmod $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; $allResults = $this->runBatchPhpcs($allTempToOriginal); return [ - 'new' => array_intersect_key($allResults, array_flip($modifiedFileNames)), - 'old' => array_intersect_key($allResults, array_flip($unmodifiedFileNames)), + 'new' => $this->mapBatchResultsToOriginalFiles($allResults, $modifiedTempToOriginal), + 'old' => $this->mapBatchResultsToOriginalFiles($allResults, $unmodifiedTempToOriginal), ]; } finally { $this->cleanupTempDir($tempDir); } } + + /** + * Re-key batch phpcs results (keyed by temp path) to original file paths for one side + * (modified or unmodified). Keying by temp path keeps the two sides separate even when + * the same original file appears in both. + * + * @param array $resultsByTempPath Maps temp file path => single-file phpcs JSON string + * @param array $tempToOriginal Maps temp file path => original file path + * @return array Maps original file path => single-file phpcs JSON string + */ + private function mapBatchResultsToOriginalFiles(array $resultsByTempPath, array $tempToOriginal): array { + $results = []; + foreach ($tempToOriginal as $tempPath => $originalPath) { + $results[$originalPath] = $resultsByTempPath[$tempPath] ?? ''; + } + return $results; + } } From dae764e5fba1beb29707e7db1d3e3d5816a9b1f9 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 13:05:11 -0400 Subject: [PATCH 05/12] Exercise real batch phpcs path in tests and remove per-file methods The workflow tests previously overrode getPhpcsOutputFor{Git,Svn}Batch in the test shells, looping over the per-file phpcs methods. That left the real ShellRunner batch path (temp-file writing, single combined phpcs invocation, and per-file JSON splitting) completely untested -- which is how the new/old collision bug went unnoticed. The test shells now let the real batch path run and intercept only the final combined phpcs invocation, synthesizing its output from the temp files the batch writes (buildBatchPhpcsOutput). Command registrations drop the obsolete '| phpcs' per-file suffix, and the shells prefer an exact command match so a file-contents command no longer shadows that same command piped to git hash-object. With the real batch covered, the now-unused per-file methods (getPhpcsOutputOf{Modified,Unmodified}{Git,Svn}File) and their helpers (getPhpcsCommand, processPhpcsOutput) are removed from ShellOperator, UnixShell, WindowsShell, and ShellRunner. --- PhpcsChanged/ShellOperator.php | 8 --- PhpcsChanged/ShellRunner.php | 53 ------------------ PhpcsChanged/UnixShell.php | 20 ------- PhpcsChanged/WindowsShell.php | 20 ------- tests/GitWorkflowTest.php | 90 +++++++++++++++--------------- tests/GitWorkflowWindowsTest.php | 8 +-- tests/SvnWorkflowTest.php | 28 +++++----- tests/SvnWorkflowWindowsTest.php | 12 ++-- tests/helpers/Functions.php | 28 ++++++++++ tests/helpers/TestShell.php | 38 +++++-------- tests/helpers/WindowsTestShell.php | 38 +++++-------- 11 files changed, 125 insertions(+), 218 deletions(-) diff --git a/PhpcsChanged/ShellOperator.php b/PhpcsChanged/ShellOperator.php index c5ddf6d..7367ea4 100644 --- a/PhpcsChanged/ShellOperator.php +++ b/PhpcsChanged/ShellOperator.php @@ -35,14 +35,6 @@ public function getGitHashOfModifiedFile(string $fileName): string; public function getGitHashOfUnmodifiedFile(string $fileName): string; - public function getPhpcsOutputOfModifiedGitFile(string $fileName): string; - - public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string; - - public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string; - - public function getPhpcsOutputOfUnmodifiedSvnFile(string $fileName): string; - public function getGitUnifiedDiff(string $fileName): string; public function getGitMergeBase(): string; diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index fbc5c6e..316d58b 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -241,24 +241,6 @@ private function getPhpcsExtensionsOption(): string { return strlen($phpcsExtensions) > 0 ? ' --extensions=' . escapeshellarg($phpcsExtensions) : ''; } - public function getPhpcsOutputOfModifiedGitFile(string $fileName): string { - $debug = getDebug($this->options->debug); - $fileContentsCommand = $this->getModifiedFileContentsCommand($fileName); - $command = "{$fileContentsCommand} | " . $this->getPhpcsCommand($fileName); - $debug('running modified file phpcs command:', $command); - $modifiedFilePhpcsOutput = $this->platform->executeCommand($command); - return $this->processPhpcsOutput($fileName, 'modified', $modifiedFilePhpcsOutput); - } - - public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string { - $debug = getDebug($this->options->debug); - $unmodifiedFileContentsCommand = $this->getUnmodifiedFileContentsCommand($fileName); - $command = "{$unmodifiedFileContentsCommand} | " . $this->getPhpcsCommand($fileName); - $debug('running unmodified file phpcs command:', $command); - $unmodifiedFilePhpcsOutput = $this->platform->executeCommand($command); - return $this->processPhpcsOutput($fileName, 'unmodified', $unmodifiedFilePhpcsOutput); - } - public function getGitUnifiedDiff(string $fileName): string { $debug = getDebug($this->options->debug); $git = $this->options->getExecutablePath('git'); @@ -291,41 +273,6 @@ public function getGitMergeBase(): string { return trim($mergeBase); } - public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string { - $debug = getDebug($this->options->debug); - $command = $this->platform->getLocalFileContentsCommand($fileName) . ' | ' . $this->getPhpcsCommand($fileName); - $debug('running modified file phpcs command:', $command); - $modifiedFilePhpcsOutput = $this->platform->executeCommand($command); - return $this->processPhpcsOutput($fileName, 'modified', $modifiedFilePhpcsOutput); - } - - public function getPhpcsOutputOfUnmodifiedSvnFile(string $fileName): string { - $debug = getDebug($this->options->debug); - $svn = $this->options->getExecutablePath('svn'); - $command = "{$svn} cat " . escapeshellarg($fileName) . " | " . $this->getPhpcsCommand($fileName); - $debug('running unmodified file phpcs command:', $command); - $unmodifiedFilePhpcsOutput = $this->platform->executeCommand($command); - return $this->processPhpcsOutput($fileName, 'unmodified', $unmodifiedFilePhpcsOutput); - } - - private function getPhpcsCommand(string $fileName): string { - $phpcs = $this->getPhpcsExecutable(); - return "{$phpcs} --report=json -q" . $this->getPhpcsStandardOption() . $this->getPhpcsExtensionsOption() . ' --stdin-path=' . escapeshellarg($fileName) . ' -'; - } - - private function processPhpcsOutput(string $fileName, string $modifiedOrUnmodified, string $phpcsOutput): string { - $debug = getDebug($this->options->debug); - if (! $phpcsOutput) { - throw new ShellException("Cannot get {$modifiedOrUnmodified} file phpcs output for file '{$fileName}'"); - } - $debug("{$modifiedOrUnmodified} file phpcs command output:", $phpcsOutput); - if (false !== strpos($phpcsOutput, 'You must supply at least one file or directory to process')) { - $debug("phpcs output implies {$modifiedOrUnmodified} file is empty"); - return ''; - } - return $phpcsOutput; - } - public function doesUnmodifiedFileExistInSvn(string $fileName): bool { $svnFileInfo = $this->getSvnFileInfo($fileName); return (false !== strpos($svnFileInfo, 'Schedule: add')); diff --git a/PhpcsChanged/UnixShell.php b/PhpcsChanged/UnixShell.php index e8f4033..27787bf 100644 --- a/PhpcsChanged/UnixShell.php +++ b/PhpcsChanged/UnixShell.php @@ -154,26 +154,6 @@ public function getGitHashOfUnmodifiedFile(string $fileName): string { return $this->runner->getGitHashOfUnmodifiedFile($fileName); } - #[\Override] - public function getPhpcsOutputOfModifiedGitFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfModifiedGitFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfUnmodifiedGitFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfModifiedSvnFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfUnmodifiedSvnFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfUnmodifiedSvnFile($fileName); - } - #[\Override] public function getGitUnifiedDiff(string $fileName): string { return $this->runner->getGitUnifiedDiff($fileName); diff --git a/PhpcsChanged/WindowsShell.php b/PhpcsChanged/WindowsShell.php index fc9fea1..54634fd 100644 --- a/PhpcsChanged/WindowsShell.php +++ b/PhpcsChanged/WindowsShell.php @@ -168,26 +168,6 @@ public function getGitHashOfUnmodifiedFile(string $fileName): string { return $this->runner->getGitHashOfUnmodifiedFile($fileName); } - #[\Override] - public function getPhpcsOutputOfModifiedGitFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfModifiedGitFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfUnmodifiedGitFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfUnmodifiedGitFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfModifiedSvnFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfModifiedSvnFile($fileName); - } - - #[\Override] - public function getPhpcsOutputOfUnmodifiedSvnFile(string $fileName): string { - return $this->runner->getPhpcsOutputOfUnmodifiedSvnFile($fileName); - } - #[\Override] public function getGitUnifiedDiff(string $fileName): string { return $this->runner->getGitUnifiedDiff($fileName); diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index c555fa7..6a12022 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -35,8 +35,8 @@ public function testFullGitWorkflowForOneFileStaged() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -60,8 +60,8 @@ public function testFullGitWorkflowForOneFileStagedWithReplacedGit() { $shell->registerCommand("{$gitPath} diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("{$gitPath} status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("{$gitPath} ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("{$gitPath} show HEAD:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("{$gitPath} show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("{$gitPath} show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("{$gitPath} show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("{$gitPath} rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -85,8 +85,8 @@ public function testFullGitWorkflowForOneFileStagedWithReplacedPhpcs() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -109,8 +109,8 @@ public function testFullGitWorkflowForOneFileStagedWithVendorDefaultPhpcs() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -135,8 +135,8 @@ public function testFullGitWorkflowForOneFileStagedWithNoVendorDefaultPhpcs() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -175,7 +175,7 @@ public function testFullGitWorkflowForOneChangedFileWithoutPhpcsMessagesLintsOnl $shell->registerExecutable('phpcs'); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getEmptyResults()->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getEmptyResults()->toPhpcsJson()); // With batch approach, unmodified is always scanned for non-new files even when modified has no messages $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getEmptyResults()->toPhpcsJson()); @@ -203,8 +203,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCache() { $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -216,8 +216,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCache() { $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); } public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWithSeveritySet() { @@ -238,8 +238,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -252,8 +252,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); foreach( $cache->getEntries() as $entry ) { $this->assertEquals( 'standard:w1e2', $entry->phpcsStandard ); @@ -278,8 +278,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs --report=json -q --standard='standard' --warning-severity='0' --error-severity='0'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -292,8 +292,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); $cacheEntries = $cache->getEntries(); $this->assertNotEmpty($cacheEntries); @@ -318,8 +318,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -332,8 +332,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); $cacheEntries = $cache->getEntries(); $this->assertNotEmpty($cacheEntries); @@ -357,8 +357,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenClearsOldCach $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -372,8 +372,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenClearsOldCach $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertTrue($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertTrue($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("cat 'foobar.php'")); } public function testFullGitWorkflowForOneFileUnstagedCachesDataThenClearsNewCacheWhenFileChanges() { @@ -391,8 +391,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenClearsNewCach $shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [21, 20], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show :0:'files/foobar.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("cat 'foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); @@ -406,8 +406,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenClearsNewCach $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertTrue($shell->wasCommandCalled("cat 'foobar.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertTrue($shell->wasCommandCalled("cat 'foobar.php'")); } public function testFullGitWorkflowForMultipleFilesStaged() { @@ -571,9 +571,9 @@ public function testFullGitWorkflowForInterBranchDiff() { $shell->registerCommand("git diff '0123456789abcdef0123456789abcdef01234567'... --no-prefix 'bin/foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'bin/foobar.php'", $this->fixture->getModifiedFileInfo('bin/foobar.php')); $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php'", ''); - $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php'", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | git hash-object --stdin", 'previous-file-hash'); - $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6, 7], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/bin/foobar.php'", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6, 7], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); @@ -593,9 +593,9 @@ public function testFullGitWorkflowForUnchangedFileForInterBranchDiff() { $shell->registerCommand("git diff '0123456789abcdef0123456789abcdef01234567'... --no-prefix 'bin/foobar.php'", ''); $shell->registerCommand("git status --porcelain 'bin/foobar.php'", ''); $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php'", ''); - $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php'", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/bin/foobar.php' | git hash-object --stdin", 'previous-file-hash'); - $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/bin/foobar.php'", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/bin\/foobar.php', [6], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git show HEAD:'files/bin/foobar.php' | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); @@ -614,7 +614,7 @@ public function testFullGitWorkflowWithUntrackedFileForInterBranchDiff() { $shell->registerCommand("git status --porcelain 'bin/foobar.php'", ""); // With empty full path (untracked file), cat-file uses '' as the path; non-zero return means file not in base (treated as new) $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':''", '', 1); - $shell->registerCommand("git show HEAD:'' | phpcs --report=json -q --stdin-path='bin/foobar.php' -", '{"totals":{"errors":0,"warnings":0,"fixable":0},"files":{"bin\/foobar.php":{"errors":0,"warnings":0,"messages":[]}}}'); + $shell->registerCommand("git show HEAD:''", '{"totals":{"errors":0,"warnings":0,"fixable":0},"files":{"bin\/foobar.php":{"errors":0,"warnings":0,"messages":[]}}}'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager( new TestCache() ); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); @@ -685,10 +685,10 @@ public function testFullGitWorkflowBatchTwoFilesWithCacheHitsSkipsPhpcs() { // Both files processed correctly from cache $this->assertNotEmpty($messages->getMessages()); // No phpcs invocations needed since all were cached - $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/baz.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php' | phpcs")); - $this->assertFalse($shell->wasCommandCalled("git show :0:'files/baz.php' | phpcs")); + $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("git show HEAD:'files/baz.php'")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/foobar.php'")); + $this->assertFalse($shell->wasCommandCalled("git show :0:'files/baz.php'")); } public function testNameDetectionInFullGitWorkflowForInterBranchDiff() { @@ -704,7 +704,7 @@ public function testNameDetectionInFullGitWorkflowForInterBranchDiff() { $shell->registerCommand("git merge-base 'master' HEAD", "0123456789abcdef0123456789abcdef01234567\n"); $shell->registerCommand("git diff '0123456789abcdef0123456789abcdef01234567'... --no-prefix 'test.php'", $fixture); $shell->registerCommand("git cat-file -e '0123456789abcdef0123456789abcdef01234567':'files/test.php'", '', 128); - $shell->registerCommand("git show HEAD:'files/test.php' | phpcs --report=json -q --stdin-path='test.php' -", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/test.php', [6, 7, 8], "Found unused symbol 'Foobar'.")->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/test.php'", $this->phpcs->getResults('\/srv\/www\/wordpress-default\/public_html\/test\/test.php', [6, 7, 8], "Found unused symbol 'Foobar'.")->toPhpcsJson()); $shell->registerCommand("git show '0123456789abcdef0123456789abcdef01234567':'files/test.php' | git hash-object --stdin", 'previous-file-hash'); $shell->registerCommand("git show HEAD:'files/test.php | git hash-object --stdin", 'new-file-hash'); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); diff --git a/tests/GitWorkflowWindowsTest.php b/tests/GitWorkflowWindowsTest.php index a9b97c7..38ff32d 100644 --- a/tests/GitWorkflowWindowsTest.php +++ b/tests/GitWorkflowWindowsTest.php @@ -34,8 +34,8 @@ public function testFullGitWorkflowForOneFileStagedOnWindows() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager(new TestCache()); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); @@ -59,8 +59,8 @@ public function testFullGitWorkflowForOneFileStagedWithVendorDefaultPhpcsOnWindo $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show HEAD:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); - $shell->registerCommand("git show :0:'files/foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $cache = new CacheManager(new TestCache()); $expected = $this->phpcs->getResults('bin/foobar.php', [20], 'Found unused symbol Foobar.'); diff --git a/tests/SvnWorkflowTest.php b/tests/SvnWorkflowTest.php index b628626..53a1e95 100644 --- a/tests/SvnWorkflowTest.php +++ b/tests/SvnWorkflowTest.php @@ -37,8 +37,8 @@ public function testFullSvnWorkflowForOneFile() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -58,8 +58,8 @@ public function testFullSvnWorkflowForOneFileWithReplacedSvn() { $shell->registerExecutable('cat'); $shell->registerCommand("{$svnPath} diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("{$svnPath} info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("{$svnPath} cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("{$svnPath} cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -79,8 +79,8 @@ public function testFullSvnWorkflowForOneFileWithReplacedCat() { $shell->registerExecutable($catPath); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("{$catPath} 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("{$catPath} 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -100,8 +100,8 @@ public function testFullSvnWorkflowForOneFileWithReplacedPhpcs() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -120,8 +120,8 @@ public function testFullSvnWorkflowForOneFileWithVendorDefaultPhpcs() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -142,8 +142,8 @@ public function testFullSvnWorkflowForOneFileWithNoVendorDefaultPhpcs() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -749,8 +749,8 @@ public function testFullSvnWorkflowForOneFileWithSeveritySetToZero() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs --report=json -q --standard='standard' --warning-severity='0' --error-severity='0'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("cat 'foobar.php' | phpcs --report=json -q --standard='standard' --warning-severity='0' --error-severity='0'" , $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("cat 'foobar.php'" , $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $cache = new CacheManager(new TestCache()); runSvnWorkflow([$svnFile], $options, $shell, $cache, '\PhpcsChangedTests\debug' ); diff --git a/tests/SvnWorkflowWindowsTest.php b/tests/SvnWorkflowWindowsTest.php index 06b315c..78c2ccc 100644 --- a/tests/SvnWorkflowWindowsTest.php +++ b/tests/SvnWorkflowWindowsTest.php @@ -35,9 +35,9 @@ public function testFullSvnWorkflowForOneFileOnWindows() { // Note: no 'cat' registration needed on Windows — 'type' is a built-in $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); // Windows uses 'type' instead of 'cat' for reading the modified local file - $shell->registerCommand("type 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("type 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -57,9 +57,9 @@ public function testFullSvnWorkflowForOneFileWithReplacedCatOnWindows() { $shell->registerExecutable($catPath); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); // Custom cat path is used instead of 'type' - $shell->registerCommand("{$catPath} 'foobar.php' | phpcs", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("{$catPath} 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); @@ -78,8 +78,8 @@ public function testFullSvnWorkflowForOneFileWithVendorPhpcsBatOnWindows() { $shell->registerExecutable($phpcsPath); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php')); - $shell->registerCommand("svn cat 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); - $shell->registerCommand("type 'foobar.php' | {$phpcsPath}", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); + $shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson()); + $shell->registerCommand("type 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 21])->toPhpcsJson()); $expected = $this->phpcs->getResults('bin/foobar.php', [20]); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); diff --git a/tests/helpers/Functions.php b/tests/helpers/Functions.php index 6fadc87..820d8a5 100644 --- a/tests/helpers/Functions.php +++ b/tests/helpers/Functions.php @@ -10,3 +10,31 @@ function debugWithOutput(...$messages) { var_dump($message); } } + +/** + * Synthesize the output of a batched phpcs invocation for the test shells. + * + * The real ShellRunner batch path writes each file's content to a temp file (here, the + * mocked per-file phpcs JSON registered for that file's content command) and runs a single + * phpcs over all of them, keyed by temp path. We read those temp files back and re-key each + * file's data under its temp path exactly as phpcs would, so the production batch and + * JSON-splitting logic is exercised for real rather than mocked away. + */ +function buildBatchPhpcsOutput(string $command): string { + preg_match_all('#[^\s\'"]*phpcs-changed-[^\s\'"]*#', $command, $matches); + $files = []; + foreach ($matches[0] as $tempPath) { + if (! is_file($tempPath)) { + continue; + } + $perFileJson = file_get_contents($tempPath); + $decoded = $perFileJson ? json_decode($perFileJson, true) : null; + if (! is_array($decoded) || empty($decoded['files'])) { + continue; + } + $key = realpath($tempPath) ?: $tempPath; + $files[$key] = reset($decoded['files']); + } + $output = json_encode(['totals' => ['errors' => 0, 'warnings' => 0, 'fixable' => 0], 'files' => $files]); + return $output !== false ? $output : ''; +} diff --git a/tests/helpers/TestShell.php b/tests/helpers/TestShell.php index f704d38..360d3e9 100644 --- a/tests/helpers/TestShell.php +++ b/tests/helpers/TestShell.php @@ -84,9 +84,23 @@ public function getFileHash(string $fileName): string { } public function executeCommand(string $command, ?int &$return_val = null): string { + // The real ShellRunner batch path writes each file's content to a temp file and runs a + // single phpcs over all of them. Intercept that combined invocation and synthesize its + // output from the temp files so the production batch + JSON-splitting logic runs for real. + if (strpos($command, 'phpcs-changed-') !== false && strpos($command, '--report=json') !== false) { + $return_val = 0; + return buildBatchPhpcsOutput($command); + } // Normalize double quotes to single quotes so commands registered with Unix-style // quoting (single quotes) also match on Windows where escapeshellarg() uses double quotes. $normalizedCommand = str_replace('"', "'", $command); + // Prefer an exact match so a short command (e.g. a file-contents command) does not shadow + // a longer command that has it as a prefix (e.g. that same command piped to git hash-object). + if (isset($this->commands[$normalizedCommand])) { + $return_val = $this->commands[$normalizedCommand]['return_val']; + $this->commandsCalled[$normalizedCommand] = $command; + return $this->commands[$normalizedCommand]['output']; + } foreach ($this->commands as $registeredCommand => $return) { if ($registeredCommand === substr($normalizedCommand, 0, strlen($registeredCommand)) ) { $return_val = $return['return_val']; @@ -105,28 +119,4 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } - - public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { - $new = []; - foreach ($modifiedFileNames as $fileName) { - $new[$fileName] = $this->getPhpcsOutputOfModifiedGitFile($fileName); - } - $old = []; - foreach ($unmodifiedFileNames as $fileName) { - $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedGitFile($fileName); - } - return ['new' => $new, 'old' => $old]; - } - - public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { - $new = []; - foreach ($modifiedFileNames as $fileName) { - $new[$fileName] = $this->getPhpcsOutputOfModifiedSvnFile($fileName); - } - $old = []; - foreach ($unmodifiedFileNames as $fileName) { - $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedSvnFile($fileName); - } - return ['new' => $new, 'old' => $old]; - } } diff --git a/tests/helpers/WindowsTestShell.php b/tests/helpers/WindowsTestShell.php index b84d739..c62359c 100644 --- a/tests/helpers/WindowsTestShell.php +++ b/tests/helpers/WindowsTestShell.php @@ -80,9 +80,23 @@ public function getFileHash(string $fileName): string { } public function executeCommand(string $command, ?int &$return_val = null): string { + // The real ShellRunner batch path writes each file's content to a temp file and runs a + // single phpcs over all of them. Intercept that combined invocation and synthesize its + // output from the temp files so the production batch + JSON-splitting logic runs for real. + if (strpos($command, 'phpcs-changed-') !== false && strpos($command, '--report=json') !== false) { + $return_val = 0; + return buildBatchPhpcsOutput($command); + } // Normalize double quotes to single quotes so commands registered with Unix-style // quoting (single quotes) also match on Windows where escapeshellarg() uses double quotes. $normalizedCommand = str_replace('"', "'", $command); + // Prefer an exact match so a short command (e.g. a file-contents command) does not shadow + // a longer command that has it as a prefix (e.g. that same command piped to git hash-object). + if (isset($this->commands[$normalizedCommand])) { + $return_val = $this->commands[$normalizedCommand]['return_val']; + $this->commandsCalled[$normalizedCommand] = $command; + return $this->commands[$normalizedCommand]['output']; + } foreach ($this->commands as $registeredCommand => $return) { if ($registeredCommand === substr($normalizedCommand, 0, strlen($registeredCommand))) { $return_val = $return['return_val']; @@ -101,28 +115,4 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } - - public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { - $new = []; - foreach ($modifiedFileNames as $fileName) { - $new[$fileName] = $this->getPhpcsOutputOfModifiedGitFile($fileName); - } - $old = []; - foreach ($unmodifiedFileNames as $fileName) { - $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedGitFile($fileName); - } - return ['new' => $new, 'old' => $old]; - } - - public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array { - $new = []; - foreach ($modifiedFileNames as $fileName) { - $new[$fileName] = $this->getPhpcsOutputOfModifiedSvnFile($fileName); - } - $old = []; - foreach ($unmodifiedFileNames as $fileName) { - $old[$fileName] = $this->getPhpcsOutputOfUnmodifiedSvnFile($fileName); - } - return ['new' => $new, 'old' => $old]; - } } From 84dfbae4d83e3ccf64eed0ae817d006b878dc056 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 13:10:27 -0400 Subject: [PATCH 06/12] Assert the standard option reaches batched phpcs; drop unused test imports The test shells now record the intercepted batch phpcs invocation and expose wasCommandCalledContaining(), so the standard-configured workflow tests assert that --standard reaches the phpcs command (coverage previously provided by the per-file phpcs registrations). Also removes use statements in TestShell that became unused when the per-file batch overrides were deleted. --- tests/GitWorkflowTest.php | 6 ++++++ tests/SvnWorkflowTest.php | 2 ++ tests/helpers/TestShell.php | 14 ++++++++++---- tests/helpers/WindowsTestShell.php | 10 ++++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index 6a12022..71f8d7a 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -248,6 +248,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + $this->assertTrue($shell->wasCommandCalledContaining("--standard='standard'")); + $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); @@ -288,6 +290,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + $this->assertTrue($shell->wasCommandCalledContaining("--standard='standard'")); + $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); @@ -328,6 +332,8 @@ public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCacheWith runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + $this->assertTrue($shell->wasCommandCalledContaining("--standard='standard'")); + $shell->resetCommandsCalled(); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); diff --git a/tests/SvnWorkflowTest.php b/tests/SvnWorkflowTest.php index 53a1e95..ae2dc22 100644 --- a/tests/SvnWorkflowTest.php +++ b/tests/SvnWorkflowTest.php @@ -754,6 +754,8 @@ public function testFullSvnWorkflowForOneFileWithSeveritySetToZero() { $cache = new CacheManager(new TestCache()); runSvnWorkflow([$svnFile], $options, $shell, $cache, '\PhpcsChangedTests\debug' ); + $this->assertTrue($shell->wasCommandCalledContaining("--standard='standard'")); + $cacheEntries = $cache->getEntries(); $this->assertNotEmpty($cacheEntries); foreach( $cacheEntries as $entry ) { diff --git a/tests/helpers/TestShell.php b/tests/helpers/TestShell.php index 360d3e9..fc5ffeb 100644 --- a/tests/helpers/TestShell.php +++ b/tests/helpers/TestShell.php @@ -4,10 +4,6 @@ namespace PhpcsChangedTests; use PhpcsChanged\CliOptions; -use PhpcsChanged\Modes; -use PhpcsChanged\ShellOperator; -use PhpcsChanged\ShellException; -use PhpcsChanged\NoChangesException; use PhpcsChanged\UnixShell; class TestShell extends UnixShell { @@ -89,6 +85,7 @@ public function executeCommand(string $command, ?int &$return_val = null): strin // output from the temp files so the production batch + JSON-splitting logic runs for real. if (strpos($command, 'phpcs-changed-') !== false && strpos($command, '--report=json') !== false) { $return_val = 0; + $this->commandsCalled[$command] = $command; return buildBatchPhpcsOutput($command); } // Normalize double quotes to single quotes so commands registered with Unix-style @@ -119,4 +116,13 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } + + public function wasCommandCalledContaining(string $needle): bool { + foreach ($this->commandsCalled as $calledCommand) { + if (strpos($calledCommand, $needle) !== false) { + return true; + } + } + return false; + } } diff --git a/tests/helpers/WindowsTestShell.php b/tests/helpers/WindowsTestShell.php index c62359c..93bd338 100644 --- a/tests/helpers/WindowsTestShell.php +++ b/tests/helpers/WindowsTestShell.php @@ -85,6 +85,7 @@ public function executeCommand(string $command, ?int &$return_val = null): strin // output from the temp files so the production batch + JSON-splitting logic runs for real. if (strpos($command, 'phpcs-changed-') !== false && strpos($command, '--report=json') !== false) { $return_val = 0; + $this->commandsCalled[$command] = $command; return buildBatchPhpcsOutput($command); } // Normalize double quotes to single quotes so commands registered with Unix-style @@ -115,4 +116,13 @@ public function resetCommandsCalled(): void { public function wasCommandCalled(string $registeredCommand): bool { return isset($this->commandsCalled[$registeredCommand]); } + + public function wasCommandCalledContaining(string $needle): bool { + foreach ($this->commandsCalled as $calledCommand) { + if (strpos($calledCommand, $needle) !== false) { + return true; + } + } + return false; + } } From aef76ac97b35cd8e90b0733e233fd588f251003d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 13:20:23 -0400 Subject: [PATCH 07/12] Make standard-option assertion quote-agnostic for Windows wasCommandCalledContaining matched the raw recorded command, but on Windows escapeshellarg() emits double quotes, so --standard="standard" never matched the single-quoted needle. Normalize double quotes to single quotes before comparing, matching how the test shells already normalize commands. --- tests/helpers/TestShell.php | 4 +++- tests/helpers/WindowsTestShell.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/helpers/TestShell.php b/tests/helpers/TestShell.php index fc5ffeb..d1cf9a4 100644 --- a/tests/helpers/TestShell.php +++ b/tests/helpers/TestShell.php @@ -119,7 +119,9 @@ public function wasCommandCalled(string $registeredCommand): bool { public function wasCommandCalledContaining(string $needle): bool { foreach ($this->commandsCalled as $calledCommand) { - if (strpos($calledCommand, $needle) !== false) { + // Normalize double quotes to single quotes so a needle written with Unix-style + // quoting also matches on Windows where escapeshellarg() uses double quotes. + if (strpos(str_replace('"', "'", $calledCommand), $needle) !== false) { return true; } } diff --git a/tests/helpers/WindowsTestShell.php b/tests/helpers/WindowsTestShell.php index 93bd338..5d8c72c 100644 --- a/tests/helpers/WindowsTestShell.php +++ b/tests/helpers/WindowsTestShell.php @@ -119,7 +119,9 @@ public function wasCommandCalled(string $registeredCommand): bool { public function wasCommandCalledContaining(string $needle): bool { foreach ($this->commandsCalled as $calledCommand) { - if (strpos($calledCommand, $needle) !== false) { + // Normalize double quotes to single quotes so a needle written with Unix-style + // quoting also matches on Windows where escapeshellarg() uses double quotes. + if (strpos(str_replace('"', "'", $calledCommand), $needle) !== false) { return true; } } From 5e98ce0cf39bd5ff8d498a509bf17d9fd712d84f Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 16:29:46 -0400 Subject: [PATCH 08/12] Fail batch phpcs when it cannot produce a JSON report The batch path silently swallowed phpcs processing errors (eg: an uninstalled standard). phpcs writes a non-JSON error to stdout in that case, and runBatchPhpcs returned an empty result, which mapped each file to empty output and produced a bogus phantom STDIN success with exit 0. Treat any phpcs output that is not decodable JSON with a 'files' key as a failure and throw a ShellException, restoring the pre-batch behavior where a processing error surfaces and exits non-zero. --- PhpcsChanged/ShellRunner.php | 17 +++++++++++------ tests/GitWorkflowTest.php | 22 ++++++++++++++++++++++ tests/helpers/Functions.php | 6 ++++++ 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index 316d58b..b847c86 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -353,18 +353,23 @@ private function runBatchPhpcs(array $tempToOriginal): array { if (empty($tempToOriginal)) { return []; } + $debug = getDebug($this->options->debug); $phpcs = $this->getPhpcsExecutable(); $args = implode(' ', array_map('escapeshellarg', array_keys($tempToOriginal))); $command = "{$phpcs} --report=json -q" . $this->getPhpcsStandardOption() . $this->getPhpcsExtensionsOption() . ' ' . $args; + $debug('running batch phpcs command:', $command); $phpcsOutput = $this->platform->executeCommand($command); - - if (! $phpcsOutput) { - return []; - } - + $debug('batch phpcs command output:', $phpcsOutput); + + // When phpcs cannot run (eg: a missing coding standard) it writes a plain-text + // error to stdout rather than valid JSON. We do not key off the exit code because + // phpcs exits non-zero (1/2) as its normal "found errors/warnings" result, while a + // non-decodable JSON response reliably means phpcs failed to produce a report. Treat + // any output that is not JSON with a 'files' key as a failure so we surface the phpcs + // error instead of silently reporting success. $decoded = json_decode($phpcsOutput, true); if (! is_array($decoded) || ! isset($decoded['files'])) { - return []; + throw new ShellException("Failed to run phpcs on batch of files; phpcs output: " . var_export($phpcsOutput, true)); } $results = []; diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index 71f8d7a..f3b7956 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -44,6 +44,28 @@ public function testFullGitWorkflowForOneFileStaged() { $this->assertEquals($expected->getMessages(), $messages->getMessages()); } + public function testFullGitWorkflowThrowsWhenBatchPhpcsErrors() { + // When phpcs cannot run (eg: an uninstalled standard) it writes a non-JSON error to + // stdout and exits non-zero. The batch path must surface this as a failure rather than + // silently reporting a bogus success with zero messages. + $gitFile = 'foobar.php'; + $options = CliOptions::fromArray(['no-cache-git-root' => false, 'git-staged' => false, 'files' => [$gitFile]]); + $shell = new TestShell($options, [$gitFile]); + $shell->registerExecutable('git'); + $shell->registerExecutable('phpcs'); + $fixture = $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;'); + $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); + $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); + $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); + $phpcsError = 'BATCH_PHPCS_RAW:ERROR: the "WordPress-Core" coding standard is not installed.'; + $shell->registerCommand("git show HEAD:'files/foobar.php'", $phpcsError); + $shell->registerCommand("git show :0:'files/foobar.php'", $phpcsError); + $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); + $cache = new CacheManager( new TestCache() ); + $this->expectException(ShellException::class); + runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + } + public function testFullGitWorkflowForOneFileStagedWithReplacedGit() { $gitFile = 'foobar.php'; $gitPath = 'bin/foo/git'; diff --git a/tests/helpers/Functions.php b/tests/helpers/Functions.php index 820d8a5..1649a01 100644 --- a/tests/helpers/Functions.php +++ b/tests/helpers/Functions.php @@ -28,6 +28,12 @@ function buildBatchPhpcsOutput(string $command): string { continue; } $perFileJson = file_get_contents($tempPath); + // A file's content beginning with this sentinel simulates phpcs failing on the whole + // batch (eg: an uninstalled standard), where phpcs writes a non-JSON error to stdout + // instead of valid JSON. Emit the raw error as the entire batch output. + if (is_string($perFileJson) && strpos($perFileJson, 'BATCH_PHPCS_RAW:') === 0) { + return substr($perFileJson, strlen('BATCH_PHPCS_RAW:')); + } $decoded = $perFileJson ? json_decode($perFileJson, true) : null; if (! is_array($decoded) || empty($decoded['files'])) { continue; From 0d823e1907bb61744524eef6a719599de7e03eae Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 16:57:28 -0400 Subject: [PATCH 09/12] Preserve exact file bytes when writing batch temp files The batch path materialized each file's content by capturing the content command (git show / cat) through the line-oriented executeCommand(), which rejoins exec() output and unconditionally appends a trailing newline (and collapses runs of trailing blank lines). The temp file phpcs scanned therefore always ended in \n, so the batch path missed PSR2.Files.EndFileNewline.NoneFound (and .TooMany) for files that genuinely lack a trailing newline. Add ShellPlatform::writeCommandOutputToFile(), which redirects the content command's stdout straight to the temp file, preserving exact bytes. writeTempFile() now uses it and throws when the content command fails, so a fetch failure surfaces instead of producing a silently-empty temp file. The mocked TestShell cannot reproduce the exec() byte corruption, so add a UnixShellTest exercising the real shell to lock in the byte-preserving contract. --- PhpcsChanged/ShellPlatform.php | 11 ++++ PhpcsChanged/ShellRunner.php | 10 +++- PhpcsChanged/UnixShell.php | 8 +++ PhpcsChanged/WindowsShell.php | 8 +++ tests/GitWorkflowTest.php | 8 ++- tests/UnixShellTest.php | 80 ++++++++++++++++++++++++++++++ tests/helpers/TestShell.php | 10 ++++ tests/helpers/WindowsTestShell.php | 10 ++++ 8 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 tests/UnixShellTest.php diff --git a/PhpcsChanged/ShellPlatform.php b/PhpcsChanged/ShellPlatform.php index 665d43b..7ef1d44 100644 --- a/PhpcsChanged/ShellPlatform.php +++ b/PhpcsChanged/ShellPlatform.php @@ -14,6 +14,17 @@ interface ShellPlatform { */ public function executeCommand(string $command, ?int &$return_val = null): string; + /** + * Run $command and write its stdout to $filePath, preserving the exact bytes. + * + * Unlike executeCommand(), which is line-oriented and normalizes trailing + * newlines, this redirects the command's stdout straight to the file so that + * file content (e.g. from `git show` or `cat`) reaches phpcs byte-for-byte. + * + * @return int The command's exit code. + */ + public function writeCommandOutputToFile(string $command, string $filePath): int; + /** * Validate that an executable exists and is runnable. * diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index b847c86..ea129e0 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -341,8 +341,14 @@ private function writeTempFile(string $contentCommand, string $tempPath): void { if (! is_dir($dir)) { mkdir($dir, 0777, true); } - $content = $this->platform->executeCommand($contentCommand); - file_put_contents($tempPath, $content); + // Redirect the content command's stdout straight to the temp file rather than + // round-tripping through the line-oriented executeCommand(), which would force a + // trailing newline and collapse trailing blank lines. phpcs must scan the file's + // exact bytes so that eg: PSR2.Files.EndFileNewline violations are detected. + $returnVal = $this->platform->writeCommandOutputToFile($contentCommand, $tempPath); + if ($returnVal !== 0) { + throw new ShellException("Cannot get file contents for temp file '{$tempPath}'; command failed with code {$returnVal}: {$contentCommand}"); + } } /** diff --git a/PhpcsChanged/UnixShell.php b/PhpcsChanged/UnixShell.php index 27787bf..ca34fe4 100644 --- a/PhpcsChanged/UnixShell.php +++ b/PhpcsChanged/UnixShell.php @@ -41,6 +41,14 @@ public function executeCommand(string $command, ?int &$return_val = null): strin return implode(PHP_EOL, $output) . PHP_EOL; } + #[\Override] + public function writeCommandOutputToFile(string $command, string $filePath): int { + $output = []; + $return_val = 0; + exec($command . ' > ' . escapeshellarg($filePath), $output, $return_val); + return $return_val; + } + #[\Override] public function validateExecutableExists(string $name, string $command): void { exec(sprintf("type %s > /dev/null 2>&1", escapeshellarg($command)), $ignore, $returnVal); diff --git a/PhpcsChanged/WindowsShell.php b/PhpcsChanged/WindowsShell.php index 54634fd..d33a671 100644 --- a/PhpcsChanged/WindowsShell.php +++ b/PhpcsChanged/WindowsShell.php @@ -45,6 +45,14 @@ public function executeCommand(string $command, ?int &$return_val = null): strin return implode(PHP_EOL, $output) . PHP_EOL; } + #[\Override] + public function writeCommandOutputToFile(string $command, string $filePath): int { + $output = []; + $return_val = 0; + exec($command . ' > ' . escapeshellarg($filePath), $output, $return_val); + return $return_val; + } + #[\Override] public function validateExecutableExists(string $name, string $command): void { // Full or relative path — check that the file exists on disk diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index f3b7956..cc3cd17 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -574,12 +574,10 @@ public function testFullGitWorkflowForEmptyNewFile() { $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getNewFileInfo('foobar.php')); $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); - $fixture ='ERROR: You must supply at least one file or directory to process. - -Run "phpcs --help" for usage information -'; $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); - $shell->registerCommand("git show :0:'files/foobar.php", $fixture, 1); + // An empty staged new file: `git show :0:` succeeds (exit 0) with empty content, so the + // batch path writes an empty temp file and phpcs reports no messages for it. + $shell->registerCommand("git show :0:'files/foobar.php", '', 0); $cache = new CacheManager( new TestCache() ); $expected = PhpcsMessages::fromArrays([], '/dev/null'); diff --git a/tests/UnixShellTest.php b/tests/UnixShellTest.php new file mode 100644 index 0000000..0933858 --- /dev/null +++ b/tests/UnixShellTest.php @@ -0,0 +1,80 @@ +tmpFiles as $file) { + if (is_file($file)) { + unlink($file); + } + } + $this->tmpFiles = []; + parent::tearDown(); + } + + private function makeTempFile(string $contents): string { + $path = tempnam(sys_get_temp_dir(), 'phpcs-changed-test-'); + $this->tmpFiles[] = $path; + file_put_contents($path, $contents); + return $path; + } + + private function shell(): UnixShell { + return new UnixShell(CliOptions::fromArray(['git-staged' => true, 'files' => ['foo.php']])); + } + + /** + * @dataProvider provideExactByteContents + */ + public function testWriteCommandOutputToFilePreservesExactBytes(string $contents) { + if (PHP_OS_FAMILY === 'Windows') { + $this->markTestSkipped('UnixShell test does not run on Windows'); + } + $source = $this->makeTempFile($contents); + $dest = tempnam(sys_get_temp_dir(), 'phpcs-changed-test-out-'); + $this->tmpFiles[] = $dest; + + $shell = $this->shell(); + $returnVal = $shell->writeCommandOutputToFile('cat ' . escapeshellarg($source), $dest); + + $this->assertSame(0, $returnVal); + $this->assertSame($contents, file_get_contents($dest), 'temp file should be byte-identical to the source'); + } + + public function provideExactByteContents(): array { + return [ + 'no trailing newline' => [" [" [" [''], + ]; + } + + public function testWriteCommandOutputToFileReturnsNonZeroWhenCommandFails() { + if (PHP_OS_FAMILY === 'Windows') { + $this->markTestSkipped('UnixShell test does not run on Windows'); + } + $dest = tempnam(sys_get_temp_dir(), 'phpcs-changed-test-out-'); + $this->tmpFiles[] = $dest; + + $shell = $this->shell(); + $missing = sys_get_temp_dir() . '/phpcs-changed-does-not-exist-' . getmypid(); + $returnVal = $shell->writeCommandOutputToFile('cat ' . escapeshellarg($missing) . ' 2>/dev/null', $dest); + + $this->assertNotSame(0, $returnVal); + } +} diff --git a/tests/helpers/TestShell.php b/tests/helpers/TestShell.php index d1cf9a4..5bf40d4 100644 --- a/tests/helpers/TestShell.php +++ b/tests/helpers/TestShell.php @@ -79,6 +79,16 @@ public function getFileHash(string $fileName): string { return $this->fileHashes[$fileName] ?? $fileName; } + public function writeCommandOutputToFile(string $command, string $filePath): int { + // The real shell redirects the content command's stdout to the file to preserve exact + // bytes. Here we capture the registered output and write it verbatim (file_put_contents + // does not alter bytes), keeping the batch test harness working. + $return_val = 0; + $content = $this->executeCommand($command, $return_val); + file_put_contents($filePath, $content); + return $return_val; + } + public function executeCommand(string $command, ?int &$return_val = null): string { // The real ShellRunner batch path writes each file's content to a temp file and runs a // single phpcs over all of them. Intercept that combined invocation and synthesize its diff --git a/tests/helpers/WindowsTestShell.php b/tests/helpers/WindowsTestShell.php index 5d8c72c..9f82501 100644 --- a/tests/helpers/WindowsTestShell.php +++ b/tests/helpers/WindowsTestShell.php @@ -79,6 +79,16 @@ public function getFileHash(string $fileName): string { return $this->fileHashes[$fileName] ?? $fileName; } + public function writeCommandOutputToFile(string $command, string $filePath): int { + // The real shell redirects the content command's stdout to the file to preserve exact + // bytes. Here we capture the registered output and write it verbatim (file_put_contents + // does not alter bytes), keeping the batch test harness working. + $return_val = 0; + $content = $this->executeCommand($command, $return_val); + file_put_contents($filePath, $content); + return $return_val; + } + public function executeCommand(string $command, ?int &$return_val = null): string { // The real ShellRunner batch path writes each file's content to a temp file and runs a // single phpcs over all of them. Intercept that combined invocation and synthesize its From f45807de71c5114b84ac859420dfed09a32e28e3 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 17:42:25 -0400 Subject: [PATCH 10/12] Model real empty-new-file behavior in git workflow test testFullGitWorkflowForEmptyNewFile mocked git show returning exit 1 with phpcs's 'You must supply at least one file' text, which described the old per-file path that piped empty stdin into phpcs. The batch path instead writes the empty content to a temp file and passes it as an argument, so git show succeeds (exit 0) and phpcs emits an Internal.NoCodeFound warning. Because the file is new, that warning is reported as a new message. Update the test to register that real behavior and assert the warning is reported. --- tests/GitWorkflowTest.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index cc3cd17..86129f8 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -576,11 +576,21 @@ public function testFullGitWorkflowForEmptyNewFile() { $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); // An empty staged new file: `git show :0:` succeeds (exit 0) with empty content, so the - // batch path writes an empty temp file and phpcs reports no messages for it. - $shell->registerCommand("git show :0:'files/foobar.php", '', 0); + // batch path writes an empty temp file. phpcs then reports an "Internal.NoCodeFound" + // warning for the empty file, and since the file is new that warning is a new message. + $noCodeFound = [[ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 1, + 'source' => 'Internal.NoCodeFound', + 'line' => 1, + 'message' => 'No PHP code was found in this file and short open tags are not allowed by this install of PHP. This file may be using short open tags but PHP does not allow them.', + ]]; + $shell->registerCommand("git show :0:'files/foobar.php'", PhpcsMessages::fromArrays($noCodeFound, 'STDIN')->toPhpcsJson(), 0); $cache = new CacheManager( new TestCache() ); - $expected = PhpcsMessages::fromArrays([], '/dev/null'); + $expected = PhpcsMessages::fromArrays($noCodeFound, 'foobar.php'); $messages = runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); } From 550a1de14d89f3878f825fcbc02063a60880c77a Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 17:43:43 -0400 Subject: [PATCH 11/12] Model real empty-new-file behavior in svn workflow test testFullSvnWorkflowForEmptyNewFile mocked cat returning phpcs's 'You must supply at least one file' text, describing the old per-file path that piped empty stdin into phpcs. The batch path writes the empty content to a temp file and passes it as an argument, so cat succeeds and phpcs emits an Internal.NoCodeFound warning that is reported as a new message for the new file. Mirror the git workflow test fix. --- tests/SvnWorkflowTest.php | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/SvnWorkflowTest.php b/tests/SvnWorkflowTest.php index ae2dc22..ebeecd6 100644 --- a/tests/SvnWorkflowTest.php +++ b/tests/SvnWorkflowTest.php @@ -667,12 +667,20 @@ public function testFullSvnWorkflowForEmptyNewFile() { $shell->registerExecutable('cat'); $shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getNewFileDiff('foobar.php')); $shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfoNewFile('foobar.php')); - $fixture = 'ERROR: You must supply at least one file or directory to process. - -Run "phpcs --help" for usage information -'; - $shell->registerCommand( "cat 'foobar.php'", $fixture); - $expected = PhpcsMessages::fromArrays([], 'STDIN'); + // An empty new file: `cat` succeeds (exit 0) with empty content, so the batch path writes + // an empty temp file. phpcs then reports an "Internal.NoCodeFound" warning for the empty + // file, and since the file is new that warning is a new message. + $noCodeFound = [[ + 'type' => 'WARNING', + 'severity' => 5, + 'fixable' => false, + 'column' => 1, + 'source' => 'Internal.NoCodeFound', + 'line' => 1, + 'message' => 'No PHP code was found in this file and short open tags are not allowed by this install of PHP. This file may be using short open tags but PHP does not allow them.', + ]]; + $shell->registerCommand("cat 'foobar.php'", PhpcsMessages::fromArrays($noCodeFound, 'STDIN')->toPhpcsJson()); + $expected = PhpcsMessages::fromArrays($noCodeFound, 'foobar.php'); $messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug'); $this->assertEquals($expected->getMessages(), $messages->getMessages()); } From 0650dc1036b73c14a9affed5fccd81991139ccd9 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sat, 20 Jun 2026 18:30:54 -0400 Subject: [PATCH 12/12] Pass batched phpcs files via --file-list instead of inline args Inlining one shell argument per temp file overflowed the OS ARG_MAX limit once a batch reached thousands of files, failing with a cryptic "Argument list too long". Write all temp paths to a phpcs --file-list file in the batch temp dir instead, keeping the command line a constant size regardless of file count. This also handles paths with spaces cleanly. Update the test mock shell to read the file list and add a regression guard asserting files are never inlined as phpcs arguments. --- PhpcsChanged/ShellRunner.php | 18 +++++++++++++----- tests/GitWorkflowTest.php | 26 ++++++++++++++++++++++++++ tests/helpers/Functions.php | 28 ++++++++++++++++++++++++++-- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/PhpcsChanged/ShellRunner.php b/PhpcsChanged/ShellRunner.php index ea129e0..828e98c 100644 --- a/PhpcsChanged/ShellRunner.php +++ b/PhpcsChanged/ShellRunner.php @@ -353,16 +353,24 @@ private function writeTempFile(string $contentCommand, string $tempPath): void { /** * @param array $tempToOriginal Maps temp file path => original file path + * @param string $tempDir The batch temp directory; the --file-list file is written here so it is cleaned up with the rest of the batch * @return array Maps temp file path => single-file phpcs JSON string */ - private function runBatchPhpcs(array $tempToOriginal): array { + private function runBatchPhpcs(array $tempToOriginal, string $tempDir): array { if (empty($tempToOriginal)) { return []; } $debug = getDebug($this->options->debug); $phpcs = $this->getPhpcsExecutable(); - $args = implode(' ', array_map('escapeshellarg', array_keys($tempToOriginal))); - $command = "{$phpcs} --report=json -q" . $this->getPhpcsStandardOption() . $this->getPhpcsExtensionsOption() . ' ' . $args; + // Pass the files to scan via a phpcs --file-list file rather than as command-line + // arguments. Inlining one argument per file overflows the OS ARG_MAX limit (and fails + // with a cryptic "Argument list too long") once a batch reaches thousands of files; a + // file list keeps the command line a constant size regardless of how many files we scan. + $listFile = $tempDir . '/phpcs-file-list.txt'; + if (file_put_contents($listFile, implode("\n", array_keys($tempToOriginal))) === false) { + throw new ShellException("Cannot write phpcs file list to '{$listFile}'"); + } + $command = "{$phpcs} --report=json -q" . $this->getPhpcsStandardOption() . $this->getPhpcsExtensionsOption() . ' --file-list=' . escapeshellarg($listFile); $debug('running batch phpcs command:', $command); $phpcsOutput = $this->platform->executeCommand($command); $debug('batch phpcs command output:', $phpcsOutput); @@ -447,7 +455,7 @@ public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmod $unmodifiedTempToOriginal[$tempPath] = $fileName; } $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; - $allResults = $this->runBatchPhpcs($allTempToOriginal); + $allResults = $this->runBatchPhpcs($allTempToOriginal, $tempDir); return [ 'new' => $this->mapBatchResultsToOriginalFiles($allResults, $modifiedTempToOriginal), 'old' => $this->mapBatchResultsToOriginalFiles($allResults, $unmodifiedTempToOriginal), @@ -485,7 +493,7 @@ public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmod $unmodifiedTempToOriginal[$tempPath] = $fileName; } $allTempToOriginal = $modifiedTempToOriginal + $unmodifiedTempToOriginal; - $allResults = $this->runBatchPhpcs($allTempToOriginal); + $allResults = $this->runBatchPhpcs($allTempToOriginal, $tempDir); return [ 'new' => $this->mapBatchResultsToOriginalFiles($allResults, $modifiedTempToOriginal), 'old' => $this->mapBatchResultsToOriginalFiles($allResults, $unmodifiedTempToOriginal), diff --git a/tests/GitWorkflowTest.php b/tests/GitWorkflowTest.php index 86129f8..9526ac7 100644 --- a/tests/GitWorkflowTest.php +++ b/tests/GitWorkflowTest.php @@ -44,6 +44,32 @@ public function testFullGitWorkflowForOneFileStaged() { $this->assertEquals($expected->getMessages(), $messages->getMessages()); } + public function testFullGitWorkflowBatchesPhpcsViaFileListNotCommandLineArgs() { + // Files to scan must be passed to phpcs through a --file-list file, never inlined as + // command-line arguments. Inlining one argument per file overflows the OS ARG_MAX limit + // once a batch reaches thousands of files; the file list keeps the command line a + // constant size regardless of how many files are scanned. + $gitFile = 'foobar.php'; + $options = CliOptions::fromArray(['no-cache-git-root' => false, 'git-staged' => false, 'files' => [$gitFile]]); + $shell = new TestShell($options, [$gitFile]); + $shell->registerExecutable('git'); + $shell->registerExecutable('phpcs'); + $fixture = $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;'); + $shell->registerCommand("git diff --staged --no-prefix 'foobar.php'", $fixture); + $shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php')); + $shell->registerCommand("git ls-files --full-name 'foobar.php'", "files/foobar.php"); + $shell->registerCommand("git show HEAD:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20])->toPhpcsJson()); + $shell->registerCommand("git show :0:'files/foobar.php'", $this->phpcs->getResults('STDIN', [20, 21], 'Found unused symbol Foobar.')->toPhpcsJson()); + $shell->registerCommand("git rev-parse --show-toplevel", 'run-from-git-root'); + $cache = new CacheManager( new TestCache() ); + runGitWorkflow($options, $shell, $cache, '\PhpcsChangedTests\Debug'); + $this->assertTrue($shell->wasCommandCalledContaining('--report=json'), 'expected a batch phpcs invocation'); + $this->assertTrue($shell->wasCommandCalledContaining('--file-list='), 'batch phpcs must pass files via --file-list'); + // The temp file paths live under the batch temp dir; none of them should appear inline as + // command-line arguments alongside --report=json. + $this->assertFalse($shell->wasCommandCalledContaining("/new/foobar.php"), 'temp file paths must not be inlined as phpcs arguments'); + } + public function testFullGitWorkflowThrowsWhenBatchPhpcsErrors() { // When phpcs cannot run (eg: an uninstalled standard) it writes a non-JSON error to // stdout and exits non-zero. The batch path must surface this as a failure rather than diff --git a/tests/helpers/Functions.php b/tests/helpers/Functions.php index 1649a01..6668f4e 100644 --- a/tests/helpers/Functions.php +++ b/tests/helpers/Functions.php @@ -21,9 +21,9 @@ function debugWithOutput(...$messages) { * JSON-splitting logic is exercised for real rather than mocked away. */ function buildBatchPhpcsOutput(string $command): string { - preg_match_all('#[^\s\'"]*phpcs-changed-[^\s\'"]*#', $command, $matches); + $tempPaths = readBatchPhpcsFileList($command); $files = []; - foreach ($matches[0] as $tempPath) { + foreach ($tempPaths as $tempPath) { if (! is_file($tempPath)) { continue; } @@ -44,3 +44,27 @@ function buildBatchPhpcsOutput(string $command): string { $output = json_encode(['totals' => ['errors' => 0, 'warnings' => 0, 'fixable' => 0], 'files' => $files]); return $output !== false ? $output : ''; } + +/** + * Extract the temp file paths a batched phpcs invocation will scan. + * + * The real ShellRunner passes the files to scan via a phpcs --file-list file (rather than as + * command-line arguments) so the batch never overflows ARG_MAX. Read that list back so the test + * shells see exactly the files phpcs would, regardless of how many there are. + * + * @return string[] + */ +function readBatchPhpcsFileList(string $command): array { + if (! preg_match('#--file-list=([\'"]?)(.+?)\1(?:\s|$)#', $command, $match)) { + return []; + } + $listFile = $match[2]; + if (! is_file($listFile)) { + return []; + } + $contents = file_get_contents($listFile); + if ($contents === false || $contents === '') { + return []; + } + return array_values(array_filter(explode("\n", $contents), 'strlen')); +}