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..4b72e67 100644 --- a/bin/docbook-cs +++ b/bin/docbook-cs @@ -25,4 +25,4 @@ use DocbookCS\Application; exit(2); })(); -new Application($argv ?? [])->run(); +exit(Application::fromGlobals($argv ?? [])->run()); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index eaa454f..540a1a7 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -21,6 +21,9 @@ tests/Integration + + tests/Feature + diff --git a/src/Application.php b/src/Application.php index 9827b56..bb36f76 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 @@ -32,21 +32,50 @@ final class Application /** @var resource */ private $stderr; - /** @var resource */ - private $stdin; + private ?string $stdin; + + /** + * @param list $argv + * @throws \RuntimeException if redirected stdin cannot be read. + * @api + */ + public static function fromGlobals(array $argv): self + { + $stdin = null; + $stat = fstat(STDIN); + + if ($stat === false) { + return new self($argv, stdin: $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, stdin: $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) - { + public function __construct( + array $argv, + mixed $stdout = null, + mixed $stderr = null, + ?string $stdin = null, + ) { $this->argv = $argv; $this->stdout = $stdout ?? STDOUT; $this->stderr = $stderr ?? STDERR; - $this->stdin = $stdin ?? STDIN; + $this->stdin = $stdin; } /** @@ -54,7 +83,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 +111,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->stdin); + } 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 +141,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 +150,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 +165,7 @@ private function parseArgv(): array 'colors' => $this->detectColorSupport(), 'quiet' => false, 'paths' => [], - 'diff' => null, + 'wide' => false, 'perf' => false, ]; @@ -227,23 +230,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 +245,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 +359,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/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..3e7c146 --- /dev/null +++ b/src/Diff/GitDiffProvider.php @@ -0,0 +1,92 @@ +runOrThrow( + ['git', 'rev-parse', '--show-toplevel'], + $workingDirectory, + 'Could not find Git repository.', + )); + + $baseReference = $this->resolveBaseReference($repositoryRoot); + $mergeBase = $this->runOrThrow( + ['git', 'merge-base', 'HEAD', $baseReference], + $repositoryRoot, + sprintf('Unclear where HEAD branched from %s.', $baseReference), + ); + + return $this->runOrThrow( + ['git', 'diff', '--no-ext-diff', '--no-color', trim($mergeBase), '--'], + $repositoryRoot, + 'Could not read diff.', + ); + } + + /** @throws \RuntimeException if no default branch reference exists. */ + private function resolveBaseReference(string $repositoryRoot): string + { + $candidates = []; + + foreach (['upstream', 'origin'] as $remote) { + $result = $this->processRunner->run( + ['git', 'symbolic-ref', '--quiet', sprintf('refs/remotes/%s/HEAD', $remote)], + $repositoryRoot, + ); + + if ($result->exitCode === 0) { + $candidates[] = trim($result->stdout); + } + + $candidates[] = sprintf('refs/remotes/%s/main', $remote); + $candidates[] = sprintf('refs/remotes/%s/master', $remote); + } + + $candidates[] = 'refs/heads/main'; + $candidates[] = 'refs/heads/master'; + + foreach (array_unique($candidates) as $candidate) { + $result = $this->processRunner->run( + ['git', 'rev-parse', '--verify', '--quiet', $candidate . '^{commit}'], + $repositoryRoot, + ); + + if ($result->exitCode === 0) { + return $candidate; + } + } + + throw new \RuntimeException('Could not determine the upstream default branch for the contribution diff.'); + } + + /** + * @param list $command + * @throws \RuntimeException if the command fails. + */ + private function runOrThrow(array $command, string $workingDirectory, string $error): string + { + $result = $this->processRunner->run($command, $workingDirectory); + + if ($result->exitCode === 0) { + return $result->stdout; + } + + $detail = trim($result->stderr); + + throw new \RuntimeException($detail !== '' ? "$error $detail" : $error); + } +} 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..dce353c --- /dev/null +++ b/src/Process/NativeProcessRunner.php @@ -0,0 +1,38 @@ + $command + * @throws \RuntimeException if the process cannot be started. + */ + public function run(array $command, string $workingDirectory): ProcessResult; +} diff --git a/src/Runner/RunPlan.php b/src/Runner/RunPlan.php new file mode 100644 index 0000000..71917e1 --- /dev/null +++ b/src/Runner/RunPlan.php @@ -0,0 +1,23 @@ + $sniffs + * @param array $targets + * @param array $entities + */ + public function __construct( + public array $sniffs, + public array $targets, + public array $entities, + ) { + } +} diff --git a/src/Runner/RunPlanner.php b/src/Runner/RunPlanner.php new file mode 100644 index 0000000..b17c3cf --- /dev/null +++ b/src/Runner/RunPlanner.php @@ -0,0 +1,79 @@ +diffProvider = $diffProvider ?? new GitDiffProvider(); + $this->entityResolver = new EntityResolver( + $config->getProjectRoots(), + $config->getEntityPaths(), + ); + } + + /** + * @param list $paths + * @throws \InvalidArgumentException if paths and a piped diff are both provided. + * @throws \RuntimeException if the contribution diff cannot be determined. + * @throws \UnexpectedValueException if an entity or selected directory cannot be read. + */ + public function plan(array $paths, ?string $pipedDiff): RunPlan + { + if ($paths === []) { + return $this->planDiff(new DiffParser()->parse($pipedDiff ?? $this->diffProvider->for(getcwd() ?: '.'))); + } + + if ($pipedDiff !== null) { + throw new \InvalidArgumentException('Paths cannot be combined with diff input.'); + } + + return $this->planPaths($paths); + } + + /** + * @param list $paths + * @throws \UnexpectedValueException if an entity or selected directory cannot be read. + */ + public function planPaths(array $paths): RunPlan + { + return new RunPlan( + sniffs: $this->config->getSniffs(), + targets: $this->scopeResolver()->resolvePaths($paths), + entities: $this->entityResolver->resolve(), + ); + } + + /** @throws \UnexpectedValueException if an entity directory cannot be read. */ + public function planDiff(Diff $diff): RunPlan + { + return new RunPlan( + sniffs: $this->config->getSniffs(), + targets: $this->scopeResolver()->resolveDiff($diff), + entities: $this->entityResolver->resolve(), + ); + } + + /** @throws \UnexpectedValueException if an entity directory cannot be read. */ + private function scopeResolver(): RunScopeResolver + { + return new RunScopeResolver($this->config, $this->entityResolver->paths(), $this->wide); + } +} diff --git a/src/Runner/RunScopeResolver.php b/src/Runner/RunScopeResolver.php new file mode 100644 index 0000000..931da12 --- /dev/null +++ b/src/Runner/RunScopeResolver.php @@ -0,0 +1,212 @@ + $entityPaths */ + public function __construct( + private ConfigData $config, + private array $entityPaths, + private bool $wide = false, + ) { + $this->pathMatcher = new PathMatcher( + $config->getBasePath(), + $config->getExcludePatterns(), + ); + } + + /** + * @param list $paths + * @return array + * @throws \UnexpectedValueException if a selected directory cannot be read. + */ + public function resolvePaths(array $paths): array + { + $targets = []; + + foreach (new PathLoader($this->absolutePaths($paths), $this->pathMatcher)->loadPaths() as $file) { + $targets[$file] = null; + } + + return $this->finalize($targets); + } + + /** @return array */ + public function resolveDiff(Diff $diff): array + { + $resolvedDiff = new DiffPathLoader( + $diff, + getcwd() ?: '.', + $this->config->getBasePath(), + $this->config->getProjectRoots(), + $this->pathMatcher, + )->load(); + + $targets = []; + + foreach ($resolvedDiff->fileChanges as $fileChange) { + $targets[$fileChange->filePath] = $fileChange; + } + + return $this->finalize($targets); + } + + /** + * @param array $targets + * @return array + */ + private function finalize(array $targets): array + { + if ($this->wide) { + $targets = array_fill_keys(array_keys($targets), null); + $this->expandReferencedTargets($targets); + } + + ksort($targets); + + return $targets; + } + + /** + * @param list $paths + * @return list + */ + private function absolutePaths(array $paths): array + { + $workingDirectory = getcwd() ?: '.'; + $absolutePaths = []; + + foreach ($paths as $path) { + $absolutePath = str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $path) + ? $path + : $workingDirectory . '/' . $path; + + $absolutePaths[] = $this->normalizePath($absolutePath); + } + + return $absolutePaths; + } + + /** @param array $targets */ + private function expandReferencedTargets(array &$targets): void + { + $pending = array_keys($targets); + $visitedFiles = []; + $visitedEntityPaths = []; + + for ($i = 0; isset($pending[$i]); $i++) { + $file = $pending[$i]; + + if (isset($visitedFiles[$file])) { + continue; + } + + $visitedFiles[$file] = true; + $content = @file_get_contents($file); + + if ($content === false) { + continue; + } + + foreach ($this->targetFilesFromContent($content, $visitedEntityPaths) as $targetFile) { + if (array_key_exists($targetFile, $targets)) { + continue; + } + + $targets[$targetFile] = null; + $pending[] = $targetFile; + } + } + } + + /** + * @param array $visitedEntityPaths + * @return list + */ + private function targetFilesFromContent(string $content, array &$visitedEntityPaths): array + { + if (!preg_match_all(self::ENTITY_PATTERN, $content, $matches)) { + return []; + } + + $files = []; + + foreach ($matches[1] as $name) { + if (!isset($this->entityPaths[$name])) { + continue; + } + + foreach ($this->expandEntityPath($this->entityPaths[$name], $visitedEntityPaths) as $file) { + $files[$file] = true; + } + } + + return array_keys($files); + } + + /** + * @param array $visited + * @return list + */ + private function expandEntityPath(string $path, array &$visited): array + { + $path = $this->normalizePath($path); + + if (isset($visited[$path]) || !is_file($path)) { + return []; + } + + $visited[$path] = true; + + if (str_ends_with($path, '.xml')) { + return $this->pathMatcher->isIncluded($path) ? [$path] : []; + } + + $content = @file_get_contents($path); + + return $content !== false + ? $this->targetFilesFromContent($content, $visited) + : []; + } + + private function normalizePath(string $path): string + { + $path = str_replace('\\', '/', $path); + $prefix = ''; + + if (preg_match('#^([a-zA-Z]:/|/)#', $path, $matches)) { + $prefix = $matches[1]; + $path = substr($path, strlen($prefix)); + } + + $segments = []; + foreach (explode('/', $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/Runner/SniffRunner.php b/src/Runner/SniffRunner.php index 2884155..923919b 100644 --- a/src/Runner/SniffRunner.php +++ b/src/Runner/SniffRunner.php @@ -4,12 +4,7 @@ namespace DocbookCS\Runner; -use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; -use DocbookCS\Diff\Diff; -use DocbookCS\Path\EntityResolver; -use DocbookCS\Path\PathLoader; -use DocbookCS\Path\PathMatcher; use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; use DocbookCS\Report\Report; @@ -25,45 +20,29 @@ public function __construct(?ProgressInterface $progress = null) } /** - * @param list|null $overridePaths * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. - * @throws \UnexpectedValueException if no files are found to scan. */ - public function run(ConfigData $config, ?array $overridePaths = null, ?Diff $diff = null): Report + public function run(RunPlan $plan): Report { $startTime = microtime(true); - $sniffs = $this->instantiateSniffs($config->getSniffs()); - - $matcher = new PathMatcher($config->getBasePath(), $config->getExcludePatterns()); - - $includePaths = $overridePaths ?? $config->getIncludePaths(); - - $entityResolver = new EntityResolver($config->getProjectRoots(), $config->getEntityPaths()); - $entities = $entityResolver->resolve(); - - $pathLoader = new PathLoader($includePaths, $matcher); - $files = $pathLoader->loadPaths(); - - if ($diff !== null) { - $files = array_values(array_filter( - $files, - static fn(string $file): bool => $diff->changeFor($file) !== null, - )); - } + $sniffs = $this->instantiateSniffs($plan->sniffs); $report = new Report(); - $preprocessor = new EntityPreprocessor($entities); + $preprocessor = new EntityPreprocessor($plan->entities); $processor = new XmlFileProcessor($sniffs, $preprocessor, $report); - $total = count($files); + $total = count($plan->targets); $this->progress->start($total); - foreach ($files as $index => $file) { + $index = 0; + foreach ($plan->targets as $file => $fileChange) { $report->incrementFilesScanned(); - $changedLines = $diff?->changeFor($file)?->addedLineNumbers; + $changedLines = $fileChange !== null + ? array_values(array_unique([...$fileChange->addedLineNumbers, ...$fileChange->deletionAnchors])) + : null; $fileReport = $processor->processFile( $file, @@ -77,7 +56,7 @@ public function run(ConfigData $config, ?array $overridePaths = null, ?Diff $dif $report->addFileReport($fileReport); } - $this->progress->advance($index + 1, $file, $violationCount); + $this->progress->advance(++$index, $file, $violationCount); } $this->progress->finish(); diff --git a/tests/Feature/ApplicationInputTest.php b/tests/Feature/ApplicationInputTest.php new file mode 100644 index 0000000..a68ac9a --- /dev/null +++ b/tests/Feature/ApplicationInputTest.php @@ -0,0 +1,99 @@ +stdout = $stdout; + $this->stderr = $stderr; + } + + #[Test] + public function itRejectsTheRemovedDiffOption(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff'], + $this->stdout, + $this->stderr, + ); + + self::assertSame(2, $app->run()); + self::assertStringContainsString('Unknown option: --diff', $this->readStream($this->stderr)); + } + + #[Test] + public function itDetectsAPipedDiffWithoutAFlag(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG], + $this->stdout, + $this->stderr, + stdin: '', + ); + + self::assertSame(0, $app->run()); + self::assertSame('', $this->readStream($this->stderr)); + } + + #[Test] + public function itRejectsPathsCombinedWithAPipedDiff(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG, self::SCAN_FILE], + $this->stdout, + $this->stderr, + stdin: '', + ); + + self::assertSame(2, $app->run()); + self::assertStringContainsString('Paths cannot be combined with diff input', $this->readStream($this->stderr)); + } + + #[Test] + public function itIncludesTheWideOptionInHelp(): void + { + $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); + + $app->run(); + + $output = $this->readStream($this->stdout); + + self::assertStringContainsString('--wide', $output); + self::assertStringNotContainsString('--diff', $output); + } + + /** @param resource $stream */ + private function readStream(mixed $stream): string + { + rewind($stream); + + return stream_get_contents($stream) ?: ''; + } +} diff --git a/tests/Integration/Diff/GitDiffProviderTest.php b/tests/Integration/Diff/GitDiffProviderTest.php new file mode 100644 index 0000000..3cf7dc9 --- /dev/null +++ b/tests/Integration/Diff/GitDiffProviderTest.php @@ -0,0 +1,112 @@ +processRunner = new NativeProcessRunner(); + + $tmpDir = sys_get_temp_dir() . '/docbook-cs-git-diff-' . bin2hex(random_bytes(6)); + mkdir($tmpDir); + $this->repository = $tmpDir; + } + + protected function tearDown(): void + { + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($this->repository, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST, + ); + + foreach ($files as $file) { + if (!$file instanceof \SplFileInfo) { + throw new \UnexpectedValueException('Unexpected directory entry.'); + } + + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + + rmdir($this->repository); + } + + #[Test] + public function itDiffsTheWorkingTreeFromTheUpstreamBranchPoint(): void + { + $this->git('init', '--quiet', '--initial-branch=main'); + $this->configureAuthor(); + + file_put_contents($this->repository . '/base.xml', "base\n"); + $this->git('add', 'base.xml'); + $this->git('commit', '--quiet', '-m', 'Base'); + + $base = $this->git('rev-parse', 'HEAD'); + $this->git('update-ref', 'refs/remotes/upstream/main', $base); + $this->git('symbolic-ref', 'refs/remotes/upstream/HEAD', 'refs/remotes/upstream/main'); + $this->git('switch', '--quiet', '-c', 'contribution'); + $this->git('branch', '--delete', '--force', 'main'); + + file_put_contents($this->repository . '/committed.xml', "committed\n"); + $this->git('add', 'committed.xml'); + $this->git('commit', '--quiet', '-m', 'Contribution'); + file_put_contents($this->repository . '/base.xml', "working tree\n"); + + $diff = new GitDiffProvider($this->processRunner)->for($this->repository); + + self::assertStringContainsString('diff --git a/base.xml b/base.xml', $diff); + self::assertStringContainsString('+working tree', $diff); + self::assertStringContainsString('diff --git a/committed.xml b/committed.xml', $diff); + self::assertStringContainsString('+committed', $diff); + } + + #[Test] + public function itFailsClearlyWhenNoUpstreamDefaultBranchCanBeFound(): void + { + $this->git('init', '--quiet', '--initial-branch=contribution'); + $this->configureAuthor(); + + file_put_contents($this->repository . '/base.xml', "base\n"); + $this->git('add', 'base.xml'); + $this->git('commit', '--quiet', '-m', 'Contribution'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageIsOrContains('Could not determine the upstream default branch'); + + new GitDiffProvider($this->processRunner)->for($this->repository); + } + + private function configureAuthor(): void + { + $this->git('config', 'user.name', 'DocbookCS Tests'); + $this->git('config', 'user.email', 'docbook-cs@example.invalid'); + } + + private function git(string ...$arguments): string + { + $result = $this->processRunner->run(array_values(['git', ...$arguments]), $this->repository); + + self::assertSame(0, $result->exitCode, $result->stderr); + + return trim($result->stdout); + } +} diff --git a/tests/Integration/Path/DiffPathLoaderTest.php b/tests/Integration/Path/DiffPathLoaderTest.php new file mode 100644 index 0000000..47ba2e4 --- /dev/null +++ b/tests/Integration/Path/DiffPathLoaderTest.php @@ -0,0 +1,80 @@ +directory = sys_get_temp_dir() . '/docbook-cs-diff-path-' . bin2hex(random_bytes(6)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + @unlink($this->directory . '/chapter.xml'); + @unlink($this->directory . '/excluded.xml'); + @rmdir($this->directory); + } + + #[Test] + public function itLoadsChangedXmlFilesWithoutScanningConfiguredPaths(): void + { + $file = $this->directory . '/chapter.xml'; + file_put_contents($file, ''); + + $loader = new DiffPathLoader( + new Diff([new FileChange('docs/chapter.xml', [1])]), + workingDirectory: dirname($this->directory), + basePath: $this->directory, + projectRoots: [$this->directory => 'docs'], + matcher: new PathMatcher($this->directory, []), + ); + + $change = $loader->load()->changeFor($file); + + self::assertNotNull($change); + self::assertSame([1], $change->addedLineNumbers); + } + + #[Test] + public function itIgnoresMissingNonXmlAndExcludedFiles(): void + { + $excluded = $this->directory . '/excluded.xml'; + file_put_contents($excluded, ''); + + $loader = new DiffPathLoader( + new Diff([ + new FileChange('excluded.xml', [1]), + new FileChange('notes.txt', [1]), + new FileChange('missing.xml', [1]), + ]), + workingDirectory: $this->directory, + basePath: $this->directory, + projectRoots: [], + matcher: new PathMatcher($this->directory, ['excluded.xml']), + ); + + self::assertSame([], $loader->load()->fileChanges); + } +} diff --git a/tests/Integration/Process/NativeProcessRunnerTest.php b/tests/Integration/Process/NativeProcessRunnerTest.php new file mode 100644 index 0000000..9b930fb --- /dev/null +++ b/tests/Integration/Process/NativeProcessRunnerTest.php @@ -0,0 +1,33 @@ +run( + [PHP_BINARY, '-r', 'fwrite(STDOUT, "out"); fwrite(STDERR, "err"); exit(7);'], + getcwd() ?: '.', + ); + + self::assertSame(7, $result->exitCode); + self::assertSame('out', $result->stdout); + self::assertSame('err', $result->stderr); + } +} diff --git a/tests/Integration/Runner/RunScopeResolverTest.php b/tests/Integration/Runner/RunScopeResolverTest.php new file mode 100644 index 0000000..f4368bf --- /dev/null +++ b/tests/Integration/Runner/RunScopeResolverTest.php @@ -0,0 +1,146 @@ +directory = sys_get_temp_dir() . '/docbook-cs-scope-' . bin2hex(random_bytes(6)); + mkdir($this->directory); + + $this->sourceFile = $this->directory . '/source.xml'; + $this->targetFile = $this->directory . '/target.xml'; + $this->entityFile = $this->directory . '/bridge.ent'; + + file_put_contents($this->sourceFile, '&bridge;'); + file_put_contents($this->targetFile, ''); + file_put_contents($this->entityFile, '⌖'); + } + + protected function tearDown(): void + { + @unlink($this->sourceFile); + @unlink($this->targetFile); + @unlink($this->entityFile); + @rmdir($this->directory); + } + + #[Test] + public function narrowScopeKeepsOnlySelectedFilesAndDiffLines(): void + { + $targets = $this->resolver()->resolveDiff(new Diff([new FileChange('source.xml', [2, 3])])); + + self::assertSame([2, 3], $targets[$this->sourceFile]?->addedLineNumbers); + self::assertCount(1, $targets); + } + + #[Test] + public function wideScopeWidensSelectedFilesAndFollowsReferencedTargets(): void + { + $targets = $this->resolver(wide: true)->resolveDiff(new Diff([new FileChange('source.xml', [2, 3])])); + + self::assertNull($targets[$this->sourceFile]); + self::assertNull($targets[$this->targetFile]); + self::assertCount(2, $targets); + } + + #[Test] + public function expandedScopeHonorsTargetExclusions(): void + { + $resolver = new RunScopeResolver( + $this->config(['target.xml']), + [ + 'bridge' => $this->entityFile, + 'target' => $this->targetFile, + ], + wide: true, + ); + + self::assertSame([$this->sourceFile => null], $resolver->resolvePaths([$this->sourceFile])); + } + + #[Test] + public function pathScopeResolvesRelativePathsAgainstTheWorkingDirectory(): void + { + $path = 'tests/fixtures/sniff_runner/default/file_a.xml'; + $absolutePath = (getcwd() ?: '.') . '/' . $path; + + self::assertSame([$absolutePath => null], $this->resolver()->resolvePaths([$path])); + } + + #[Test] + public function widePathScopeDoesNotDuplicateLexicallyEquivalentTargets(): void + { + $resolver = new RunScopeResolver( + $this->config(), + [ + 'bridge' => $this->entityFile, + 'target' => $this->directory . '/./target.xml', + ], + wide: true, + ); + + self::assertSame( + [ + $this->sourceFile => null, + $this->targetFile => null, + ], + $resolver->resolvePaths([$this->directory . '/.']), + ); + } + + private function resolver(bool $wide = false): RunScopeResolver + { + return new RunScopeResolver( + $this->config(), + [ + 'bridge' => $this->entityFile, + 'target' => $this->targetFile, + ], + $wide, + ); + } + + /** @param list $excludePatterns */ + private function config(array $excludePatterns = []): ConfigData + { + return new ConfigData( + projectRoots: [], + sniffs: [], + includePaths: [$this->sourceFile], + excludePatterns: $excludePatterns, + entityPaths: [], + basePath: $this->directory, + ); + } +} diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 275288a..50f1f09 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -6,15 +6,19 @@ use DocbookCS\Application; use DocbookCS\Config\ConfigData; -use DocbookCS\Diff\Diff; -use DocbookCS\Diff\DiffParser; -use DocbookCS\Diff\FileChange; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; use DocbookCS\Config\SniffEntry; +use DocbookCS\Diff\Diff; +use DocbookCS\Diff\DiffParser; +use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; +use DocbookCS\Process\NativeProcessRunner; +use DocbookCS\Process\ProcessResult; use DocbookCS\Progress\NullProgress; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; @@ -22,6 +26,9 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\SniffRunner; use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Sniff\ExceptionNameSniff; @@ -47,11 +54,18 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), CoversClass(SniffRunner::class), CoversClass(XmlFileProcessor::class), UsesClass(Diff::class), + UsesClass(DiffPathLoader::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(NativeProcessRunner::class), + UsesClass(ProcessResult::class), + UsesClass(RunScopeResolver::class), ] final class ApplicationTest extends TestCase { @@ -249,7 +263,7 @@ public function itResolvesRelativeOverridePathsAgainstCwd(): void public function itCatchesRuntimeErrorFromRunner(): void { $app = new Application( - ['docbook-cs', '--config=' . self::INVALID_SNIFF_CONFIG], + ['docbook-cs', '--config=' . self::INVALID_SNIFF_CONFIG, self::SCAN_FILE], $this->stdout, $this->stderr, ); @@ -288,114 +302,6 @@ public function itPassesThroughAbsoluteOverridePaths(): void self::assertNotSame(2, $exitCode); } - #[Test] - public function itSupportsDiffFromFile(): void - { - $diffFile = tempnam(sys_get_temp_dir(), 'docbookcs_test_'); - self::assertIsString($diffFile); - - // Diff that references no XML files the config would normally scan. - file_put_contents($diffFile, <<<'DIFF' -diff --git a/nonexistent.xml b/nonexistent.xml ---- a/nonexistent.xml -+++ b/nonexistent.xml -@@ -1,1 +1,2 @@ - line1 -+line2 -DIFF); - - try { - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, "--diff={$diffFile}"], - $this->stdout, - $this->stderr, - ); - - $exitCode = $app->run(); - - // No matching files → no violations → exit 0. - self::assertSame(0, $exitCode); - self::assertSame('', $this->readStream($this->stderr)); - } finally { - unlink($diffFile); - } - } - - #[Test] - public function itSupportsDiffFromStdin(): void - { - $stdin = fopen('php://memory', 'rb+'); - self::assertIsResource($stdin); - - fwrite($stdin, <<<'DIFF' -diff --git a/nonexistent.xml b/nonexistent.xml ---- a/nonexistent.xml -+++ b/nonexistent.xml -@@ -1,1 +1,2 @@ - line1 -+line2 -DIFF); - rewind($stdin); - - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff'], - $this->stdout, - $this->stderr, - $stdin, - ); - - $exitCode = $app->run(); - - self::assertSame(0, $exitCode); - self::assertSame('', $this->readStream($this->stderr)); - } - - #[Test] - public function itSupportsDiffFromStdinWithExplicitDash(): void - { - $stdin = fopen('php://memory', 'rb+'); - self::assertIsResource($stdin); - - fwrite($stdin, ''); - rewind($stdin); - - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff=-'], - $this->stdout, - $this->stderr, - $stdin, - ); - - $exitCode = $app->run(); - - self::assertSame(0, $exitCode); - } - - #[Test] - public function itReturnsErrorWhenDiffFileCannotBeRead(): void - { - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff=/nonexistent/path.patch'], - $this->stdout, - $this->stderr, - ); - - $exitCode = $app->run(); - - self::assertSame(2, $exitCode); - self::assertStringContainsString('Error reading diff', $this->readStream($this->stderr)); - } - - #[Test] - public function itIncludesDiffOptionInHelp(): void - { - $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); - - $app->run(); - - self::assertStringContainsString('--diff', $this->readStream($this->stdout)); - } - #[Test] public function itSuppressesProgressWhenQuietFlagIsSet(): void { diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index ad3e5d2..1e49b92 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -176,6 +176,26 @@ public function itIgnoresRemovedLines(): void self::assertSame([], $result['file.xml']); } + #[Test] + public function itAnchorsRemovedLinesInTheResultingFile(): void + { + $diff = <<<'DIFF' +diff --git a/file.xml b/file.xml +--- a/file.xml ++++ b/file.xml +@@ -1,4 +1,3 @@ + line1 +-removed line + line2 + line3 +DIFF; + + $change = $this->parser->parse($diff)->changeFor('file.xml'); + self::assertNotNull($change); + + self::assertSame([2], $change->deletionAnchors); + } + #[Test] public function itIgnoresTheMissingFinalNewlineMarker(): void { @@ -195,6 +215,26 @@ public function itIgnoresTheMissingFinalNewlineMarker(): void self::assertSame([1, 2], $result['file.xml']); } + #[Test] + public function itAnchorsReplacedLinesWhenTheMissingFinalNewlineMarkerIsPresent(): void + { + $diff = <<<'DIFF' +diff --git a/file.xml b/file.xml +--- a/file.xml ++++ b/file.xml +@@ -1 +1,2 @@ +-old +\ No newline at end of file ++new ++second +DIFF; + + $change = $this->parser->parse($diff)->changeFor('file.xml'); + self::assertNotNull($change); + + self::assertSame([1], $change->deletionAnchors); + } + #[Test] public function itTracksLineNumbersAcrossMultipleHunks(): void { diff --git a/tests/Unit/Path/EntityResolverPathsTest.php b/tests/Unit/Path/EntityResolverPathsTest.php new file mode 100644 index 0000000..d09d1ff --- /dev/null +++ b/tests/Unit/Path/EntityResolverPathsTest.php @@ -0,0 +1,38 @@ +paths(); + $entities = $resolver->resolve(); + + self::assertSame($fixtureRoot . '/included.ent', $paths['inc']); + self::assertSame('child-value', $entities['child']); + } + + #[Test] + public function itDoesNotExposeUnreadableEntityTargets(): void + { + $fixture = __DIR__ . '/../../fixtures/entity_tree/system/missing_target.ent'; + $resolver = new EntityResolver([], [$fixture]); + + self::assertArrayNotHasKey('missing', $resolver->paths()); + } +} diff --git a/tests/Unit/Runner/RunPlannerTest.php b/tests/Unit/Runner/RunPlannerTest.php new file mode 100644 index 0000000..fa0a912 --- /dev/null +++ b/tests/Unit/Runner/RunPlannerTest.php @@ -0,0 +1,67 @@ +createMock(DiffProviderInterface::class); + $diffProvider + ->expects(self::once()) + ->method('for') + ->willReturn(<<<'DIFF' +diff --git a/nonexistent.xml b/nonexistent.xml +--- a/nonexistent.xml ++++ b/nonexistent.xml +@@ -1 +1 @@ +-old ++new +DIFF); + + $planner = new RunPlanner($config, diffProvider: $diffProvider); + + self::assertSame([], $planner->plan([], null)->targets); + } +} diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 47b6d25..c98c4be 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -8,6 +8,8 @@ use DocbookCS\Config\SniffEntry; use DocbookCS\Diff\Diff; use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; @@ -17,7 +19,11 @@ use DocbookCS\Report\Report; use DocbookCS\Report\Severity; use DocbookCS\Report\Violation; +use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\SniffRunner; use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Sniff\SniffInterface; @@ -35,12 +41,18 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), CoversClass(SniffRunner::class), CoversClass(Violation::class), CoversClass(XmlFileProcessor::class), UsesClass(Diff::class), + UsesClass(DiffPathLoader::class), + UsesClass(EntityExpansionMarker::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(RunScopeResolver::class), ] final class SniffRunnerTest extends TestCase { @@ -65,7 +77,7 @@ public function itProcessesFilesWithoutViolations(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); @@ -78,7 +90,7 @@ public function itUsesOverridePathsWhenProvided(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config, [self::FIXTURE_DIR . '/../override']); + $report = $runner->run($this->planPaths($config, [self::FIXTURE_DIR . '/../override'])); self::assertSame(1, $report->getFilesScanned()); } @@ -101,7 +113,7 @@ public function itCallsProgressMethods(): void $config = $this->createConfig(); $runner = new SniffRunner($progress); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -134,7 +146,7 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertCount(2, $report->getFileReports()); @@ -171,7 +183,7 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); foreach ($report->getFileReports() as $fileReport) { self::assertFalse( @@ -206,7 +218,7 @@ public function process(\DOMDocument $document, string $content, string $filePat $config = $this->createConfig(sniffs: [new SniffEntry($sniffClass::class, ['someProp' => 'someValue'])]); $runner = new SniffRunner(); - $runner->run($config); + $runner->run($this->planPaths($config)); self::assertSame('someValue', $sniffClass::$captured); } @@ -221,7 +233,7 @@ public function itThrowsWhenSniffClassDoesNotExist(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('does not exist'); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -234,7 +246,7 @@ public function itThrowsWhenClassDoesNotImplementSniffInterface(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('does not implement'); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -243,8 +255,8 @@ public function itFiltersFilesToOnlyThoseInTheDiff(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [1])]); - $report = $runner->run($config, null, $diff); + $diff = new Diff([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [1])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } @@ -256,7 +268,7 @@ public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void $runner = new SniffRunner(); $diff = new Diff([new FileChange('completely/different/file.xml', [1, 2, 3])]); - $report = $runner->run($config, null, $diff); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(0, $report->getFilesScanned()); } @@ -270,7 +282,7 @@ public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void $discoveredPath = self::FIXTURE_DIR . '/file_a.xml'; $diff = new Diff([new FileChange($discoveredPath, [1])]); - $report = $runner->run($config, null, $diff); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } @@ -281,11 +293,55 @@ public function itScansAllFilesWhenNoDiffIsGiven(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); } + #[Test] + public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void + { + $directory = sys_get_temp_dir() . '/docbook-cs-scan-' . bin2hex(random_bytes(6)); + mkdir($directory); + + $sourceFile = $directory . '/source.xml'; + $targetFile = $directory . '/target.xml'; + $entityFile = $directory . '/bridge.ent'; + + file_put_contents($sourceFile, '&bridge;'); + file_put_contents($targetFile, ''); + file_put_contents($entityFile, '⌖'); + + try { + $config = new ConfigData([], [], [], [], [], $directory); + $resolver = new RunScopeResolver( + $config, + [ + 'bridge' => $entityFile, + 'target' => $directory . '/./target.xml', + ], + wide: true, + ); + $plan = new RunPlan( + sniffs: [], + targets: $resolver->resolvePaths([$directory . '/.']), + entities: [ + 'bridge' => '⌖', + 'target' => '', + ], + ); + + $report = new SniffRunner()->run($plan); + + self::assertSame(2, $report->getFilesScanned()); + } finally { + @unlink($sourceFile); + @unlink($targetFile); + @unlink($entityFile); + @rmdir($directory); + } + } + #[Test] public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { @@ -316,10 +372,21 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [])]); - $report = $runner->run($config, null, $diff); + $diff = new Diff([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); } + + /** @param list|null $paths */ + private function planPaths(ConfigData $config, ?array $paths = null): RunPlan + { + return new RunPlanner($config)->planPaths($paths ?? $config->getIncludePaths()); + } + + private function planDiff(ConfigData $config, Diff $diff): RunPlan + { + return new RunPlanner($config)->planDiff($diff); + } }