Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion bin/docbook-cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ use DocbookCS\Application;
exit(2);
})();

new Application($argv ?? [])->run();
exit(Application::fromGlobals($argv ?? [])->run());
3 changes: 3 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
<testsuite name="integration">
<directory>tests/Integration</directory>
</testsuite>
<testsuite name="feature">
<directory>tests/Feature</directory>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in an earlier PR, I prefer to have this in Unit as well.

</testsuite>
</testsuites>

<source restrictNotices="true" restrictWarnings="true">
Expand Down
153 changes: 64 additions & 89 deletions src/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,29 +32,64 @@ final class Application
/** @var resource */
private $stderr;

/** @var resource */
private $stdin;
private ?string $stdin;

/**
* @param list<string> $argv
* @throws \RuntimeException if redirected stdin cannot be read.
* @api
*/
public static function fromGlobals(array $argv): self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withArguments would be better since Globals could mean something different.

{
$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<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)
{
public function __construct(
array $argv,
mixed $stdout = null,
mixed $stderr = null,
?string $stdin = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having $stdin as a string variable doesn't make sense. We should rename this to $diff or $diffContents.

) {
$this->argv = $argv;
$this->stdout = $stdout ?? STDOUT;
$this->stderr = $stderr ?? STDERR;
$this->stdin = $stdin ?? STDIN;
$this->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();
Expand All @@ -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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$runPlanner

} catch (\Throwable $e) {
$this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL);

Expand All @@ -118,27 +141,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,
Expand All @@ -148,9 +150,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
{
Expand All @@ -162,7 +165,7 @@ private function parseArgv(): array
'colors' => $this->detectColorSupport(),
'quiet' => false,
'paths' => [],
'diff' => null,
'wide' => false,
'perf' => false,
];

Expand Down Expand Up @@ -227,57 +230,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
{
Expand Down Expand Up @@ -382,22 +359,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;

Expand Down
16 changes: 15 additions & 1 deletion src/Diff/DiffParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,23 @@ public function parse(string $diff): Diff
{
/** @var array<string, list<int>> $changedLinesByFile */
$changedLinesByFile = [];
/** @var array<string, list<int>> $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 ')) {
$currentFile = null;
$deleted = false;
$newLineNumber = 0;
$inHunk = false;
$previousLineWasDeletion = false;
continue;
}

Expand All @@ -43,6 +47,7 @@ public function parse(string $diff): Diff
$inHunk = false;
if ($currentFile !== null && !isset($changedLinesByFile[$currentFile])) {
$changedLinesByFile[$currentFile] = [];
$deletionAnchorsByFile[$currentFile] = [];
}
continue;
}
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions src/Diff/DiffProviderInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace DocbookCS\Diff;

interface DiffProviderInterface
{
public function for(string $workingDirectory): string;
}
Loading