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]; + } }