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/RelativePath.php b/src/RelativePath.php
new file mode 100644
index 0000000..1f0689b
--- /dev/null
+++ b/src/RelativePath.php
@@ -0,0 +1,23 @@
+createElement('file');
- $fileNode->setAttribute('name', $fileReport->filePath);
+ $fileNode->setAttribute('name', RelativePath::fromWorkingDirectory($fileReport->filePath));
foreach ($fileReport->getViolations() as $violation) {
$errorNode = $dom->createElement('error');
diff --git a/src/Report/Reporter/ConsoleReporter.php b/src/Report/Reporter/ConsoleReporter.php
index 2599bce..3e2a1bb 100644
--- a/src/Report/Reporter/ConsoleReporter.php
+++ b/src/Report/Reporter/ConsoleReporter.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Report\Reporter;
+use DocbookCS\RelativePath;
use DocbookCS\Report\Report;
use DocbookCS\Report\Severity;
@@ -27,9 +28,11 @@ public function generate(Report $report): string
continue;
}
+ $filePath = RelativePath::fromWorkingDirectory($fileReport->filePath);
+
$output .= PHP_EOL;
- $output .= $this->bold('FILE: ' . $fileReport->filePath) . PHP_EOL;
- $output .= str_repeat('-', min(80, 6 + strlen($fileReport->filePath))) . PHP_EOL;
+ $output .= $this->bold('FILE: ' . $filePath) . PHP_EOL;
+ $output .= str_repeat('-', min(80, 6 + strlen($filePath))) . PHP_EOL;
foreach ($fileReport->getViolations() as $violation) {
$output .= sprintf(
@@ -41,7 +44,7 @@ public function generate(Report $report): string
) . PHP_EOL;
}
- $output .= str_repeat('-', min(80, 6 + strlen($fileReport->filePath))) . PHP_EOL;
+ $output .= str_repeat('-', min(80, 6 + strlen($filePath))) . PHP_EOL;
}
$output .= PHP_EOL;
diff --git a/src/Report/Reporter/JsonReporter.php b/src/Report/Reporter/JsonReporter.php
index 0e52b0f..0eb3e0f 100644
--- a/src/Report/Reporter/JsonReporter.php
+++ b/src/Report/Reporter/JsonReporter.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Report\Reporter;
+use DocbookCS\RelativePath;
use DocbookCS\Report\Report;
final class JsonReporter implements ReporterInterface
@@ -38,7 +39,7 @@ public function generate(Report $report): string
];
}
- $data['files'][$fileReport->filePath] = [
+ $data['files'][RelativePath::fromWorkingDirectory($fileReport->filePath)] = [
'violations' => count($violations),
'messages' => $violations,
];
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..eee358d 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,50 +20,34 @@ 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,
$changedLines,
- $this->makeRelative($file),
+ $file,
);
$violationCount = $fileReport->getViolationCount();
@@ -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();
@@ -125,21 +104,4 @@ private function instantiateSniffs(array $entries): array
return $sniffs;
}
-
- private function makeRelative(string $absolutePath): string
- {
- $cwd = getcwd();
- if ($cwd === false) {
- return $absolutePath; // @codeCoverageIgnore
- }
-
- $prefix = rtrim(str_replace('\\', '/', $cwd), '/') . '/';
- $normalized = str_replace('\\', '/', $absolutePath);
-
- if (str_starts_with($normalized, $prefix)) {
- return substr($normalized, strlen($prefix));
- }
-
- return $absolutePath; // @codeCoverageIgnore
- }
}
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/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php
index 86eca0c..c15648f 100644
--- a/tests/Unit/Report/ReportTest.php
+++ b/tests/Unit/Report/ReportTest.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Tests\Unit\Report;
+use DocbookCS\RelativePath;
use DocbookCS\Report\FileReport;
use DocbookCS\Report\Report;
use DocbookCS\Report\Severity;
@@ -14,6 +15,7 @@
#[
CoversClass(FileReport::class),
+ CoversClass(RelativePath::class),
CoversClass(Report::class),
CoversClass(Violation::class),
]
@@ -94,6 +96,16 @@ public function itOverwritesFileReportWithSamePath(): void
self::assertSame($second, $report->getFileReports()['file.xml']);
}
+ #[Test]
+ public function itKeepsTheFileReportPathWhileRenderingItRelativeToWorkingDirectory(): void
+ {
+ $filePath = (getcwd() ?: '') . '/src/chapter.xml';
+ $fileReport = new FileReport($filePath);
+
+ self::assertSame($filePath, $fileReport->filePath);
+ self::assertSame('src/chapter.xml', RelativePath::fromWorkingDirectory($fileReport->filePath));
+ }
+
#[Test]
public function itReturnsTotalViolationsAcrossAllFiles(): void
{
diff --git a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php
index 87cbfb1..ec30288 100644
--- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php
+++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Tests\Unit\Report\Reporter;
+use DocbookCS\RelativePath;
use DocbookCS\Report\FileReport;
use DocbookCS\Report\Report;
use DocbookCS\Report\Reporter\CheckstyleReporter;
@@ -11,6 +12,7 @@
use DocbookCS\Report\Violation;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
use PHPUnit\Framework\TestCase;
#[
@@ -18,6 +20,8 @@
CoversClass(FileReport::class),
CoversClass(Report::class),
CoversClass(Violation::class),
+ //
+ UsesClass(RelativePath::class),
]
final class CheckstyleReporterTest extends TestCase
{
@@ -113,6 +117,23 @@ public function itIncludesFileNodeWithNameAttribute(): void
self::assertSame('src/broken.xml', $fileNodes->item(0)?->getAttribute('name'));
}
+ #[Test]
+ public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void
+ {
+ $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml');
+ $fileReport->addViolation($this->createViolation());
+
+ $report = new Report();
+ $report->addFileReport($fileReport);
+
+ $dom = $this->parseOutput($this->reporter->generate($report));
+
+ self::assertSame(
+ 'src/broken.xml',
+ $dom->getElementsByTagName('file')->item(0)?->getAttribute('name'),
+ );
+ }
+
#[Test]
public function itSetsLineAttribute(): void
{
diff --git a/tests/Unit/Report/Reporter/ConsoleReporterTest.php b/tests/Unit/Report/Reporter/ConsoleReporterTest.php
index ad2ba58..34f0bac 100644
--- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php
+++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Tests\Unit\Report\Reporter;
+use DocbookCS\RelativePath;
use DocbookCS\Report\FileReport;
use DocbookCS\Report\Report;
use DocbookCS\Report\Reporter\ConsoleReporter;
@@ -11,6 +12,7 @@
use DocbookCS\Report\Violation;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
use PHPUnit\Framework\TestCase;
#[
@@ -18,6 +20,8 @@
CoversClass(FileReport::class),
CoversClass(Report::class),
CoversClass(Violation::class),
+ //
+ UsesClass(RelativePath::class),
]
final class ConsoleReporterTest extends TestCase
{
@@ -90,6 +94,20 @@ public function itShowsFilePathInHeader(): void
self::assertStringContainsString('FILE: src/broken.xml', $output);
}
+ #[Test]
+ public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void
+ {
+ $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml');
+ $fileReport->addViolation($this->createViolation());
+
+ $report = new Report();
+ $report->addFileReport($fileReport);
+
+ $output = $this->reporter->generate($report);
+
+ self::assertStringContainsString('FILE: src/broken.xml', $output);
+ }
+
#[Test]
public function itShowsDashSeparatorAfterFileHeader(): void
{
diff --git a/tests/Unit/Report/Reporter/JsonReporterTest.php b/tests/Unit/Report/Reporter/JsonReporterTest.php
index 3cbbe72..0a7915c 100644
--- a/tests/Unit/Report/Reporter/JsonReporterTest.php
+++ b/tests/Unit/Report/Reporter/JsonReporterTest.php
@@ -4,6 +4,7 @@
namespace DocbookCS\Tests\Unit\Report\Reporter;
+use DocbookCS\RelativePath;
use DocbookCS\Report\FileReport;
use DocbookCS\Report\Report;
use DocbookCS\Report\Reporter\JsonReporter;
@@ -11,6 +12,7 @@
use DocbookCS\Report\Violation;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
use PHPUnit\Framework\TestCase;
#[
@@ -18,6 +20,8 @@
CoversClass(JsonReporter::class),
CoversClass(Report::class),
CoversClass(Violation::class),
+ //
+ UsesClass(RelativePath::class),
]
final class JsonReporterTest extends TestCase
{
@@ -312,6 +316,20 @@ public function itDoesNotEscapeSlashesInOutput(): void
self::assertStringNotContainsString('path\/to\/file.xml', $output);
}
+ #[Test]
+ public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void
+ {
+ $fileReport = new FileReport((getcwd() ?: '') . '/path/to/file.xml');
+ $fileReport->addViolation($this->createViolation());
+
+ $report = new Report();
+ $report->addFileReport($fileReport);
+
+ $data = $this->parseOutput($this->reporter->generate($report));
+
+ self::assertArrayHasKey('path/to/file.xml', $data['files']);
+ }
+
#[Test]
public function itUsesPrettyPrintedJson(): void
{
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..6118344 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());
@@ -142,7 +154,7 @@ public function setProperty(string $name, string $value): void
}
#[Test]
- public function itStoresRelativePathsInFileReports(): void
+ public function itStoresAbsolutePathsInFileReports(): void
{
$sniff = new class implements SniffInterface {
public function getCode(): string
@@ -171,12 +183,12 @@ 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(
+ self::assertTrue(
str_starts_with($fileReport->filePath, '/'),
- 'Expected relative path, got: ' . $fileReport->filePath,
+ 'Expected absolute path, got: ' . $fileReport->filePath,
);
}
}
@@ -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);
+ }
}