Rework CLI input and scope handling (#33)

* refactor: reworked command flags, scopes, and diff detection
* review: removed legacy --diff flag acceptance

Author: NickSdot

README.md

@@ -63,6 +63,26 @@ Register it in your config:
 <sniff class="Acme\DocbookSniffs\MySniff" />
 ```
 
+## 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

bin/docbook-cs

@@ -25,4 +25,4 @@ use DocbookCS\Application;
     exit(2);
 })();
 
-new Application($argv ?? [])->run();
+exit(Application::withArguments($argv ?? [])->run());

phpstan.neon

@@ -7,6 +7,7 @@ parameters:
 
   paths:
     - bin/
+    - bin/docbook-cs # extension-less file
     - src/
     - tests/
 

phpunit.xml.dist

@@ -18,9 +18,6 @@
 		<testsuite name="unit">
 			<directory>tests/Unit</directory>
 		</testsuite>
-		<testsuite name="integration">
-			<directory>tests/Integration</directory>
-		</testsuite>
 	</testsuites>
 
 	<source restrictNotices="true" restrictWarnings="true">

src/Application.php

@@ -7,14 +7,14 @@
 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;
 use DocbookCS\Report\Reporter\CheckstyleReporter;
 use DocbookCS\Report\Reporter\ConsoleReporter;
 use DocbookCS\Report\Reporter\JsonReporter;
 use DocbookCS\Report\Reporter\ReporterInterface;
+use DocbookCS\Runner\RunPlanner;
 use DocbookCS\Runner\SniffRunner;
 
 final class Application
@@ -23,38 +23,65 @@ final class Application
 
     private const string DEFAULT_CONFIG = 'docbookcs.xml';
 
-    /** @var list<string> */
-    private array $argv;
-
     /** @var resource */
     private $stdout;
 
     /** @var resource */
     private $stderr;
 
-    /** @var resource */
-    private $stdin;
+    /**
+     * @param list<string> $argv
+     * @throws \RuntimeException if redirected stdin cannot be read.
+     */
+    public static function withArguments(array $argv): self
+    {
+        $stdin = null;
+        $stat = fstat(STDIN);
+
+        if ($stat === false) {
+            return new self($argv, unifiedDiff: $stdin);
+        }
+
+        $type = $stat['mode'] & 0170000;
+
+        if ($type === 0010000 || $type === 0100000) {
+            $stdin = stream_get_contents(STDIN);
+
+            if ($stdin === false) {
+                throw new \RuntimeException('Could not read diff from stdin.');
+            }
+        }
+
+        return new self($argv, unifiedDiff: $stdin);
+    }
 
     /**
      * @param list<string> $argv
      * @param ?resource $stdout
      * @param ?resource $stderr
-     * @param ?resource $stdin
      */
-    public function __construct(array $argv, mixed $stdout = null, mixed $stderr = null, mixed $stdin = null)
-    {
-        $this->argv = $argv;
+    public function __construct(
+        private readonly array $argv,
+        mixed $stdout = null,
+        mixed $stderr = null,
+        private readonly ?string $unifiedDiff = null,
+    ) {
         $this->stdout = $stdout ?? STDOUT;
         $this->stderr = $stderr ?? STDERR;
-        $this->stdin = $stdin ?? STDIN;
     }
 
     /**
      * @return int Exit code (0 = success, 1 = violations found, 2 = runtime error).
      */
     public function run(): int
     {
-        $options = $this->parseArgv();
+        try {
+            $options = $this->parseArgv();
+        } catch (\InvalidArgumentException $e) {
+            $this->writeError('Error: ' . $e->getMessage() . PHP_EOL);
+
+            return 2;
+        }
 
         if ($options['help']) {
             $this->printHelp();
@@ -76,31 +103,19 @@ public function run(): int
             return 2;
         }
 
-        $overridePaths = $options['paths'] !== [] ? $options['paths'] : null;
-
-        // If override paths are relative, resolve them against cwd.
-        if ($overridePaths !== null) {
-            $overridePaths = $this->resolveOverridePaths($overridePaths);
-        }
-
-        $diff = null;
-
-        if ($options['diff'] !== null) {
-            try {
-                $diffContent = $this->readDiff($options['diff']);
-                $diff = (new DiffParser())->parse($diffContent);
-            } catch (\Throwable $e) {
-                $this->writeError('Error reading diff: ' . $e->getMessage() . PHP_EOL);
+        try {
+            $runPlan = new RunPlanner($config, $options['wide'])->plan($options['paths'], $this->unifiedDiff);
+        } catch (\Throwable $e) {
+            $this->writeError('Error resolving input: ' . $e->getMessage() . PHP_EOL);
 
-                return 2;
-            }
+            return 2;
         }
 
         $progress = $this->createProgress($options);
 
         try {
             $runner = new SniffRunner($progress);
-            $report = $runner->run($config, $overridePaths, $diff);
+            $report = $runner->run($runPlan);
         } catch (\Throwable $e) {
             $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL);
 
@@ -118,27 +133,6 @@ public function run(): int
         return (int) $report->hasViolations();
     }
 
-    /**
-     * @param list<string> $paths
-     * @return list<string>
-     */
-    private function resolveOverridePaths(array $paths): array
-    {
-        $cwd = getcwd() ?: '.';
-        $resolved = [];
-
-        foreach ($paths as $path) {
-            if (str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $path)) {
-                $resolved[] = $path;
-                continue;
-            }
-
-            $resolved[] = $cwd . '/' . $path;
-        }
-
-        return $resolved;
-    }
-
     /**
      * @return array{
      *     help: bool,
@@ -148,9 +142,10 @@ private function resolveOverridePaths(array $paths): array
      *     colors: bool,
      *     quiet: bool,
      *     paths: list<string>,
-     *     diff: string|null,
+     *     wide: bool,
      *     perf: bool,
      * }
+     * @throws \InvalidArgumentException for unsupported options.
      */
     private function parseArgv(): array
     {
@@ -162,7 +157,7 @@ private function parseArgv(): array
             'colors' => $this->detectColorSupport(),
             'quiet' => false,
             'paths' => [],
-            'diff' => null,
+            'wide' => false,
             'perf' => false,
         ];
 
@@ -227,57 +222,31 @@ 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;
             }
 
             // Anything else is a path to scan.
             if (!str_starts_with($arg, '-')) {
                 $result['paths'][] = $arg;
+                $i++;
+                continue;
             }
 
-            $i++;
+            throw new \InvalidArgumentException(sprintf('Unknown option: %s', $arg));
         }
 
         return $result;
     }
 
-    /** @throws \RuntimeException if the source cannot be read. */
-    private function readDiff(string $source): string
-    {
-        if ($source === '' || $source === '-') {
-            $content = stream_get_contents($this->stdin);
-            if ($content === false) {
-                throw new \RuntimeException('Could not read diff from stdin.'); // @codeCoverageIgnore
-            }
-            return $content;
-        }
-
-        $content = @file_get_contents($source);
-        if ($content === false) {
-            throw new \RuntimeException(sprintf('Could not read diff file: %s', $source));
-        }
-
-        return $content;
-    }
-
     /** @param array{report: string, quiet: bool, colors: bool} $options */
     private function createProgress(array $options): ProgressInterface
     {
@@ -382,22 +351,20 @@ private function printHelp(): void
   --report=<format>     Output format: console (default), checkstyle, json.
   --colors              Force ANSI color output.
   --no-colors           Disable ANSI color output.
-  --diff[=<file>]       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:
   <file-or-directory>   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;