diff --git a/README.md b/README.md index 5ee464c..4be5799 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,26 @@ Register it in your config: ``` +## CLI Scope + +By default, DocbookCS checks the current Git diff from its upstream branch point +through the working tree. Alternatively, a unified diff can be piped or file and +directory paths passed. The inspection scope is limited to the given diff or the +full contents of the given file paths. + +XML references are expanded by default, but reported violations remain limited +to the given scope. With `--wide`, every file inferred from paths or a diff is +checked as a whole, and referenced `SYSTEM` XML files are recursively included. + +| Input | `--wide` | Full File(s) | References | +|------------|---------:|-------------:|-----------:| +| none | no | no | no | +| none | yes | yes | yes | +| path | no | yes | no | +| path | yes | yes | yes | +| piped diff | no | no | no | +| piped diff | yes | yes | yes | + ## License Apache 2.0 diff --git a/bin/docbook-cs b/bin/docbook-cs index 45e5abf..68d109a 100644 --- a/bin/docbook-cs +++ b/bin/docbook-cs @@ -25,4 +25,4 @@ use DocbookCS\Application; exit(2); })(); -new Application($argv ?? [])->run(); +exit(Application::withArguments($argv ?? [])->run()); diff --git a/phpstan.neon b/phpstan.neon index a7c21c0..1b51da6 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,7 @@ parameters: paths: - bin/ + - bin/docbook-cs # extension-less file - src/ - tests/ diff --git a/phpunit.xml.dist b/phpunit.xml.dist index eaa454f..f173b58 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,9 +18,6 @@ tests/Unit - - tests/Integration - diff --git a/src/Application.php b/src/Application.php index 9827b56..f7cc8fe 100644 --- a/src/Application.php +++ b/src/Application.php @@ -7,7 +7,6 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; -use DocbookCS\Diff\DiffParser; use DocbookCS\Progress\ConsoleProgress; use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; @@ -15,6 +14,7 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Report\Reporter\ReporterInterface; +use DocbookCS\Runner\RunPlanner; use DocbookCS\Runner\SniffRunner; final class Application @@ -23,30 +23,51 @@ final class Application private const string DEFAULT_CONFIG = 'docbookcs.xml'; - /** @var list */ - private array $argv; - /** @var resource */ private $stdout; /** @var resource */ private $stderr; - /** @var resource */ - private $stdin; + /** + * @param list $argv + * @throws \RuntimeException if redirected stdin cannot be read. + */ + public static function withArguments(array $argv): self + { + $stdin = null; + $stat = fstat(STDIN); + + if ($stat === false) { + return new self($argv, unifiedDiff: $stdin); + } + + $type = $stat['mode'] & 0170000; + + if ($type === 0010000 || $type === 0100000) { + $stdin = stream_get_contents(STDIN); + + if ($stdin === false) { + throw new \RuntimeException('Could not read diff from stdin.'); + } + } + + return new self($argv, unifiedDiff: $stdin); + } /** * @param list $argv * @param ?resource $stdout * @param ?resource $stderr - * @param ?resource $stdin */ - public function __construct(array $argv, mixed $stdout = null, mixed $stderr = null, mixed $stdin = null) - { - $this->argv = $argv; + public function __construct( + private readonly array $argv, + mixed $stdout = null, + mixed $stderr = null, + private readonly ?string $unifiedDiff = null, + ) { $this->stdout = $stdout ?? STDOUT; $this->stderr = $stderr ?? STDERR; - $this->stdin = $stdin ?? STDIN; } /** @@ -54,7 +75,13 @@ public function __construct(array $argv, mixed $stdout = null, mixed $stderr = n */ public function run(): int { - $options = $this->parseArgv(); + try { + $options = $this->parseArgv(); + } catch (\InvalidArgumentException $e) { + $this->writeError('Error: ' . $e->getMessage() . PHP_EOL); + + return 2; + } if ($options['help']) { $this->printHelp(); @@ -76,31 +103,19 @@ public function run(): int return 2; } - $overridePaths = $options['paths'] !== [] ? $options['paths'] : null; - - // If override paths are relative, resolve them against cwd. - if ($overridePaths !== null) { - $overridePaths = $this->resolveOverridePaths($overridePaths); - } - - $diff = null; - - if ($options['diff'] !== null) { - try { - $diffContent = $this->readDiff($options['diff']); - $diff = (new DiffParser())->parse($diffContent); - } catch (\Throwable $e) { - $this->writeError('Error reading diff: ' . $e->getMessage() . PHP_EOL); + try { + $runPlan = new RunPlanner($config, $options['wide'])->plan($options['paths'], $this->unifiedDiff); + } catch (\Throwable $e) { + $this->writeError('Error resolving input: ' . $e->getMessage() . PHP_EOL); - return 2; - } + return 2; } $progress = $this->createProgress($options); try { $runner = new SniffRunner($progress); - $report = $runner->run($config, $overridePaths, $diff); + $report = $runner->run($runPlan); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); @@ -118,27 +133,6 @@ public function run(): int return (int) $report->hasViolations(); } - /** - * @param list $paths - * @return list - */ - private function resolveOverridePaths(array $paths): array - { - $cwd = getcwd() ?: '.'; - $resolved = []; - - foreach ($paths as $path) { - if (str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $path)) { - $resolved[] = $path; - continue; - } - - $resolved[] = $cwd . '/' . $path; - } - - return $resolved; - } - /** * @return array{ * help: bool, @@ -148,9 +142,10 @@ private function resolveOverridePaths(array $paths): array * colors: bool, * quiet: bool, * paths: list, - * diff: string|null, + * wide: bool, * perf: bool, * } + * @throws \InvalidArgumentException for unsupported options. */ private function parseArgv(): array { @@ -162,7 +157,7 @@ private function parseArgv(): array 'colors' => $this->detectColorSupport(), 'quiet' => false, 'paths' => [], - 'diff' => null, + 'wide' => false, 'perf' => false, ]; @@ -227,23 +222,14 @@ private function parseArgv(): array continue; } - // --diff = read from stdin - // --diff=FILE = read from file - // --diff=- = read from stdin (explicit) - if ($arg === '--diff') { - $result['diff'] = ''; - $i++; - continue; - } - - if (str_starts_with($arg, '--diff=')) { - $result['diff'] = substr($arg, 7); + if ($arg === '--perf') { + $result['perf'] = true; $i++; continue; } - if ($arg === '--perf') { - $result['perf'] = true; + if ($arg === '--wide') { + $result['wide'] = true; $i++; continue; } @@ -251,33 +237,16 @@ private function parseArgv(): array // Anything else is a path to scan. if (!str_starts_with($arg, '-')) { $result['paths'][] = $arg; + $i++; + continue; } - $i++; + throw new \InvalidArgumentException(sprintf('Unknown option: %s', $arg)); } return $result; } - /** @throws \RuntimeException if the source cannot be read. */ - private function readDiff(string $source): string - { - if ($source === '' || $source === '-') { - $content = stream_get_contents($this->stdin); - if ($content === false) { - throw new \RuntimeException('Could not read diff from stdin.'); // @codeCoverageIgnore - } - return $content; - } - - $content = @file_get_contents($source); - if ($content === false) { - throw new \RuntimeException(sprintf('Could not read diff file: %s', $source)); - } - - return $content; - } - /** @param array{report: string, quiet: bool, colors: bool} $options */ private function createProgress(array $options): ProgressInterface { @@ -382,22 +351,20 @@ private function printHelp(): void --report= Output format: console (default), checkstyle, json. --colors Force ANSI color output. --no-colors Disable ANSI color output. - --diff[=] Restrict analysis to files changed in a unified diff. - Omit the value or pass "-" to read the diff from stdin. - Violations are only reported when the violating element - is on or contains a changed line (parent-context aware). + --wide Check whole selected files and recursively include + referenced XML files. Arguments: One or more files or directories to scan. - If omitted, the paths from the config file are used. + Paths cannot be combined with diff input. Examples: docbook-cs docbook-cs --config=myconfig.xml reference/ docbook-cs --report=checkstyle --no-colors > report.xml docbook-cs reference/strings/functions/strlen.xml - git diff HEAD | docbook-cs --diff --report=checkstyle - docbook-cs --diff=changes.patch --report=json + git diff HEAD | docbook-cs + git diff HEAD | docbook-cs --wide --report=checkstyle HELP; diff --git a/src/Diff/DiffBaseResolver.php b/src/Diff/DiffBaseResolver.php new file mode 100644 index 0000000..c4dd306 --- /dev/null +++ b/src/Diff/DiffBaseResolver.php @@ -0,0 +1,95 @@ +repositoryName($repoRoot); + + if ($repoName === null) { + return $this->localMergeBase($repoRoot); + } + + return $this->officialUpstream->resolve($repoRoot, $repoName) + ?? $this->localMergeBase($repoRoot); + } + + /** @throws GitException */ + private function localMergeBase(string $repoRoot): string + { + $baseReference = $this->localBaseReference($repoRoot); + + if (null === $mergeBase = $this->git->findMergeBase($repoRoot, 'HEAD', $baseReference)) { + throw GitException::mergeBaseNotFound($baseReference); + } + + return $mergeBase; + } + + /** @throws GitException */ + private function localBaseReference(string $repoRoot): string + { + if ($this->git->currentBranchName($repoRoot) === self::DEFAULT_BRANCH) { + return $this->git->upstreamReferenceFromLocalConfiguration($repoRoot, self::DEFAULT_BRANCH) ?? 'HEAD'; + } + + if ($this->git->resolveCommitHash($repoRoot, self::DEFAULT_BRANCH_REFERENCE) !== null) { + return self::DEFAULT_BRANCH_REFERENCE; + } + + throw GitException::localMasterNotFound(); + } + + /** @throws GitException */ + private function repositoryName(string $repoRoot): ?string + { + $repoNames = []; + + foreach ($this->git->remoteUrlsFromLocalConfiguration($repoRoot) as $url) { + $repoName = $this->repositoryNameFrom($url); + + if ($repoName !== null) { + $repoNames[$repoName] = true; + } + } + + if (count($repoNames) === 1) { + return array_key_first($repoNames); + } + + return $repoNames === [] + ? $this->repositoryNameFrom($repoRoot) + : null; + } + + private function repositoryNameFrom(string $path): ?string + { + $path = rtrim(str_replace(['\\', ':'], '/', $path), '/'); + $name = preg_replace('/\\.git$/i', '', basename($path)); + + $isPhpDocsRepo = is_string($name) + && preg_match(self::PHP_DOCS_REPO_PATTERN, $name) === 1; + + return $isPhpDocsRepo ? strtolower($name) : null; + } +} diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 9dbd2fe..0c62f3a 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -12,12 +12,15 @@ public function parse(string $diff): Diff { /** @var array> $changedLinesByFile */ $changedLinesByFile = []; + /** @var array> $deletionAnchorsByFile */ + $deletionAnchorsByFile = []; $currentFile = null; $deleted = false; $newLineNumber = 0; $oldLinesRemaining = 0; $newLinesRemaining = 0; $inHunk = false; + $previousLineWasDeletion = false; foreach (explode("\n", $diff) as $line) { if (str_starts_with($line, 'diff --git ')) { @@ -25,6 +28,7 @@ public function parse(string $diff): Diff $deleted = false; $newLineNumber = 0; $inHunk = false; + $previousLineWasDeletion = false; continue; } @@ -43,6 +47,7 @@ public function parse(string $diff): Diff $inHunk = false; if ($currentFile !== null && !isset($changedLinesByFile[$currentFile])) { $changedLinesByFile[$currentFile] = []; + $deletionAnchorsByFile[$currentFile] = []; } continue; } @@ -58,6 +63,7 @@ public function parse(string $diff): Diff $newLineNumber = (int) $m[2]; $newLinesRemaining = isset($m[3]) ? (int) $m[3] : 1; $inHunk = true; + $previousLineWasDeletion = false; } continue; } @@ -70,24 +76,32 @@ public function parse(string $diff): Diff $changedLinesByFile[$currentFile][] = $newLineNumber; $newLineNumber++; $newLinesRemaining--; + $previousLineWasDeletion = false; } elseif (str_starts_with($line, '-')) { + if (!$previousLineWasDeletion) { + $deletionAnchorsByFile[$currentFile][] = max(1, $newLineNumber); + } + $oldLinesRemaining--; + $previousLineWasDeletion = true; } elseif (str_starts_with($line, ' ')) { // Context line — present in both old and new file. $newLineNumber++; $oldLinesRemaining--; $newLinesRemaining--; + $previousLineWasDeletion = false; } if ($oldLinesRemaining === 0 && $newLinesRemaining === 0) { $inHunk = false; + $previousLineWasDeletion = false; } } $fileChanges = []; foreach ($changedLinesByFile as $filePath => $lineNumbers) { - $fileChanges[] = new FileChange($filePath, $lineNumbers); + $fileChanges[] = new FileChange($filePath, $lineNumbers, $deletionAnchorsByFile[$filePath]); } return new Diff($fileChanges); diff --git a/src/Diff/DiffProviderInterface.php b/src/Diff/DiffProviderInterface.php new file mode 100644 index 0000000..418a7b4 --- /dev/null +++ b/src/Diff/DiffProviderInterface.php @@ -0,0 +1,10 @@ + $addedLineNumbers */ + /** + * @param list $addedLineNumbers + * @param list $deletionAnchors + */ public function __construct( public string $filePath, public array $addedLineNumbers, + public array $deletionAnchors = [], ) { } } diff --git a/src/Diff/GitDiffProvider.php b/src/Diff/GitDiffProvider.php new file mode 100644 index 0000000..d827b00 --- /dev/null +++ b/src/Diff/GitDiffProvider.php @@ -0,0 +1,43 @@ +baseResolver = new DiffBaseResolver( + $gitClient, + new UpstreamResolver($gitClient, $cacheDirectory), + ); + + $this->git = $gitClient; + } + + /** @throws GitException */ + public function for(string $workingDirectory): string + { + $mergeBase = $this->baseResolver->resolve( + $repoRoot = $this->git->repoRoot($workingDirectory) + ); + + return $this->git->diffFromMergeBase($repoRoot, $mergeBase); + } +} diff --git a/src/Diff/UpstreamResolver.php b/src/Diff/UpstreamResolver.php new file mode 100644 index 0000000..4fc45ae --- /dev/null +++ b/src/Diff/UpstreamResolver.php @@ -0,0 +1,131 @@ +cacheDirectory = rtrim($cacheDirectory, '/\\'); + } + + /** Serialises cache updates across parallel runs. */ + public function resolve(string $repoRoot, string $repoName): ?string + { + if (!$this->prepareCacheDirectory()) { + return null; + } + + $lock = @fopen($this->cacheLockPath($repoName), 'c'); + + if ($lock === false) { + return null; + } + + try { + if (!flock($lock, LOCK_EX)) { + return null; + } + + return $this->refreshAndResolve($repoRoot, $repoName); + } catch (GitException) { + return null; + } finally { + flock($lock, LOCK_UN); + fclose($lock); + } + } + + /** @throws GitException */ + private function refreshAndResolve(string $repoRoot, string $repoName): ?string + { + $cacheRepository = $this->cacheRepositoryPath($repoName); + + if (!$this->prepareCacheRepository($cacheRepository)) { + return null; + } + + $this->git->fetchToCacheRepo( + $cacheRepository, + sprintf(self::OFFICIAL_REPOSITORY_URL, $repoName), + self::OFFICIAL_BRANCH, + self::CACHED_UPSTREAM_REFERENCE, + ); + + $upstreamCommit = $this->git->resolveCommitHash( + $cacheRepository, + self::CACHED_UPSTREAM_REFERENCE, + ); + + if ($upstreamCommit === null) { + return null; + } + + return $this->git->findMergeBase( + $repoRoot, + 'HEAD', + $upstreamCommit, + sprintf(self::CACHE_OBJECTS_PATH, $cacheRepository), + ); + } + + private function prepareCacheDirectory(): bool + { + return is_dir($this->cacheDirectory) + || @mkdir($this->cacheDirectory, 0777, recursive: true) + || is_dir($this->cacheDirectory); + } + + /** @throws GitException */ + private function prepareCacheRepository(string $cacheRepository): bool + { + if (!is_dir($cacheRepository)) { + return $this->git->initialiseBareRepoForCache($cacheRepository); + } + + if ($this->git->isBareRepo($cacheRepository)) { + return true; + } + + // Keep unexpected directories instead of deleting user data. + $invalidRepository = sprintf(self::INVALID_CACHE_REPOSITORY_PATH, $cacheRepository, date('YmdHis')); + + return @rename($cacheRepository, $invalidRepository) + && $this->git->initialiseBareRepoForCache($cacheRepository); + } + + private function cacheRepositoryPath(string $repoName): string + { + return sprintf(self::CACHE_REPOSITORY_PATH, $this->cacheDirectory, $repoName); + } + + private function cacheLockPath(string $repoName): string + { + return sprintf(self::CACHE_LOCK_PATH, $this->cacheDirectory, $repoName); + } +} diff --git a/src/Git/GitClient.php b/src/Git/GitClient.php new file mode 100644 index 0000000..7c7abd6 --- /dev/null +++ b/src/Git/GitClient.php @@ -0,0 +1,198 @@ +runAndRequireSuccess( + ['git', 'rev-parse', '--show-toplevel'], + $workingDirectory, + 'Could not find Git repository.', + )); + } + + /** + * @return list + * @throws GitException + */ + public function remoteUrlsFromLocalConfiguration(string $repoRoot): array + { + $result = $this->execute( + ['git', 'config', '--get-regexp', '^remote\\..*\\.url$'], + $repoRoot, + ); + + if ($result->exitCode !== 0) { + return []; + } + + $urls = []; + + foreach (preg_split('/\\R/', trim($result->stdout)) ?: [] as $line) { + $parts = preg_split('/\\s+/', $line, 2); + + if (isset($parts[1])) { + $urls[] = $parts[1]; + } + } + + return $urls; + } + + /** @throws GitException */ + public function currentBranchName(string $repoRoot): ?string + { + $result = $this->execute( + ['git', 'symbolic-ref', '--quiet', '--short', 'HEAD'], + $repoRoot, + ); + + // null only when HEAD is detached + return $result->exitCode === 0 ? trim($result->stdout) : null; + } + + /** @throws GitException */ + public function upstreamReferenceFromLocalConfiguration(string $repoRoot, string $branch): ?string + { + $result = $this->execute( + ['git', 'rev-parse', '--abbrev-ref', '--symbolic-full-name', $branch . '@{upstream}'], + $repoRoot, + ); + + return $result->exitCode === 0 ? trim($result->stdout) : null; + } + + /** @throws GitException */ + public function resolveCommitHash(string $repoRoot, string $reference): ?string + { + $result = $this->execute( + ['git', 'rev-parse', '--verify', '--quiet', $reference . '^{commit}'], + $repoRoot, + ); + + return $result->exitCode === 0 ? trim($result->stdout) : null; + } + + /** @throws GitException */ + public function findMergeBase( + string $repoRoot, + string $firstReference, + string $secondReference, + ?string $alternateObjectDirectory = null, + ): ?string { + $environment = $alternateObjectDirectory !== null + ? ['GIT_ALTERNATE_OBJECT_DIRECTORIES' => $alternateObjectDirectory] + : []; + + // finds merge base using optional external object directory + $result = $this->execute( + ['git', 'merge-base', $firstReference, $secondReference], + $repoRoot, + $environment, + ); + + return $result->exitCode === 0 ? trim($result->stdout) : null; + } + + /** @throws GitException */ + public function diffFromMergeBase(string $repoRoot, string $mergeBase): string + { + return $this->runAndRequireSuccess( + ['git', 'diff', '--no-ext-diff', '--no-color', $mergeBase, '--'], + $repoRoot, + 'Could not read diff.', + ); + } + + /** @throws GitException */ + public function isBareRepo(string $repoPath): bool + { + $result = $this->execute( + ['git', '-C', $repoPath, 'rev-parse', '--is-bare-repository'], + dirname($repoPath), + ); + + return $result->exitCode === 0 && trim($result->stdout) === 'true'; + } + + /** @throws GitException */ + public function initialiseBareRepoForCache(string $repoPath): bool + { + return $this->execute( + ['git', 'init', '--bare', '--quiet', $repoPath], + dirname($repoPath), + )->exitCode === 0; + } + + /** @throws GitException */ + public function fetchToCacheRepo(string $repoPath, string $url, string $branch, string $reference): ProcessResult + { + return $this->execute( + [ + 'git', + '-c', + 'credential.interactive=false', + '-c', + 'http.lowSpeedLimit=1', + '-c', + 'http.lowSpeedTime=10', + '-C', + $repoPath, + 'fetch', + '--quiet', + '--no-tags', + '--filter=tree:0', + $url, + sprintf(self::FORCED_BRANCH_REF_SPEC, $branch, $reference), + ], + dirname($repoPath), + ['GIT_TERMINAL_PROMPT' => '0'], + ); + } + + /** + * @param list $command + * @throws GitException + */ + private function runAndRequireSuccess(array $command, string $workingDirectory, string $error): string + { + $result = $this->execute($command, $workingDirectory); + + if ($result->exitCode === 0) { + return $result->stdout; + } + + $detail = trim($result->stderr); + + throw GitException::commandFailed($error, $detail); + } + + /** + * @param list $command + * @param array $environment + * @throws GitException + */ + private function execute(array $command, string $workingDirectory, array $environment = []): ProcessResult + { + try { + return $this->processRunner->run($command, $workingDirectory, $environment); + } catch (ProcessException $exception) { + throw GitException::processFailed($exception); + } + } +} diff --git a/src/Git/GitException.php b/src/Git/GitException.php new file mode 100644 index 0000000..7c9711d --- /dev/null +++ b/src/Git/GitException.php @@ -0,0 +1,30 @@ +getMessage(), 0, $exception); + } + + public static function commandFailed(string $message, string $detail): self + { + return new self($detail !== '' ? "$message $detail" : $message); + } + + public static function localMasterNotFound(): self + { + return new self('Could not find local master branch for the contribution diff.'); + } + + public static function mergeBaseNotFound(string $reference): self + { + return new self(sprintf('Unclear where HEAD branched from %s.', $reference)); + } +} diff --git a/src/Path/DiffPathLoader.php b/src/Path/DiffPathLoader.php new file mode 100644 index 0000000..22706e0 --- /dev/null +++ b/src/Path/DiffPathLoader.php @@ -0,0 +1,101 @@ + $projectRoots */ + public function __construct( + private Diff $diff, + private string $workingDirectory, + private string $basePath, + private array $projectRoots, + private PathMatcher $matcher, + ) { + } + + public function load(): Diff + { + $changes = []; + + foreach ($this->diff->fileChanges as $fileChange) { + foreach ($this->candidates($fileChange->filePath) as $candidate) { + if ( + is_file($candidate) + && str_ends_with(strtolower($candidate), '.xml') + && $this->matcher->isIncluded($candidate) + ) { + $changes[$candidate] = new FileChange( + $candidate, + $fileChange->addedLineNumbers, + $fileChange->deletionAnchors, + ); + break; + } + } + } + + ksort($changes); + + return new Diff(array_values($changes)); + } + + /** @return list */ + private function candidates(string $path): array + { + $path = str_replace('\\', '/', $path); + + if ($this->isAbsolute($path)) { + return [$path]; + } + + $candidates = [ + $this->workingDirectory . '/' . $path, + $this->basePath . '/' . $path, + ]; + + foreach ($this->projectRoots as $root => $directory) { + $candidates[] = $root . '/' . $path; + + $prefix = trim(str_replace('\\', '/', $directory), '/') . '/'; + if ($prefix !== '/' && str_starts_with($path, $prefix)) { + $candidates[] = $root . '/' . substr($path, strlen($prefix)); + } + } + + return array_map($this->normalize(...), $candidates) + |> array_unique(...) + |> array_values(...); + } + + private function isAbsolute(string $path): bool + { + return str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:/#', $path) === 1; + } + + private function normalize(string $path): string + { + $prefix = str_starts_with($path, '/') ? '/' : ''; + $segments = []; + + foreach (explode('/', str_replace('\\', '/', $path)) as $segment) { + if ($segment === '' || $segment === '.') { + continue; + } + + if ($segment === '..' && $segments !== [] && end($segments) !== '..') { + array_pop($segments); + continue; + } + + $segments[] = $segment; + } + + return $prefix . implode('/', $segments); + } +} diff --git a/src/Path/EntityResolver.php b/src/Path/EntityResolver.php index 64d2200..8dc3f29 100644 --- a/src/Path/EntityResolver.php +++ b/src/Path/EntityResolver.php @@ -8,6 +8,12 @@ final class EntityResolver { private string $extension; + /** @var array|null */ + private ?array $resolvedEntities = null; + + /** @var array|null */ + private ?array $resolvedPaths = null; + /** * @param array $projectRoots * @param list $entityPaths @@ -26,15 +32,41 @@ public function __construct( */ public function resolve(): array { + $this->resolveAll(); + + return $this->resolvedEntities ?? []; + } + + /** + * @return array + * @throws \UnexpectedValueException if the directory cannot be read. + */ + public function paths(): array + { + $this->resolveAll(); + + return $this->resolvedPaths ?? []; + } + + /** @throws \UnexpectedValueException if the directory cannot be read. */ + private function resolveAll(): void + { + if ($this->resolvedEntities !== null && $this->resolvedPaths !== null) { + return; + } + $entities = []; + $paths = []; foreach ($this->entityPaths as $path) { foreach ($this->getEntityFiles($path) as $file) { - $entities += $this->resolveFile($file); + $visited = []; + $entities += $this->resolveFile($file, $visited, $paths); } } - return $entities; + $this->resolvedEntities = $entities; + $this->resolvedPaths = $paths; } /** @@ -86,11 +118,13 @@ private function scanDirectory(string $directory): array /** * @param array $visited + * @param array $paths * @return array */ private function resolveFile( string $filePath, - array &$visited = [], + array &$visited, + array &$paths, ?string $originEntity = null ): array { if (isset($visited[$filePath]) || !is_readable($filePath)) { @@ -105,7 +139,7 @@ private function resolveFile( return []; // @codeCoverageIgnore } - $entities = $this->extractEntities($content, $filePath, $visited); + $entities = $this->extractEntities($content, $filePath, $visited, $paths); if ($originEntity !== null) { $entities[$originEntity] = $this->normalize($content); @@ -116,26 +150,30 @@ private function resolveFile( /** * @param array $visited + * @param array $paths * @return array */ private function extractEntities( string $content, string $filePath, - array &$visited + array &$visited, + array &$paths, ): array { return - $this->extractDtdEntities($content, $filePath, $visited) + $this->extractDtdEntities($content, $filePath, $visited, $paths) + $this->extractXmlEntities($content); } /** * @param array $visited + * @param array $paths * @return array */ private function extractDtdEntities( string $content, string $filePath, - array &$visited + array &$visited, + array &$paths, ): array { $result = []; @@ -161,7 +199,12 @@ private function extractDtdEntities( if ($type === 'SYSTEM') { $resolvedPath = $this->resolvePath($filePath, $value); - $result += $this->resolveFile($resolvedPath, $visited, $name); + + if (is_readable($resolvedPath)) { + $paths[$name] ??= $resolvedPath; + } + + $result += $this->resolveFile($resolvedPath, $visited, $paths, $name); continue; } diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php new file mode 100644 index 0000000..8e8907c --- /dev/null +++ b/src/Process/NativeProcessRunner.php @@ -0,0 +1,56 @@ +environmentWithOverrides($environment), + ); + + if (!is_resource($process)) { + throw ProcessException::couldNotStart(); + } + + fclose($pipes[0]); + $stdout = stream_get_contents($pipes[1]); + $stderr = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + + return new ProcessResult( + exitCode: proc_close($process), + stdout: $stdout !== false ? $stdout : '', + stderr: $stderr !== false ? $stderr : '', + ); + } + + /** + * @param array $overrides + * @return array|null + */ + private function environmentWithOverrides(array $overrides): ?array + { + if ($overrides === []) { + return null; + } + + // proc_open replaces inherited variables with overrides. + // Keep them, then apply only the requested overrides. + $inherited = getenv(); + + return array_replace(is_array($inherited) ? $inherited : [], $overrides); + } +} diff --git a/src/Process/ProcessException.php b/src/Process/ProcessException.php new file mode 100644 index 0000000..5dcff63 --- /dev/null +++ b/src/Process/ProcessException.php @@ -0,0 +1,13 @@ + $command + * @param array $environment + * @throws ProcessException if the process cannot be started. + */ + public function run(array $command, string $workingDirectory, array $environment = []): ProcessResult; +} diff --git a/src/Progress/ConsoleProgress.php b/src/Progress/ConsoleProgress.php index 66475e8..899235a 100644 --- a/src/Progress/ConsoleProgress.php +++ b/src/Progress/ConsoleProgress.php @@ -15,6 +15,8 @@ final class ConsoleProgress implements ProgressInterface private int $totalFiles = 0; + private int $processedFiles = 0; + /** * @param resource $stream */ @@ -27,6 +29,7 @@ public function __construct($stream, bool $useColors = true) public function start(int $totalFiles): void { $this->totalFiles = $totalFiles; + $this->processedFiles = 0; if ($totalFiles === 0) { $this->write($this->dim('No files to scan.') . PHP_EOL); @@ -38,13 +41,13 @@ public function start(int $totalFiles): void $this->drawBar(0, ''); } - public function advance(int $current, string $filePath, int $violations): void + public function advance(string $filePath, int $violations): void { if ($this->totalFiles === 0) { return; } - $this->drawBar($current, $filePath, $violations); + $this->drawBar(++$this->processedFiles, $filePath, $violations); } public function finish(): void diff --git a/src/Progress/NullProgress.php b/src/Progress/NullProgress.php index 1d97a3f..f01eb89 100644 --- a/src/Progress/NullProgress.php +++ b/src/Progress/NullProgress.php @@ -11,7 +11,7 @@ public function start(int $totalFiles): void // Intentionally left empty } - public function advance(int $current, string $filePath, int $violations): void + public function advance(string $filePath, int $violations): void { // Intentionally left empty } diff --git a/src/Progress/ProgressInterface.php b/src/Progress/ProgressInterface.php index 3712944..0a9f4eb 100644 --- a/src/Progress/ProgressInterface.php +++ b/src/Progress/ProgressInterface.php @@ -8,7 +8,7 @@ interface ProgressInterface { public function start(int $totalFiles): void; - public function advance(int $current, string $filePath, int $violations): void; + public function advance(string $filePath, int $violations): void; public function finish(): void; } diff --git a/src/Runner/EntityPreprocessor.php b/src/Runner/EntityPreprocessor.php index f7d80b8..9f597e2 100644 --- a/src/Runner/EntityPreprocessor.php +++ b/src/Runner/EntityPreprocessor.php @@ -4,17 +4,17 @@ namespace DocbookCS\Runner; -final class EntityPreprocessor +final readonly class EntityPreprocessor { private const array PREDEFINED = ['amp', 'lt', 'gt', 'quot', 'apos']; - private const string ENTITY_PATTERN = '&([a-zA-Z_][\w.\-]*);'; + private const string EXPANSION_PATTERN = '/||<\?.*?\?>|&([a-zA-Z_][\w.-]*);/s'; private const string XML_DECLARATION_PATTERN = '/<\?xml[^?]*\?>/i'; /** * @param array $entities */ public function __construct( - private array $entities, + private array $entities = [], ) { } @@ -40,7 +40,7 @@ private function expandEntities(string $content, bool $markXmlExpansions = false $changed = false; $content = preg_replace_callback( - '/||<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/', + self::EXPANSION_PATTERN, function (array $matches) use (&$changed, $markXmlExpansions): string { if ( str_starts_with($matches[0], '