diff --git a/README.md b/README.md index 5ee464c..179d77d 100644 --- a/README.md +++ b/README.md @@ -40,15 +40,16 @@ Implement `DocbookCS\Sniff\SniffInterface` (or extend `AbstractSniff`): namespace Acme\DocbookSniffs; use DocbookCS\Sniff\AbstractSniff; +use DocbookCS\Source\File; final class MySniff extends AbstractSniff { - public function getCode(): string + public static function getCode(): string { return 'Acme.MySniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { $violations = []; // ... inspect $document, add violations via $this->createViolation(...) @@ -63,6 +64,27 @@ 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 violations and fixes remain limited +to the given scope. With `--wide`, every file, inferred from paths or diff, as +a whole will be checked and referenced `SYSTEM` XML files recursively included +in violation reports and fixing. + +| 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..5db6d29 100644 --- a/bin/docbook-cs +++ b/bin/docbook-cs @@ -14,7 +14,7 @@ use DocbookCS\Application; foreach ($candidates as $file) { if (!is_file($file)) { - continue; + continue; } require $file; @@ -25,4 +25,11 @@ use DocbookCS\Application; exit(2); })(); -new Application($argv ?? [])->run(); +try { + $application = Application::fromGlobals($argv ?? []); +} catch (\RuntimeException $e) { + fwrite(STDERR, $e->getMessage() . PHP_EOL); + exit(2); +} + +exit($application->run()); diff --git a/phpcs.xml b/phpcs.xml index 93f3b2f..3495fd9 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -2,6 +2,9 @@ DocbookCS Coding Standard + + tests/* + 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..59e7e3e 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,7 +14,9 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Report\Reporter\ReporterInterface; -use DocbookCS\Runner\SniffRunner; +use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\RunPlanner; final class Application { @@ -32,21 +33,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 +84,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 +112,22 @@ 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: $config, + mode: RunMode::fromFixFlag($options['fix']), + wide: $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 = new RunCoordinator($progress)->run($runPlan); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); @@ -118,27 +145,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, @@ -146,11 +152,13 @@ private function resolveOverridePaths(array $paths): array * config: string, * report: string, * colors: bool, + * fix: bool, + * wide: bool, * quiet: bool, * paths: list, - * diff: string|null, * perf: bool, * } + * @throws \InvalidArgumentException for unsupported or removed options. */ private function parseArgv(): array { @@ -160,9 +168,10 @@ private function parseArgv(): array 'config' => self::DEFAULT_CONFIG, 'report' => 'console', 'colors' => $this->detectColorSupport(), + 'fix' => false, + 'wide' => false, 'quiet' => false, 'paths' => [], - 'diff' => null, 'perf' => false, ]; @@ -227,23 +236,20 @@ private function parseArgv(): array continue; } - // --diff = read from stdin - // --diff=FILE = read from file - // --diff=- = read from stdin (explicit) - if ($arg === '--diff') { - $result['diff'] = ''; + if ($arg === '--perf') { + $result['perf'] = true; $i++; continue; } - if (str_starts_with($arg, '--diff=')) { - $result['diff'] = substr($arg, 7); + if ($arg === '--fix') { + $result['fix'] = true; $i++; continue; } - if ($arg === '--perf') { - $result['perf'] = true; + if ($arg === '--wide') { + $result['wide'] = true; $i++; continue; } @@ -251,33 +257,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 +371,27 @@ 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). + --fix Automatically fix violations when fixers exist + (experimental). + --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 . --fix + docbook-cs reference/ docbook-cs reference/strings/functions/strlen.xml - git diff HEAD | docbook-cs --diff --report=checkstyle - docbook-cs --diff=changes.patch --report=json + docbook-cs reference/strings/functions/strlen.xml --wide + docbook-cs reference/strings/functions/strlen.xml --wide --fix + git diff HEAD | docbook-cs + git diff HEAD | docbook-cs --wide + git diff HEAD | docbook-cs --wide --fix --report=checkstyle HELP; diff --git a/src/Diff/Diff.php b/src/Diff/DiffChangeset.php similarity index 95% rename from src/Diff/Diff.php rename to src/Diff/DiffChangeset.php index 83b3bd9..c6f5319 100644 --- a/src/Diff/Diff.php +++ b/src/Diff/DiffChangeset.php @@ -4,7 +4,7 @@ namespace DocbookCS\Diff; -final readonly class Diff +final readonly class DiffChangeset { /** @param list $fileChanges */ public function __construct(public array $fileChanges) diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 9dbd2fe..b43c2c3 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -8,16 +8,19 @@ final class DiffParser { private const string NO_FINAL_LINE_MARKER = '\ No newline at end of file'; - public function parse(string $diff): Diff + public function parse(string $diff): DiffChangeset { /** @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,26 +76,38 @@ 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); + return new DiffChangeset($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..fe4e6b3 --- /dev/null +++ b/src/Diff/GitDiffProvider.php @@ -0,0 +1,99 @@ +runOrThrow( + ['git', 'rev-parse', '--show-toplevel'], + $workingDirectory, + 'Could not find Git repository.', + )); + + $baseReference = $this->resolveBaseReference($repositoryRoot); + + $error = sprintf('Unclear where HEAD branched from %s.', $baseReference); + + $mergeBase = $this->runOrThrow( + ['git', 'merge-base', 'HEAD', $baseReference], + $repositoryRoot, + $error, + ); + + 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/Fix/Fix.php b/src/Fix/Fix.php new file mode 100644 index 0000000..031b262 --- /dev/null +++ b/src/Fix/Fix.php @@ -0,0 +1,18 @@ + $fixes + */ + public function apply(File $file, array $fixes): FixResult + { + if ($fixes === []) { + return new FixResult($file); + } + + $plans = []; + foreach ($fixes as $index => $fix) { + $plan = $fix instanceof FixPlan ? $fix : new FixPlan($fix); + $plans[] = [ + 'index' => $index, + 'plan' => $plan, + ]; + } + + usort($plans, static function (array $a, array $b): int { + $offsetComparison = $a['plan']->firstOffset() <=> $b['plan']->firstOffset(); + + return $offsetComparison !== 0 + ? $offsetComparison + : $a['index'] <=> $b['index']; + }); + + /** @var list $acceptedFixes */ + $acceptedFixes = []; + $acceptedPlans = 0; + $skipped = 0; + + $content = $file->content; + $length = strlen($content); + + foreach ($plans as ['plan' => $plan]) { + if (!$this->canApply($file, $length, $plan, $acceptedFixes)) { + $skipped++; + continue; + } + + if (!$this->changesContent($content, $plan)) { + $skipped++; + continue; + } + + foreach ($plan->fixes as $fix) { + $this->insertFix($acceptedFixes, $fix); + } + + $acceptedPlans++; + } + + $fixedContent = ''; + $sourceOffset = 0; + + foreach ($acceptedFixes as $fix) { + $fixedContent .= substr($content, $sourceOffset, $fix->beginOffset - $sourceOffset); + $fixedContent .= $fix->replacement; + $sourceOffset = $fix->untilOffset; + } + + $content = $fixedContent . substr($content, $sourceOffset); + + return new FixResult( + file: $file->withContent($content), + applied: $acceptedPlans, + skipped: $skipped, + appliedFixes: $acceptedFixes, + ); + } + + /** + * @param list $acceptedFixes + */ + private function canApply(File $file, int $contentLength, FixPlan $plan, array $acceptedFixes): bool + { + $content = $file->content; + $first = $plan->fixes[0]; + $previous = null; + + foreach ($plan->fixes as $fix) { + if ( + $fix->filePath !== $file->path + || $fix->filePath !== $first->filePath + || $fix->sniffCode !== $first->sniffCode + || $fix->beginOffset < 0 + || $fix->untilOffset < $fix->beginOffset + || $fix->untilOffset > $contentLength + || ($previous !== null && self::overlaps($previous, $fix)) + || $this->conflictsWithAcceptedFix($fix, $acceptedFixes) + ) { + return false; + } + + $currentContent = substr($content, $fix->beginOffset, $fix->untilOffset - $fix->beginOffset); + + if ($fix->expectedContent !== null && $currentContent !== $fix->expectedContent) { + return false; + } + + $previous = $fix; + } + + return true; + } + + private function changesContent(string $content, FixPlan $plan): bool + { + foreach ($plan->fixes as $fix) { + $currentContent = substr($content, $fix->beginOffset, $fix->untilOffset - $fix->beginOffset); + + if ($currentContent !== $fix->replacement) { + return true; + } + } + + return false; + } + + /** + * @param list $acceptedFixes + */ + private function conflictsWithAcceptedFix(Fix $fix, array $acceptedFixes): bool + { + $index = $this->insertionIndex($acceptedFixes, $fix); + + return ($index > 0 && self::overlaps($acceptedFixes[$index - 1], $fix)) + || ($index < count($acceptedFixes) && self::overlaps($acceptedFixes[$index], $fix)); + } + + /** + * @param list $fixes + */ + private function insertFix(array &$fixes, Fix $fix): void + { + $lastIndex = count($fixes) - 1; + if ($lastIndex < 0 || self::compare($fixes[$lastIndex], $fix) < 0) { + $fixes[] = $fix; + return; + } + + array_splice($fixes, $this->insertionIndex($fixes, $fix), 0, [$fix]); + } + + /** + * @param list $fixes + */ + private function insertionIndex(array $fixes, Fix $fix): int + { + $low = 0; + $high = count($fixes); + + while ($low < $high) { + $middle = intdiv($low + $high, 2); + if (self::compare($fixes[$middle], $fix) < 0) { + $low = $middle + 1; + } else { + $high = $middle; + } + } + + return $low; + } + + private static function compare(Fix $a, Fix $b): int + { + return [ + $a->beginOffset, + $a->untilOffset, + ] <=> [ + $b->beginOffset, + $b->untilOffset, + ]; + } + + private static function overlaps(Fix $a, Fix $b): bool + { + $aIsInsertion = $a->beginOffset === $a->untilOffset; + $bIsInsertion = $b->beginOffset === $b->untilOffset; + + if ($aIsInsertion && $bIsInsertion) { + return $a->beginOffset === $b->beginOffset; + } + + if ($aIsInsertion) { + return $a->beginOffset > $b->beginOffset && $a->beginOffset < $b->untilOffset; + } + + if ($bIsInsertion) { + return $b->beginOffset > $a->beginOffset && $b->beginOffset < $a->untilOffset; + } + + return $a->beginOffset < $b->untilOffset && $b->beginOffset < $a->untilOffset; + } +} diff --git a/src/Fix/FixPlan.php b/src/Fix/FixPlan.php new file mode 100644 index 0000000..d3d2f63 --- /dev/null +++ b/src/Fix/FixPlan.php @@ -0,0 +1,34 @@ + */ + public array $fixes; + + public function __construct(Fix $first, Fix ...$additionalFixes) + { + $fixes = [$first, ...$additionalFixes]; + + usort( + $fixes, + static fn(Fix $a, Fix $b): int => [ + $a->beginOffset, + $a->untilOffset, + ] <=> [ + $b->beginOffset, + $b->untilOffset, + ], + ); + + $this->fixes = $fixes; + } + + public function firstOffset(): int + { + return $this->fixes[0]->beginOffset; + } +} diff --git a/src/Fix/FixResult.php b/src/Fix/FixResult.php new file mode 100644 index 0000000..e4d4b00 --- /dev/null +++ b/src/Fix/FixResult.php @@ -0,0 +1,19 @@ + $appliedFixes */ + public function __construct( + public File $file, + public int $applied = 0, + public int $skipped = 0, + public array $appliedFixes = [], + ) { + } +} diff --git a/src/Fix/Fixer/AttributeOrderFixer.php b/src/Fix/Fixer/AttributeOrderFixer.php new file mode 100644 index 0000000..c49ad3e --- /dev/null +++ b/src/Fix/Fixer/AttributeOrderFixer.php @@ -0,0 +1,80 @@ +'; + private const string OPENING_TAG_PATTERN = '/^<([a-zA-Z0-9:_-]+)\b([^<>]*?)>$/'; + private const string ATTRIBUTE_TOKEN_PATTERN = '/\s+([a-zA-Z0-9:_-]+)\s*=\s*(?:"[^"]*"|\'[^\']*\')/'; + + /** @throws FixerException */ + public function process(Violation $violation): Fix + { + if ($violation->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if (!preg_match(self::OPENING_TAG_PATTERN, $violation->content, $matches)) { + throw FixerException::cannotFixInvalidContent($violation); + } + + $fixedAttributeString = $this->fixedAttributeString($matches[2]); + + if ($fixedAttributeString === null) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return new Fix( + $violation->filePath, + $violation->beginOffset, + $violation->untilOffset, + sprintf(self::OPENING_TAG_FORMAT, $matches[1], $fixedAttributeString), + $violation->sniffCode, + ); + } + + private function fixedAttributeString(string $attrString): ?string + { + if (!str_contains($attrString, 'xml:id') || !str_contains($attrString, 'xmlns')) { + return null; + } + + preg_match_all(self::ATTRIBUTE_TOKEN_PATTERN, $attrString, $matches, PREG_OFFSET_CAPTURE); + + $xmlIdToken = null; + $firstXmlnsStart = null; + + foreach ($matches[0] as $i => [$token, $start]) { + $start = (int) $start; + $name = $matches[1][$i][0]; + + if ($name === 'xml:id') { + $xmlIdToken = [ + 'text' => $token, + 'start' => $start, + 'end' => $start + strlen($token), + ]; + } + + if (($name === 'xmlns' || str_starts_with($name, 'xmlns:')) && $firstXmlnsStart === null) { + $firstXmlnsStart = $start; + } + } + + if ($xmlIdToken === null || $firstXmlnsStart === null || $xmlIdToken['start'] < $firstXmlnsStart) { + return null; + } + + return substr($attrString, 0, $firstXmlnsStart) + . $xmlIdToken['text'] + . substr($attrString, $firstXmlnsStart, $xmlIdToken['start'] - $firstXmlnsStart) + . substr($attrString, $xmlIdToken['end']); + } +} diff --git a/src/Fix/Fixer/ExceptionNameFixer.php b/src/Fix/Fixer/ExceptionNameFixer.php new file mode 100644 index 0000000..0949246 --- /dev/null +++ b/src/Fix/Fixer/ExceptionNameFixer.php @@ -0,0 +1,54 @@ +]*)>([^<]*)<\/classname>$/'; + + /** @throws FixerException */ + public function process(Violation $violation): FixPlan + { + if ($violation->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if (!preg_match(self::CLASSNAME_PATTERN, $violation->content, $matches)) { + throw FixerException::cannotFixInvalidContent($violation); + } + + $text = trim($matches[2]); + + if ($text === '' || !ExceptionNameSniff::looksLikeException($text)) { + throw FixerException::cannotFixInvalidContent($violation); + } + + if (count($violation->affectedRanges) !== 2) { + throw FixerException::cannotFixInvalidContent($violation); + } + + $fixes = []; + foreach ($violation->affectedRanges as $range) { + $fixes[] = new Fix( + $violation->filePath, + $range->beginOffset, + $range->untilOffset, + self::TARGET_ELEMENT, + $violation->sniffCode, + self::SOURCE_ELEMENT, + ); + } + + return new FixPlan(...$fixes); + } +} diff --git a/src/Fix/Fixer/Fixer.php b/src/Fix/Fixer/Fixer.php new file mode 100644 index 0000000..a488c7f --- /dev/null +++ b/src/Fix/Fixer/Fixer.php @@ -0,0 +1,16 @@ +content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if ( + !preg_match(self::INDENTATION_PATTERN, $violation->content) + || !str_contains($violation->content, ' ') + || !str_contains($violation->content, "\t") + ) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return new Fix( + filePath: $violation->filePath, + beginOffset: $violation->beginOffset, + untilOffset: $violation->untilOffset, + replacement: str_replace("\t", ' ', $violation->content), + sniffCode: $violation->sniffCode, + expectedContent: $violation->content, + ); + } +} diff --git a/src/Fix/Fixer/SimparaFixer.php b/src/Fix/Fixer/SimparaFixer.php new file mode 100644 index 0000000..b815a26 --- /dev/null +++ b/src/Fix/Fixer/SimparaFixer.php @@ -0,0 +1,47 @@ +]*)>(.*)<\/para>$/s'; + + /** @throws FixerException */ + public function process(Violation $violation): FixPlan + { + if ($violation->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if (!preg_match(self::PARA_PATTERN, $violation->content, $matches)) { + throw FixerException::cannotFixInvalidContent($violation); + } + + if (count($violation->affectedRanges) !== 2) { + throw FixerException::cannotFixInvalidContent($violation); + } + + $fixes = []; + foreach ($violation->affectedRanges as $range) { + $fixes[] = new Fix( + $violation->filePath, + $range->beginOffset, + $range->untilOffset, + self::TARGET_ELEMENT, + $violation->sniffCode, + self::SOURCE_ELEMENT, + ); + } + + return new FixPlan(...$fixes); + } +} diff --git a/src/Fix/Fixer/TrailingWhitespaceFixer.php b/src/Fix/Fixer/TrailingWhitespaceFixer.php new file mode 100644 index 0000000..092e534 --- /dev/null +++ b/src/Fix/Fixer/TrailingWhitespaceFixer.php @@ -0,0 +1,35 @@ +content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if (!preg_match(self::WHITESPACE_PATTERN, $violation->content)) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return new Fix( + filePath: $violation->filePath, + beginOffset: $violation->beginOffset, + untilOffset: $violation->untilOffset, + replacement: '', + sniffCode: $violation->sniffCode, + expectedContent: $violation->content, + ); + } +} diff --git a/src/Fix/Fixer/WhitespaceFixer.php b/src/Fix/Fixer/WhitespaceFixer.php new file mode 100644 index 0000000..f630150 --- /dev/null +++ b/src/Fix/Fixer/WhitespaceFixer.php @@ -0,0 +1,43 @@ +content === null) { + throw FixerException::cannotFixMissingContent(); + } + + $fixed = rtrim($violation->content, " \t"); + + if (preg_match('/^[ \t]+/', $fixed, $matches)) { + $fixedIndent = str_replace("\t", ' ', $matches[0]); + $fixed = $fixedIndent . substr($fixed, strlen($matches[0])); + } + + if ($fixed === $violation->content) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return new Fix( + $violation->filePath, + $violation->beginOffset, + $violation->untilOffset, + $fixed, + $violation->sniffCode, + ); + } +} diff --git a/src/Fix/FixerException.php b/src/Fix/FixerException.php new file mode 100644 index 0000000..afad17b --- /dev/null +++ b/src/Fix/FixerException.php @@ -0,0 +1,49 @@ +sniffCode, + $violation->filePath, + $violation->line, + )); + } + + public static function cannotReadFixedContent(): self + { + return new self('Cannot read fixed content when no fix application was attempted.'); + } + + public static function invalidFixedXml(string $filePath): self + { + return new self( + sprintf('Fixers produced invalid XML for %s; no changes were written.', $filePath), + ); + } + + public static function didNotConverge(string $filePath): self + { + return new self( + sprintf('Fixers did not converge for %s; no changes were written.', $filePath), + ); + } +} diff --git a/src/Path/DiffPathLoader.php b/src/Path/DiffPathLoader.php new file mode 100644 index 0000000..82aa1d9 --- /dev/null +++ b/src/Path/DiffPathLoader.php @@ -0,0 +1,103 @@ + $projectRoots + */ + public function __construct( + private DiffChangeset $diff, + private string $workingDirectory, + private string $basePath, + private array $projectRoots, + private PathMatcher $matcher, + ) { + } + + public function load(): DiffChangeset + { + $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 DiffChangeset(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..aafb27b 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,13 +118,11 @@ private function scanDirectory(string $directory): array /** * @param array $visited + * @param array $paths * @return array */ - private function resolveFile( - string $filePath, - array &$visited = [], - ?string $originEntity = null - ): array { + private function resolveFile(string $filePath, array &$visited, array &$paths, ?string $originEntity = null): array + { if (isset($visited[$filePath]) || !is_readable($filePath)) { return []; } @@ -105,7 +135,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,27 +146,22 @@ private function resolveFile( /** * @param array $visited + * @param array $paths * @return array */ - private function extractEntities( - string $content, - string $filePath, - array &$visited - ): array { - return - $this->extractDtdEntities($content, $filePath, $visited) + private function extractEntities(string $content, string $filePath, array &$visited, array &$paths): array + { + return $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 { + private function extractDtdEntities(string $content, string $filePath, array &$visited, array &$paths): array + { $result = []; if (!str_contains($content, '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 @@ + */ @@ -19,6 +22,14 @@ public function addViolation(Violation $violation): void $this->violations[] = $violation; } + /** @param list $violations */ + public function addViolations(array $violations): void + { + foreach ($violations as $violation) { + $this->addViolation($violation); + } + } + /** @return list */ public function getViolations(): array { diff --git a/src/Report/Report.php b/src/Report/Report.php index 37ccbe5..77e7335 100644 --- a/src/Report/Report.php +++ b/src/Report/Report.php @@ -4,6 +4,8 @@ namespace DocbookCS\Report; +use DocbookCS\Violation\Violation; + final class Report { /** @var array */ diff --git a/src/Report/Reporter/CheckstyleReporter.php b/src/Report/Reporter/CheckstyleReporter.php index 6076f2c..024d9f9 100644 --- a/src/Report/Reporter/CheckstyleReporter.php +++ b/src/Report/Reporter/CheckstyleReporter.php @@ -4,6 +4,7 @@ namespace DocbookCS\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\Report; final class CheckstyleReporter implements ReporterInterface @@ -28,7 +29,7 @@ public function generate(Report $report): string } $fileNode = $dom->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..b6967c6 100644 --- a/src/Report/Reporter/ConsoleReporter.php +++ b/src/Report/Reporter/ConsoleReporter.php @@ -4,8 +4,9 @@ namespace DocbookCS\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; +use DocbookCS\Violation\Severity; final class ConsoleReporter implements ReporterInterface { @@ -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/Report/Violation.php b/src/Report/Violation.php deleted file mode 100644 index 24df707..0000000 --- a/src/Report/Violation.php +++ /dev/null @@ -1,17 +0,0 @@ -||<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/', function (array $matches) use (&$changed, $markXmlExpansions): string { - if ( - str_starts_with($matches[0], '', + ' ']]>', + ' '?>', + ]; + /** @var array */ protected array $properties = []; + public function __construct( + public RunMode $mode = RunMode::Sniff, + ) { + } + public function setProperty(string $name, string $value): void { $this->properties[$name] = $value; @@ -28,20 +41,116 @@ protected function isSourceBacked(\DOMNode $node): bool return !EntityExpansionMarker::contains($node); } - /** @throws \LogicException if an invalid severity level is configured */ + /** + * @param list $affectedRanges + * @throws \LogicException if an invalid severity level is configured + */ protected function createViolation( string $filePath, int $line, + int $beginOffset, + int $untilOffset, string $message, + ?string $content = null, Severity $severity = Severity::ERROR, + array $affectedRanges = [], ): Violation { return new Violation( - sniffCode: $this->getCode(), + sniffCode: static::getCode(), filePath: $filePath, line: $line, + beginOffset: $beginOffset, + untilOffset: $untilOffset, message: $message, + content: $content, severity: Severity::tryFrom($this->getProperty('severity', $severity->value)) ?: throw new \LogicException('Invalid severity level configured for ExceptionNameSniff.'), + affectedRanges: $affectedRanges, ); } + + protected function maskNonElementMarkup(string $source): string + { + $masked = $source; + $offset = 0; + + while (false !== $start = strpos($source, '<', $offset)) { + $endOffset = $this->nonElementMarkupEndOffset($source, $start); + + if ($endOffset === null) { + $offset = $start + 1; + continue; + } + + for ($i = $start; $i < $endOffset; $i++) { + $masked[$i] = ' '; + } + + $offset = $endOffset; + } + + return $masked; + } + + private function nonElementMarkupEndOffset(string $source, int $start): ?int + { + foreach (self::NON_ELEMENT_DELIMITERS as $opening => $closing) { + if (substr_compare($source, $opening, $start, strlen($opening)) === 0) { + return $this->offsetAfterDelimiter($source, $closing, $start); + } + } + + if (substr_compare($source, 'declarationEndOffset($source, $start); + } + + return null; + } + + private function offsetAfterDelimiter(string $source, string $delimiter, int $offset): int + { + $end = strpos($source, $delimiter, $offset); + + return $end === false ? strlen($source) : $end + strlen($delimiter); + } + + private function declarationEndOffset(string $source, int $offset): int + { + $length = strlen($source); + $quote = null; + $bracketDepth = 0; + + for ($i = $offset; $i < $length; $i++) { + $character = $source[$i]; + + if ($quote !== null) { + if ($character === $quote) { + $quote = null; + } + + continue; + } + + if ($character === '"' || $character === "'") { + $quote = $character; + continue; + } + + if ($character === '[') { + $bracketDepth++; + continue; + } + + if ($character === ']') { + $bracketDepth--; + continue; + } + + if ($character === '>' && $bracketDepth === 0) { + return $i + 1; + } + } + + return $length; + } } diff --git a/src/Sniff/AttributeOrderSniff.php b/src/Sniff/AttributeOrderSniff.php index b4d03eb..119824c 100644 --- a/src/Sniff/AttributeOrderSniff.php +++ b/src/Sniff/AttributeOrderSniff.php @@ -4,6 +4,9 @@ namespace DocbookCS\Sniff; +use DocbookCS\Fix\Fixer\AttributeOrderFixer; +use DocbookCS\Source\File; + /** * Ensures that when an element has both xml:id and xmlns (or xmlns:*) * attributes, xml:id appears first. @@ -11,20 +14,36 @@ * This is a stylistic convention in the PHP documentation project: * identity attributes should precede namespace declarations. */ -final class AttributeOrderSniff extends AbstractSniff +final class AttributeOrderSniff extends AbstractSniff implements Fixable { - public function getCode(): string + private const string OPENING_TAG_PATTERN = '/<([a-zA-Z0-9:_-]+)\b([^<>]*?)>/'; + private const string ATTRIBUTE_NAME_PATTERN = '/([a-zA-Z0-9:_-]+)\s*=/'; + + public static function getCode(): string { return 'DocbookCS.AttributeOrder'; } - /** @throws \LogicException if an invalid severity level is configured */ - public function process(\DOMDocument $document, string $content, string $filePath): array + public static function fixerClassName(): string + { + return AttributeOrderFixer::class; + } + + /** + * @throws \LogicException if an invalid severity level is configured + * @throws \OutOfBoundsException if a matched tag offset lies outside the source + */ + public function process(\DOMDocument $document, File $file): array { $violations = []; // Match ONLY opening tags (skip closing, comments, xml decl) - preg_match_all('/<([a-zA-Z0-9:_-]+)\b([^<>]*?)>/s', $content, $matches, PREG_OFFSET_CAPTURE); + preg_match_all( + self::OPENING_TAG_PATTERN, + $this->maskNonElementMarkup($file->content), + $matches, + PREG_OFFSET_CAPTURE, + ); foreach ($matches[0] as $i => [$fullMatch, $offset]) { $tagName = $matches[1][$i][0]; @@ -38,12 +57,17 @@ public function process(\DOMDocument $document, string $content, string $filePat continue; } + $beginOffset = (int) $offset; + $this->checkAttributes( $tagName, $attrString, - $filePath, - $this->lineFromOffset($content, (int)$offset), - $violations + $file->path, + $file->lineAtOffset($beginOffset)->number, + $beginOffset, + $beginOffset + strlen($fullMatch), + $violations, + $fullMatch, ); } @@ -51,7 +75,8 @@ public function process(\DOMDocument $document, string $content, string $filePat } /** - * @param list<\DocbookCS\Report\Violation> &$violations + * @param list<\DocbookCS\Violation\Violation> &$violations + * * @throws \LogicException if an invalid severity level is configured */ private function checkAttributes( @@ -59,9 +84,12 @@ private function checkAttributes( string $attrString, string $filePath, int $line, - array &$violations + int $beginOffset, + int $untilOffset, + array &$violations, + string $content, ): void { - preg_match_all('/([a-zA-Z0-9:_-]+)\s*=/', $attrString, $matches); + preg_match_all(self::ATTRIBUTE_NAME_PATTERN, $attrString, $matches); $attributes = $matches[1]; $xmlIdPos = null; @@ -80,20 +108,17 @@ private function checkAttributes( } } - if ($xmlIdPos !== null && $xmlnsPos !== PHP_INT_MAX && $xmlIdPos > $xmlnsPos) { - $violations[] = $this->createViolation( - $filePath, - $line, - sprintf( - 'Element <%s>: xml:id should appear before xmlns attributes.', - $tagName, - ), - ); + if ($xmlIdPos === null || $xmlnsPos === PHP_INT_MAX || $xmlIdPos <= $xmlnsPos) { + return; } - } - private function lineFromOffset(string $content, int $offset): int - { - return substr_count($content, "\n", 0, $offset) + 1; + $violations[] = $this->createViolation( + $filePath, + $line, + $beginOffset, + $untilOffset, + sprintf('Element <%s>: xml:id should appear before xmlns attributes.', $tagName), + $content, + ); } } diff --git a/src/Sniff/ExceptionNameSniff.php b/src/Sniff/ExceptionNameSniff.php index acbfbda..49c3cc4 100644 --- a/src/Sniff/ExceptionNameSniff.php +++ b/src/Sniff/ExceptionNameSniff.php @@ -4,6 +4,10 @@ namespace DocbookCS\Sniff; +use DocbookCS\Fix\Fixer\ExceptionNameFixer; +use DocbookCS\Source\File; +use DocbookCS\Violation\SourceRange; + /** * Detects exception/error class names wrapped in that * should use instead. @@ -12,10 +16,13 @@ * exception." When a element's text content matches a * known exception/error pattern, this sniff flags it. */ -final class ExceptionNameSniff extends AbstractSniff +final class ExceptionNameSniff extends AbstractSniff implements Fixable { + private const string ELEMENT_NAME = 'classname'; + /** * Default suffixes that indicate the class is an exception or error. + * @var list */ private const array DEFAULT_SUFFIXES = [ 'Exception', @@ -23,18 +30,33 @@ final class ExceptionNameSniff extends AbstractSniff 'Throwable', ]; - public function getCode(): string + private const string CLASSNAME_PATTERN = '/]*>([^<]*)<\/classname>/'; + + public static function getCode(): string { return 'DocbookCS.ExceptionName'; } - /** @throws \LogicException if an invalid severity level is configured */ - public function process(\DOMDocument $document, string $content, string $filePath): array + public static function fixerClassName(): string + { + return ExceptionNameFixer::class; + } + + /** + * @throws \LogicException + * @throws \OutOfBoundsException if a matched tag offset lies outside the source + */ + public function process(\DOMDocument $document, File $file): array { $violations = []; - $suffixes = $this->getSuffixes(); + $sourceMatchIndex = 0; $classnames = $document->getElementsByTagName('classname'); + if ($classnames->length === 0) { + return []; + } + + $sourceMatches = $this->sourceMatches($file); /** @var \DOMElement $node */ foreach ($classnames as $node) { @@ -43,6 +65,8 @@ public function process(\DOMDocument $document, string $content, string $filePat } $text = trim($node->textContent); + $match = $sourceMatches[$sourceMatchIndex] ?? null; + $sourceMatchIndex++; if ($text === '') { continue; @@ -52,38 +76,79 @@ public function process(\DOMDocument $document, string $content, string $filePat continue; } - if ($this->looksLikeException($text, $suffixes)) { - $violations[] = $this->createViolation( - $filePath, - $node->getLineNo(), - sprintf( - '"%s" is wrapped in but should use .', - $text, - ), - ); + if (!self::looksLikeException($text)) { + continue; + } + + if ($match === null || $match['text'] !== $text) { + throw new \LogicException('Could not map classname violation to source content.'); } + + $violations[] = $this->createViolation( + $file->path, + $match['affectedRanges'][0]->line, + $match['beginOffset'], + $match['untilOffset'], + sprintf('"%s" is wrapped in but should use .', $text), + $match['content'], + affectedRanges: $match['affectedRanges'], + ); } return $violations; } - /** - * @param list $suffixes - */ - private function looksLikeException(string $text, array $suffixes): bool + public static function looksLikeException(string $text): bool { $parts = explode('\\', $text); $baseName = end($parts); - return array_any( - $suffixes, - static fn($suffix) => str_ends_with($baseName, $suffix) - ); + return array_any(self::DEFAULT_SUFFIXES, static fn(string $suffix): bool => str_ends_with($baseName, $suffix)); } - /** @return list */ - private function getSuffixes(): array + /** + * @return list + * }> + * @throws \OutOfBoundsException if a matched tag offset lies outside the source + */ + private function sourceMatches(File $file): array { - return self::DEFAULT_SUFFIXES; + preg_match_all( + self::CLASSNAME_PATTERN, + $this->maskNonElementMarkup($file->content), + $matches, + PREG_OFFSET_CAPTURE, + ); + + $sourceMatches = []; + foreach ($matches[0] as $i => [$fullMatch, $offset]) { + $offset = (int) $offset; + $closingOffset = $offset + (int) strrpos($fullMatch, ''); + $sourceMatches[] = [ + 'beginOffset' => $offset, + 'untilOffset' => $offset + strlen($fullMatch), + 'content' => $fullMatch, + 'text' => trim($matches[1][$i][0]), + 'affectedRanges' => [ + new SourceRange( + $file->lineAtOffset($offset)->number, + $offset + 1, + $offset + 1 + strlen(self::ELEMENT_NAME), + ), + new SourceRange( + $file->lineAtOffset($closingOffset)->number, + $closingOffset + 2, + $closingOffset + 2 + strlen(self::ELEMENT_NAME), + ), + ], + ]; + } + + return $sourceMatches; } } diff --git a/src/Sniff/Fixable.php b/src/Sniff/Fixable.php new file mode 100644 index 0000000..e2a4018 --- /dev/null +++ b/src/Sniff/Fixable.php @@ -0,0 +1,13 @@ + */ + public static function fixerClassName(): string; +} diff --git a/src/Sniff/MixedIndentationSniff.php b/src/Sniff/MixedIndentationSniff.php new file mode 100644 index 0000000..ee01c63 --- /dev/null +++ b/src/Sniff/MixedIndentationSniff.php @@ -0,0 +1,52 @@ +lines() as $line) { + if (!preg_match(self::INDENTATION_PATTERN, $line->content, $matches)) { + continue; + } + + $indentation = $matches[0]; + if (!str_contains($indentation, ' ') || !str_contains($indentation, "\t")) { + continue; + } + + $violations[] = $this->createViolation( + $file->path, + $line->number, + $line->beginOffset, + $line->beginOffset + strlen($indentation), + self::MESSAGE, + $indentation, + ); + } + + return $violations; + } +} diff --git a/src/Sniff/SimparaSniff.php b/src/Sniff/SimparaSniff.php index 667da93..f41d164 100644 --- a/src/Sniff/SimparaSniff.php +++ b/src/Sniff/SimparaSniff.php @@ -4,8 +4,16 @@ namespace DocbookCS\Sniff; -final class SimparaSniff extends AbstractSniff +use DocbookCS\Fix\Fixer\SimparaFixer; +use DocbookCS\Source\File; +use DocbookCS\Violation\SourceRange; + +final class SimparaSniff extends AbstractSniff implements Fixable { + private const string ELEMENT_NAME = 'para'; + private const string MESSAGE = ' contains only inline content and should be .'; + private const string PARA_TAG_PATTERN = '/<\/?para\b[^>]*>/'; + private const array SIMPARA_ALLOWED = [ 'abbrev', 'acronym', @@ -96,18 +104,32 @@ final class SimparaSniff extends AbstractSniff 'xref', ]; - public function getCode(): string + public static function getCode(): string { return 'DocbookCS.Simpara'; } - /** @throws \LogicException if an invalid severity level is configured */ - public function process(\DOMDocument $document, string $content, string $filePath): array + public static function fixerClassName(): string + { + return SimparaFixer::class; + } + + /** + * @throws \LogicException if an invalid severity level is configured + * @throws \OutOfBoundsException if a matched tag offset lies outside the source + */ + public function process(\DOMDocument $document, File $file): array { $violations = []; - $allowed = $this->getAllowedElements(); + $sourceMatchIndex = 0; $paras = $document->getElementsByTagName('para'); + if ($paras->length === 0) { + return []; + } + + $sourceMatches = $this->sourceMatches($file); + $allowed = $this->getAllowedElements(); /** @var \DOMElement $para */ foreach ($paras as $para) { @@ -115,6 +137,17 @@ public function process(\DOMDocument $document, string $content, string $filePat continue; } + $match = $sourceMatches[$sourceMatchIndex] ?? null; + $sourceMatchIndex++; + + if ($match === null) { + throw new \LogicException('Could not map simpara violation to source content.'); + } + + if ($match['selfClosing']) { + continue; + } + $parent = $para->parentNode; if ( $parent instanceof \DOMElement @@ -123,13 +156,19 @@ public function process(\DOMDocument $document, string $content, string $filePat continue; } - if ($this->isSimple($para, $allowed)) { - $violations[] = $this->createViolation( - $filePath, - $para->getLineNo(), - ' contains only inline content and should be .', - ); + if (!$this->isSimple($para, $allowed)) { + continue; } + + $violations[] = $this->createViolation( + $file->path, + $match['affectedRanges'][0]->line, + $match['beginOffset'], + $match['untilOffset'], + self::MESSAGE, + $match['content'], + affectedRanges: $match['affectedRanges'], + ); } return $violations; @@ -141,12 +180,14 @@ public function process(\DOMDocument $document, string $content, string $filePat private function isSimple(\DOMElement $node, array $allowed): bool { foreach ($node->childNodes as $child) { - if ($child instanceof \DOMElement) { - $name = strtolower($child->localName ?: ''); + if (!$child instanceof \DOMElement) { + continue; + } - if (!in_array($name, $allowed, true)) { - return false; - } + $name = strtolower($child->localName ?: ''); + + if (!in_array($name, $allowed, true)) { + return false; } } @@ -167,4 +208,84 @@ private function getAllowedElements(): array return array_values(array_unique(array_merge(self::SIMPARA_ALLOWED, $additional))); } + + /** + * @return list + * }> + * @throws \OutOfBoundsException if a matched tag offset lies outside the source + */ + private function sourceMatches(File $file): array + { + preg_match_all( + self::PARA_TAG_PATTERN, + $this->maskNonElementMarkup($file->content), + $matches, + PREG_OFFSET_CAPTURE, + ); + + /** @var list $stack */ + $stack = []; + $sourceMatches = []; + + foreach ($matches[0] as [$tag, $offset]) { + $offset = (int) $offset; + + if (str_ends_with(rtrim($tag), '/>')) { + $sourceMatches[] = [ + 'beginOffset' => $offset, + 'untilOffset' => $offset + strlen($tag), + 'content' => $tag, + 'selfClosing' => true, + 'affectedRanges' => [new SourceRange( + $file->lineAtOffset($offset)->number, + $offset + 1, + $offset + 1 + strlen(self::ELEMENT_NAME), + )], + ]; + continue; + } + + if (!str_starts_with($tag, ' $offset, + 'range' => new SourceRange( + $file->lineAtOffset($offset)->number, + $offset + 1, + $offset + 1 + strlen(self::ELEMENT_NAME), + ), + ]; + continue; + } + + if (null === $opening = array_pop($stack)) { + continue; + } + + $start = $opening['offset']; + $untilOffset = $offset + strlen($tag); + $sourceMatches[] = [ + 'beginOffset' => $start, + 'untilOffset' => $untilOffset, + 'content' => substr($file->content, (int)$start, $untilOffset - $start), + 'selfClosing' => false, + 'affectedRanges' => [ + $opening['range'], + new SourceRange( + $file->lineAtOffset($offset)->number, + $offset + 2, + $offset + 2 + strlen(self::ELEMENT_NAME), + ), + ], + ]; + } + + usort($sourceMatches, static fn(array $a, array $b): int => $a['beginOffset'] <=> $b['beginOffset']); + + return $sourceMatches; + } } diff --git a/src/Sniff/SniffInterface.php b/src/Sniff/SniffInterface.php index 4cc1513..b138f99 100644 --- a/src/Sniff/SniffInterface.php +++ b/src/Sniff/SniffInterface.php @@ -4,25 +4,30 @@ namespace DocbookCS\Sniff; -use DocbookCS\Report\Violation; +use DocbookCS\Runner\RunMode; +use DocbookCS\Source\File; /** - * A sniff receives a DOMDocument (already loaded) and the file path, - * then returns zero or more Violation value objects. + * A sniff receives a DOMDocument (already loaded) and its source file, + * then returns zero or more findings for reports and optional fixes. */ interface SniffInterface { + public RunMode $mode { get; } + + public function __construct(RunMode $mode); + /** * Unique, human-readable code for this sniff (e.g. "DocbookCS.MySniff"). */ - public function getCode(): string; + public static function getCode(): string; /** * Apply the sniff to the given document. * - * @return list + * @return list<\DocbookCS\Violation\Violation> */ - public function process(\DOMDocument $document, string $content, string $filePath): array; + public function process(\DOMDocument $document, File $file): array; /** * Accept a key/value property from the configuration. diff --git a/src/Sniff/TrailingWhitespaceSniff.php b/src/Sniff/TrailingWhitespaceSniff.php new file mode 100644 index 0000000..c9e4798 --- /dev/null +++ b/src/Sniff/TrailingWhitespaceSniff.php @@ -0,0 +1,50 @@ +lines() as $line) { + if (!preg_match(self::TRAILING_WHITESPACE_PATTERN, $line->content, $matches, PREG_OFFSET_CAPTURE)) { + continue; + } + + [$whitespace, $relativeOffset] = $matches[0]; + $beginOffset = $line->beginOffset + (int) $relativeOffset; + + $violations[] = $this->createViolation( + $file->path, + $line->number, + $beginOffset, + $beginOffset + strlen($whitespace), + self::MESSAGE, + $whitespace, + ); + } + + return $violations; + } +} diff --git a/src/Sniff/WhitespaceSniff.php b/src/Sniff/WhitespaceSniff.php index 6f72852..7ad8447 100644 --- a/src/Sniff/WhitespaceSniff.php +++ b/src/Sniff/WhitespaceSniff.php @@ -4,40 +4,64 @@ namespace DocbookCS\Sniff; +use DocbookCS\Fix\Fixer\WhitespaceFixer; +use DocbookCS\Source\File; + /** - * Detects whitespace and indentation issues in DocBook source files. - * - * The following violations are detected: - * - Trailing whitespace at the end of a line - * - Spaces used before tabs in indentation - * - Mixed use of tabs and spaces within indentation + * Backward-compatible aggregate of the focused whitespace rules. + * New configurations should use TrailingWhitespaceSniff and MixedIndentationSniff. */ -final class WhitespaceSniff extends AbstractSniff +final class WhitespaceSniff extends AbstractSniff implements Fixable { - public function getCode(): string + private const string LINE_ENDING_PATTERN = '/(\r\n|\n|\r)/'; + private const string WHITESPACE_PATTERN = '/([ \t]+$)|^(\t* +\t+|\t+ +\t*)|^( +)\t/'; + + public static function getCode(): string { return 'DocbookCS.Whitespace'; } - /** @throws \LogicException if an invalid severity level is configured */ - public function process(\DOMDocument $document, string $content, string $filePath): array + public static function fixerClassName(): string { - $lines = explode(PHP_EOL, $content); - $pattern = '/([ \t]+$)|^(\t* +\t+|\t+ +\t*)|^( +)\t/'; + return WhitespaceFixer::class; + } + /** @throws \LogicException if an invalid severity level is configured */ + public function process(\DOMDocument $document, File $file): array + { $violations = []; - foreach ($lines as $lineNumber => $line) { - $lineNo = $lineNumber + 1; + $offset = 0; + $line = 1; + + $lines = preg_split(self::LINE_ENDING_PATTERN, $file->content, -1, PREG_SPLIT_DELIM_CAPTURE); + if ($lines === false) { + throw new \LogicException('Could not split source content into lines.'); // @codeCoverageIgnore + } - if (preg_match($pattern, $line, $matches)) { + for ($i = 0; $i < count($lines); $i += 2) { + $lineContent = $lines[$i]; + $lineContentLength = strlen($lineContent); + $lineEnding = $lines[$i + 1] ?? ''; + + if (preg_match(self::WHITESPACE_PATTERN, $lineContent, $matches)) { $message = match (true) { !empty($matches[1]) => 'Trailing whitespace detected.', !empty($matches[2]) || !empty($matches[3]) => 'Mixed tabs and spaces in indentation.', default => 'Inconsistent indentation.', // @codeCoverageIgnore }; - $violations[] = $this->createViolation($filePath, $lineNo, $message); + $violations[] = $this->createViolation( + $file->path, + $line, + $offset, + $offset + $lineContentLength, + $message, + $lineContent, + ); } + + $offset += $lineContentLength + strlen($lineEnding); + $line++; } return $violations; diff --git a/src/Source/File.php b/src/Source/File.php new file mode 100644 index 0000000..9f66a32 --- /dev/null +++ b/src/Source/File.php @@ -0,0 +1,116 @@ +|null */ + private ?array $lineBeginOffsets = null; + + public function __construct( + public readonly string $path, + public readonly string $content, + ) { + } + + /** @return \Generator */ + public function lines(): \Generator + { + $sourceLength = strlen($this->content); + $lineBeginOffset = 0; + $lineNumber = 1; + + while (true) { + $line = $this->createLineAtOffset($lineNumber, $lineBeginOffset); + yield $line; + + if ($line->offsetAfterContent() === $sourceLength) { + return; + } + + $lineBeginOffset = $line->offsetAfterLine(); + $lineNumber++; + } + } + + /** @throws \OutOfBoundsException if the offset lies outside the source */ + public function lineAtOffset(int $offset): Line + { + $sourceLength = strlen($this->content); + if ($offset < 0 || $offset > $sourceLength) { + throw new \OutOfBoundsException( + sprintf('Source offset %d is outside the valid range 0..%d.', $offset, $sourceLength), + ); + } + + $lineBeginOffsets = $this->lineBeginOffsets(); + $low = 0; + $high = count($lineBeginOffsets) - 1; + + while ($low < $high) { + $middle = intdiv($low + $high + 1, 2); + + if ($lineBeginOffsets[$middle] <= $offset) { + $low = $middle; + } else { + $high = $middle - 1; + } + } + + return $this->createLineAtOffset($low + 1, $lineBeginOffsets[$low]); + } + + public function withContent(string $content): self + { + return $content === $this->content + ? $this + : new self($this->path, $content); + } + + /** @return non-empty-list */ + private function lineBeginOffsets(): array + { + if ($this->lineBeginOffsets !== null) { + return $this->lineBeginOffsets; + } + + $lineBeginOffsets = [0]; + + foreach ($this->lines() as $line) { + if ($line->number === 1) { + continue; + } + + $lineBeginOffsets[] = $line->beginOffset; + } + + return $this->lineBeginOffsets = $lineBeginOffsets; + } + + private function createLineAtOffset(int $lineNumber, int $lineBeginOffset): Line + { + $sourceLength = strlen($this->content); + $lineLength = strcspn($this->content, "\r\n", $lineBeginOffset); + $offsetAfterContent = $lineBeginOffset + $lineLength; + + return new Line( + number: $lineNumber, + content: substr($this->content, $lineBeginOffset, $lineLength), + lineEnding: $offsetAfterContent < $sourceLength + ? $this->lineEndingAt($offsetAfterContent) + : '', + beginOffset: $lineBeginOffset, + ); + } + + private function lineEndingAt(int $offset): string + { + if ($this->content[$offset] === "\r" && ($this->content[$offset + 1] ?? null) === "\n") { + return "\r\n"; + } + + return $this->content[$offset]; + } +} diff --git a/src/Source/Line.php b/src/Source/Line.php new file mode 100644 index 0000000..a8ad010 --- /dev/null +++ b/src/Source/Line.php @@ -0,0 +1,26 @@ +beginOffset + strlen($this->content); + } + + public function offsetAfterLine(): int + { + return $this->offsetAfterContent() + strlen($this->lineEnding); + } +} diff --git a/src/Report/Severity.php b/src/Violation/Severity.php similarity index 81% rename from src/Report/Severity.php rename to src/Violation/Severity.php index 77aae7a..e68382b 100644 --- a/src/Report/Severity.php +++ b/src/Violation/Severity.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace DocbookCS\Report; +namespace DocbookCS\Violation; enum Severity: string { diff --git a/src/Violation/SourceRange.php b/src/Violation/SourceRange.php new file mode 100644 index 0000000..8206d9d --- /dev/null +++ b/src/Violation/SourceRange.php @@ -0,0 +1,15 @@ + */ + public array $affectedRanges; + + /** + * @param list $affectedRanges + */ + public function __construct( + public string $sniffCode, + public string $filePath, + public int $line, + public int $beginOffset, + public int $untilOffset, + public string $message, + public ?string $content = null, + public Severity $severity = Severity::WARNING, + array $affectedRanges = [], + ) { + $this->affectedRanges = $affectedRanges !== [] + ? array_values($affectedRanges) + : [new SourceRange($line, $beginOffset, $untilOffset)]; + } +} diff --git a/tests/Feature/ApplicationInputTest.php b/tests/Feature/ApplicationInputTest.php new file mode 100644 index 0000000..55a5513 --- /dev/null +++ b/tests/Feature/ApplicationInputTest.php @@ -0,0 +1,99 @@ +stdout = $stdout; + $this->stderr = $stderr; + } + + #[Test] + public function itRejectsAValuelessDiffOption(): 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 itIncludesFixAndWideOptionsInHelp(): void + { + $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); + + $app->run(); + + $output = $this->readStream($this->stdout); + + self::assertStringContainsString('--fix', $output); + self::assertStringContainsString('--wide', $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..9964bb8 --- /dev/null +++ b/tests/Integration/Diff/GitDiffProviderTest.php @@ -0,0 +1,113 @@ +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 + { + $command = array_values(['git', ...$arguments]); + $result = $this->processRunner->run($command, $this->repository); + + self::assertSame(0, $result->exitCode, $result->stderr); + + return trim($result->stdout); + } +} diff --git a/tests/Integration/Fix/AttributeOrderFixerTest.php b/tests/Integration/Fix/AttributeOrderFixerTest.php new file mode 100644 index 0000000..19d22c4 --- /dev/null +++ b/tests/Integration/Fix/AttributeOrderFixerTest.php @@ -0,0 +1,95 @@ +'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new AttributeOrderSniff(RunMode::Fix)->process($document, $source); + + $beginOffset = (int)strpos($content, 'content); + self::assertSame($beginOffset, $violations[0]->beginOffset); + self::assertSame($beginOffset + strlen($sourceContent), $violations[0]->untilOffset); + self::assertSame(1, $violations[0]->line); + + $fix = new AttributeOrderFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [ $fix ]); + + self::assertSame('', $result->file->content); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itPreservesTagShapedTextInsideComments(): void + { + $content = '' + . ''; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new AttributeOrderSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + + $fix = new AttributeOrderFixer()->process($violations[0]); + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + '' + . '', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + private function createDocument(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} diff --git a/tests/Integration/Fix/ExceptionNameFixerTest.php b/tests/Integration/Fix/ExceptionNameFixerTest.php new file mode 100644 index 0000000..9744a35 --- /dev/null +++ b/tests/Integration/Fix/ExceptionNameFixerTest.php @@ -0,0 +1,151 @@ +RuntimeException'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + + $beginOffset = (int) strpos($content, ''); + $sourceContent = 'RuntimeException'; + + self::assertCount(1, $violations); + self::assertSame($sourceContent, $violations[0]->content); + self::assertSame($beginOffset, $violations[0]->beginOffset); + self::assertSame($beginOffset + strlen($sourceContent), $violations[0]->untilOffset); + self::assertSame(1, $violations[0]->line); + + $fix = new ExceptionNameFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame('RuntimeException', $result->file->content); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itPreservesClassnameAttributes(): void + { + $content = 'RuntimeException'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + self::assertSame( + 'RuntimeException', + $violations[0]->content, + ); + + $fix = new ExceptionNameFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + 'RuntimeException', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itKeepsSourceContentAlignedAfterRegularClassnames(): void + { + $content = 'RegularClassRuntimeException'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + + $sourceContent = 'RuntimeException'; + $beginOffset = (int) strpos($content, $sourceContent); + + self::assertCount(1, $violations); + self::assertSame($sourceContent, $violations[0]->content); + self::assertSame($beginOffset, $violations[0]->beginOffset); + self::assertSame($beginOffset + strlen($sourceContent), $violations[0]->untilOffset); + + $fix = new ExceptionNameFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + 'RegularClassRuntimeException', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itPreservesTagShapedTextInsideComments(): void + { + $content = '' + . 'RuntimeException'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new ExceptionNameSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + + $fix = new ExceptionNameFixer()->process($violations[0]); + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + '' + . 'RuntimeException', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + private function createDocument(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} diff --git a/tests/Integration/Fix/SimparaFixerTest.php b/tests/Integration/Fix/SimparaFixerTest.php new file mode 100644 index 0000000..23de962 --- /dev/null +++ b/tests/Integration/Fix/SimparaFixerTest.php @@ -0,0 +1,132 @@ +Text inline'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + self::assertSame('Text inline', $violations[0]->content); + + $fix = new SimparaFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame('Text inline', $result->file->content); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itPreservesParaAttributes(): void + { + $content = 'Text'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + self::assertSame('Text', $violations[0]->content); + + $fix = new SimparaFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame('Text', $result->file->content); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itCanFixNestedInnerParas(): void + { + $content = 'TextInner'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + self::assertSame('Inner', $violations[0]->content); + + $fix = new SimparaFixer()->process($violations[0]); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + 'TextInner', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itPreservesTagShapedTextInsideComments(): void + { + $content = 'Source'; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new SimparaSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(1, $violations); + + $fix = new SimparaFixer()->process($violations[0]); + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame( + 'Source', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + private function createDocument(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} diff --git a/tests/Integration/Fix/WhitespaceConcernFixersTest.php b/tests/Integration/Fix/WhitespaceConcernFixersTest.php new file mode 100644 index 0000000..247be06 --- /dev/null +++ b/tests/Integration/Fix/WhitespaceConcernFixersTest.php @@ -0,0 +1,73 @@ + \n \t \n"; + $document = new \DOMDocument(); + $document->loadXML($content); + $source = new File('file.xml', $content); + + $trailingViolations = new TrailingWhitespaceSniff(RunMode::Fix)->process($document, $source); + $indentationViolations = new MixedIndentationSniff(RunMode::Fix)->process($document, $source); + + self::assertCount(2, $trailingViolations); + self::assertCount(1, $indentationViolations); + + $fixes = []; + $trailingFixer = new TrailingWhitespaceFixer(); + foreach ($trailingViolations as $violation) { + $fixes[] = $trailingFixer->process($violation); + } + + $indentationFixer = new MixedIndentationFixer(); + foreach ($indentationViolations as $violation) { + $fixes[] = $indentationFixer->process($violation); + } + + $result = new FixApplier()->apply($source, $fixes); + + self::assertSame("\n \n", $result->file->content); + self::assertSame(3, $result->applied); + self::assertSame(0, $result->skipped); + } +} diff --git a/tests/Integration/Fix/WhitespaceFixerTest.php b/tests/Integration/Fix/WhitespaceFixerTest.php new file mode 100644 index 0000000..39c914f --- /dev/null +++ b/tests/Integration/Fix/WhitespaceFixerTest.php @@ -0,0 +1,75 @@ + \n \t\n"; + $document = $this->createDocument($content); + $source = new File('file.xml', $content); + + $violations = new WhitespaceSniff(RunMode::Fix)->process($document, $source); + + $secondLineOffset = (int) strpos($content, " \t"); + + self::assertCount(2, $violations); + self::assertSame(' ', $violations[0]->content); + self::assertSame(0, $violations[0]->beginOffset); + self::assertSame(strlen(' '), $violations[0]->untilOffset); + self::assertSame(" \t", $violations[1]->content); + self::assertSame($secondLineOffset, $violations[1]->beginOffset); + self::assertSame($secondLineOffset + strlen(" \t"), $violations[1]->untilOffset); + + $fixes = []; + $fixer = new WhitespaceFixer(); + foreach ($violations as $violation) { + $fixes[] = $fixer->process($violation); + } + + $result = new FixApplier()->apply($source, $fixes); + + self::assertSame("\n \n", $result->file->content); + self::assertSame(2, $result->applied); + } + + private function createDocument(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} diff --git a/tests/Integration/Path/DiffPathLoaderTest.php b/tests/Integration/Path/DiffPathLoaderTest.php new file mode 100644 index 0000000..0bf530d --- /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 DiffChangeset([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 DiffChangeset([ + 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..1318897 --- /dev/null +++ b/tests/Integration/Process/NativeProcessRunnerTest.php @@ -0,0 +1,36 @@ +run( + [ + PHP_BINARY, + '-r', + 'fwrite(STDOUT, "output"); fwrite(STDERR, "error"); exit(7);', + ], + getcwd() ?: '.', + ); + + self::assertSame(7, $result->exitCode); + self::assertSame('output', $result->stdout); + self::assertSame('error', $result->stderr); + } +} diff --git a/tests/Integration/Runner/FixConvergenceTest.php b/tests/Integration/Runner/FixConvergenceTest.php new file mode 100644 index 0000000..f7699eb --- /dev/null +++ b/tests/Integration/Runner/FixConvergenceTest.php @@ -0,0 +1,375 @@ +ARuntimeException'; + $filePath = $this->temporaryFile($source); + + try { + $processor = new XmlFileProcessor([ + new SimparaSniff(RunMode::Fix), + new ExceptionNameSniff(RunMode::Fix), + ]); + + $report = $this->processFile($processor, $filePath); + + self::assertSame( + 'ARuntimeException', + file_get_contents($filePath), + ); + self::assertFalse($report->hasViolations()); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itReportsRemainingViolationsAtTheirFinalLines(): void + { + $source = ''; + $filePath = $this->temporaryFile($source); + + try { + $lineBreakSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + private const string ELEMENT = ''; + + public static function getCode(): string + { + return 'Test.LineBreak'; + } + + public static function fixerClassName(): string + { + return LineBreakFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + $offset = strpos($file->content, self::ELEMENT); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + substr_count($file->content, "\n", 0, $offset) + 1, + $offset, + $offset + strlen(self::ELEMENT), + 'Replace the line-break marker.', + self::ELEMENT, + )]; + } + }; + $badElementSniff = new class (RunMode::Fix) extends AbstractSniff { + public static function getCode(): string + { + return 'Test.BadElement'; + } + + public function process(\DOMDocument $document, File $file): array + { + $element = $document->getElementsByTagName('bad')->item(0); + if (!$element instanceof \DOMElement) { + return []; + } + + $offset = strpos($file->content, ''); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + $element->getLineNo(), + $offset, + $offset + strlen(''), + 'Bad element.', + '', + )]; + } + }; + $processor = new XmlFileProcessor([ + $lineBreakSniff, + $badElementSniff, + ]); + + $report = $this->processFile($processor, $filePath); + + self::assertSame("\n", file_get_contents($filePath)); + self::assertSame(1, $report->getViolationCount()); + self::assertSame(2, $report->getViolations()[0]->line); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itKeepsChangedLineScopeAlignedAfterFixes(): void + { + $source = "\n\n"; + $filePath = $this->temporaryFile($source); + + try { + $lineBreakSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + public static function getCode(): string + { + return 'Test.ScopedLineBreak'; + } + + public static function fixerClassName(): string + { + return LineBreakFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + $element = ''; + $offset = strpos($file->content, $element); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + 2, + $offset, + $offset + strlen($element), + 'Replace the line-break marker.', + $element, + )]; + } + }; + $badElementSniff = new class (RunMode::Fix) extends AbstractSniff { + public static function getCode(): string + { + return 'Test.ScopedBadElement'; + } + + public function process(\DOMDocument $document, File $file): array + { + $element = $document->getElementsByTagName('bad')->item(0); + $offset = strpos($file->content, ''); + + if (!$element instanceof \DOMElement || $offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + $element->getLineNo(), + $offset, + $offset + strlen(''), + 'Bad element.', + '', + )]; + } + }; + $processor = new XmlFileProcessor([$lineBreakSniff, $badElementSniff]); + + $report = $this->processFile( + $processor, + $filePath, + new FileChange($filePath, [2]), + ); + + self::assertSame("\n\n\n", file_get_contents($filePath)); + self::assertSame(1, $report->getViolationCount()); + self::assertSame(3, $report->getViolations()[0]->line); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itDoesNotPersistFixesThatCycle(): void + { + $source = ''; + $filePath = $this->temporaryFile($source); + + try { + $toggleElementSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + public static function getCode(): string + { + return 'Test.ToggleElement'; + } + + public static function fixerClassName(): string + { + return ToggleElementFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + $element = str_contains($file->content, '') ? '' : ''; + $offset = strpos($file->content, $element); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + 1, + $offset, + $offset + strlen($element), + 'Toggle the element.', + $element, + )]; + } + }; + $processor = new XmlFileProcessor([ + $toggleElementSniff, + ]); + + try { + $this->processFile($processor, $filePath); + self::fail('Expected the cycling fixer to fail.'); + } catch (FixerException $exception) { + self::assertStringContainsString('did not converge', $exception->getMessage()); + } + + self::assertSame($source, file_get_contents($filePath)); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itDoesNotPersistFixesThatProduceInvalidXml(): void + { + $source = ''; + $filePath = $this->temporaryFile($source); + + try { + $invalidXmlSniff = new class (RunMode::Fix) extends AbstractSniff implements Fixable { + public static function getCode(): string + { + return 'Test.InvalidXml'; + } + + public static function fixerClassName(): string + { + return AttributeOrderFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + $offset = strpos($file->content, ''); + if ($offset === false) { + return []; + } + + return [$this->createViolation( + $file->path, + 1, + $offset, + $offset + strlen(''), + 'Produce invalid XML.', + '', + )]; + } + }; + $processor = new XmlFileProcessor([$invalidXmlSniff]); + + try { + $this->processFile($processor, $filePath); + self::fail('Expected the invalid fixer result to fail.'); + } catch (FixerException $exception) { + self::assertStringContainsString('produced invalid XML', $exception->getMessage()); + } + + self::assertSame($source, file_get_contents($filePath)); + } finally { + @unlink($filePath); + } + } + + private function processFile(XmlFileProcessor $processor, string $path, ?FileChange $fileChange = null): FileReport + { + $content = file_get_contents($path); + self::assertIsString($content); + + $result = $processor->process(new File($path, $content), $fileChange); + if ($result->isModified()) { + file_put_contents($path, $result->fixedContent()); + } + + return $result->fileReport; + } + + private function temporaryFile(string $content): string + { + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($filePath); + file_put_contents($filePath, $content); + + return $filePath; + } +} diff --git a/tests/Integration/Runner/RunCoordinatorFileFailureTest.php b/tests/Integration/Runner/RunCoordinatorFileFailureTest.php new file mode 100644 index 0000000..a3578b1 --- /dev/null +++ b/tests/Integration/Runner/RunCoordinatorFileFailureTest.php @@ -0,0 +1,107 @@ +'); + + $progress = new class ($xmlFilePath) implements ProgressInterface { + public function __construct(private string $filePath) + { + } + + public function start(int $totalFiles): void + { + @unlink($this->filePath); + } + + public function advance(int $current, string $filePath, int $violations): void + { + } + + public function finish(): void + { + } + }; + + $config = new ConfigData( + projectRoots: [], + sniffs: [], + includePaths: [$xmlFilePath], + excludePatterns: [], + entityPaths: [], + basePath: dirname($xmlFilePath), + ); + + $report = new RunCoordinator($progress)->run($this->planPaths($config)); + + self::assertTrue($report->hasViolations()); + self::assertSame('DocbookCS.Internal', $report->getAllViolations()[0]->sniffCode); + self::assertStringContainsString('Could not read file', $report->getAllViolations()[0]->message); + } + + #[Test] + public function itKeepsUnreadableFileErrorsInDiffRuns(): void + { + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($filePath); + $xmlFilePath = $filePath . '.xml'; + rename($filePath, $xmlFilePath); + file_put_contents($xmlFilePath, ''); + + $progress = $this->createMock(ProgressInterface::class); + $progress->expects($this->once())->method('start')->willReturnCallback( + static function () use ($xmlFilePath): void { + @unlink($xmlFilePath); + }, + ); + $progress->expects($this->once())->method('advance'); + $progress->expects($this->once())->method('finish'); + $config = new ConfigData( + projectRoots: [], + sniffs: [], + includePaths: [$xmlFilePath], + excludePatterns: [], + entityPaths: [], + basePath: dirname($xmlFilePath), + ); + $diff = new DiffChangeset([new FileChange($xmlFilePath, [42])]); + + $report = new RunCoordinator($progress)->run($this->planDiff($config, $diff)); + + self::assertTrue($report->hasViolations()); + self::assertSame('DocbookCS.Internal', $report->getAllViolations()[0]->sniffCode); + } + + private function planPaths(ConfigData $config): RunPlan + { + return new RunPlanner($config)->planPaths($config->getIncludePaths()); + } + + private function planDiff(ConfigData $config, DiffChangeset $diff): RunPlan + { + return new RunPlanner($config)->planDiff($diff); + } +} diff --git a/tests/Integration/Runner/RunScopeResolverTest.php b/tests/Integration/Runner/RunScopeResolverTest.php new file mode 100644 index 0000000..dbf2f28 --- /dev/null +++ b/tests/Integration/Runner/RunScopeResolverTest.php @@ -0,0 +1,158 @@ +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 + { + $resolver = $this->resolver(); + + $targets = $resolver->resolveDiff( + new DiffChangeset([new FileChange('source.xml', [2, 3])]), + ); + + self::assertSame([2, 3], $targets[$this->sourceFile]?->addedLineNumbers); + self::assertCount(1, $targets); + } + + #[Test] + public function wideScopeWidensSelectedFilesAndFollowsReferencedTargets(): void + { + $resolver = $this->resolver(wide: true); + + $targets = $resolver->resolveDiff( + new DiffChangeset([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, + ); + + $targets = $resolver->resolvePaths([$this->sourceFile]); + + self::assertSame([$this->sourceFile => null], $targets); + } + + #[Test] + public function pathScopeResolvesRelativePathsAgainstTheWorkingDirectory(): void + { + $path = 'tests/fixtures/sniff_runner/default/file_a.xml'; + $absolutePath = (getcwd() ?: '.') . '/' . $path; + + $targets = $this->resolver()->resolvePaths([$path]); + + self::assertSame([$absolutePath => null], $targets); + } + + #[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/Integration/Runner/RunScopeTest.php b/tests/Integration/Runner/RunScopeTest.php new file mode 100644 index 0000000..a34f4e5 --- /dev/null +++ b/tests/Integration/Runner/RunScopeTest.php @@ -0,0 +1,227 @@ +directory = sys_get_temp_dir() . '/docbook-cs-run-scope-' . bin2hex(random_bytes(6)); + mkdir($this->directory); + + $this->sourceFile = $this->directory . '/source.xml'; + $this->targetFile = $this->directory . '/target.xml'; + $this->entityFile = $this->directory . '/entities.ent'; + + file_put_contents($this->sourceFile, '⌖'); + 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 itExpandsReferencedTargetsOnlyWhenWideScopeIsRequested(): void + { + $config = $this->config(); + + self::assertSame(1, $this->executePaths($config, [$this->sourceFile])->getFilesScanned()); + self::assertSame( + 2, + $this->executePaths( + $config, + [$this->sourceFile], + wide: true, + )->getFilesScanned(), + ); + } + + #[Test] + public function itFixesExpandedXmlInItsTargetFileOnly(): void + { + file_put_contents($this->targetFile, 'Text'); + $config = $this->config([ + new SniffEntry(SimparaSniff::class), + ]); + + $report = $this->executePaths( + $config, + [$this->sourceFile], + mode: RunMode::Fix, + wide: true, + ); + + self::assertSame('⌖', file_get_contents($this->sourceFile)); + self::assertSame('Text', file_get_contents($this->targetFile)); + self::assertFalse($report->hasViolations()); + } + + #[Test] + public function aDiffProvidesItsOwnFilesWithoutConfiguredIncludePaths(): void + { + $config = new ConfigData( + projectRoots: [], + sniffs: [], + includePaths: [], + excludePatterns: [], + entityPaths: [], + basePath: $this->directory, + ); + + $report = $this->executeDiff( + $config, + new DiffChangeset([ + new FileChange($this->sourceFile, [1]), + ]), + ); + + self::assertSame(1, $report->getFilesScanned()); + } + + #[Test] + public function aDiffPathUsingAProjectDirectoryKeepsItsSourceRanges(): void + { + $config = new ConfigData( + projectRoots: [$this->directory => 'docs'], + sniffs: [], + includePaths: [], + excludePatterns: [], + entityPaths: [], + basePath: $this->directory, + ); + + $report = $this->executeDiff( + $config, + new DiffChangeset([ + new FileChange('docs/source.xml', [1]), + ]), + ); + + self::assertSame(1, $report->getFilesScanned()); + } + + /** @param list $sniffs */ + private function config(array $sniffs = []): ConfigData + { + return new ConfigData( + projectRoots: [], + sniffs: $sniffs, + includePaths: [$this->sourceFile], + excludePatterns: [], + entityPaths: [$this->entityFile], + basePath: $this->directory, + ); + } + + /** @param list $paths */ + private function executePaths( + ConfigData $config, + array $paths, + RunMode $mode = RunMode::Sniff, + bool $wide = false, + ): Report { + return new RunCoordinator()->run( + new RunPlanner($config, $mode, $wide)->planPaths($paths), + ); + } + + private function executeDiff( + ConfigData $config, + DiffChangeset $diff, + RunMode $mode = RunMode::Sniff, + bool $wide = false, + ): Report { + return new RunCoordinator()->run( + new RunPlanner($config, $mode, $wide)->planDiff($diff), + ); + } +} diff --git a/tests/Integration/Runner/SourceRangeScopeTest.php b/tests/Integration/Runner/SourceRangeScopeTest.php new file mode 100644 index 0000000..c637115 --- /dev/null +++ b/tests/Integration/Runner/SourceRangeScopeTest.php @@ -0,0 +1,126 @@ + + +Text + + +XML; + $expected = <<<'XML' + + +Text + + +XML; + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($filePath); + file_put_contents($filePath, $source); + + try { + $processor = new XmlFileProcessor([ + new SimparaSniff(RunMode::Fix), + ]); + + $result = $processor->process( + new File($filePath, $source), + new FileChange($filePath, [3]), + ); + file_put_contents($filePath, $result->fixedContent()); + $report = $result->fileReport; + + self::assertSame($expected, file_get_contents($filePath)); + self::assertFalse($report->hasViolations()); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itFixesAViolationCausedByADeletedLine(): void + { + $source = <<<'XML' + + +Text + + +XML; + $expected = <<<'XML' + + +Text + + +XML; + $processor = new XmlFileProcessor([ + new SimparaSniff(RunMode::Fix), + ]); + + $result = $processor->process( + new File('file.xml', $source), + new FileChange('file.xml', [], deletionAnchors: [3]), + ); + + self::assertSame($expected, $result->fixedContent()); + self::assertFalse($result->fileReport->hasViolations()); + } +} diff --git a/tests/Integration/Runner/XmlFileProcessorPipelineTest.php b/tests/Integration/Runner/XmlFileProcessorPipelineTest.php new file mode 100644 index 0000000..e34a775 --- /dev/null +++ b/tests/Integration/Runner/XmlFileProcessorPipelineTest.php @@ -0,0 +1,101 @@ +'); + + $report = $this->process( + $this->processor([new AttributeOrderSniff()]), + '', + $filePath, + ); + + self::assertCount(1, $report->getViolations()); + self::assertSame($filePath, $report->getViolations()[0]->filePath); + self::assertSame($filePath, $report->filePath); + } finally { + @unlink($filePath); + } + } + + #[Test] + public function itAppliesFixesToTheOriginalSourceWhenEntitiesExpandBeforeTheViolation(): void + { + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($filePath); + $source = '&prefix;'; + + try { + file_put_contents($filePath, $source); + + $processor = $this->processor( + [new AttributeOrderSniff(RunMode::Fix)], + new EntityPreprocessor([ + 'prefix' => 'expanded-content-before-tag', + ]), + ); + + $this->processFile($processor, $filePath); + + self::assertSame( + '&prefix;', + file_get_contents($filePath), + ); + } finally { + @unlink($filePath); + } + } + + private function process(XmlFileProcessor $processor, string $content, string $path = 'input.xml'): FileReport + { + return $processor->process(new File($path, $content))->fileReport; + } + + private function processFile(XmlFileProcessor $processor, string $path): FileReport + { + $content = file_get_contents($path); + self::assertIsString($content); + + $result = $processor->process(new File($path, $content)); + if ($result->isModified()) { + file_put_contents($path, $result->fixedContent()); + } + + return $result->fileReport; + } + + /** @param list $sniffs */ + private function processor(array $sniffs = [], ?EntityPreprocessor $pre = null): XmlFileProcessor + { + return new XmlFileProcessor( + $sniffs, + $pre ?? new EntityPreprocessor([]) // always pass array + ); + } +} diff --git a/tests/Integration/Sniff/EntityExpandedSniffTest.php b/tests/Integration/Sniff/EntityExpandedSniffTest.php index a6b1925..886a3f8 100644 --- a/tests/Integration/Sniff/EntityExpandedSniffTest.php +++ b/tests/Integration/Sniff/EntityExpandedSniffTest.php @@ -4,14 +4,18 @@ namespace DocbookCS\Tests\Integration\Sniff; -use DocbookCS\Report\Violation; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; use DocbookCS\Sniff\AbstractSniff; use DocbookCS\Sniff\ExceptionNameSniff; use DocbookCS\Sniff\SimparaSniff; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -20,35 +24,36 @@ CoversClass(EntityPreprocessor::class), CoversClass(ExceptionNameSniff::class), CoversClass(SimparaSniff::class), - CoversClass(Violation::class), + // + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), + UsesClass(Violation::class), ] final class EntityExpandedSniffTest extends TestCase { #[Test] - public function simparaIgnoresExpandedElements(): void + public function simparaIgnoresExpandedAndSelfClosingElements(): void { - $source = 'Source&expanded;'; + $source = 'Source&expanded;'; $document = $this->processedDocument($source, 'Expanded'); - $violations = new SimparaSniff()->process($document, $source, 'file.xml'); + $violations = new SimparaSniff()->process($document, new File('file.xml', $source)); self::assertCount(1, $violations); - self::assertSame(1, $violations[0]->line); + self::assertSame('Source', $violations[0]->content); } #[Test] public function exceptionNameIgnoresExpandedElements(): void { $source = 'RuntimeException&expanded;'; - $document = $this->processedDocument( - $source, - 'ExpandedException', - ); + $document = $this->processedDocument($source, 'ExpandedException'); - $violations = new ExceptionNameSniff()->process($document, $source, 'file.xml'); + $violations = new ExceptionNameSniff()->process($document, new File('file.xml', $source)); self::assertCount(1, $violations); - self::assertSame(1, $violations[0]->line); + self::assertSame('RuntimeException', $violations[0]->content); } private function processedDocument(string $source, string $expanded): \DOMDocument diff --git a/tests/Support/Fix/LineBreakFixer.php b/tests/Support/Fix/LineBreakFixer.php new file mode 100644 index 0000000..5bf2ecb --- /dev/null +++ b/tests/Support/Fix/LineBreakFixer.php @@ -0,0 +1,29 @@ +content !== '') { + throw FixerException::cannotFixInvalidContent($violation); + } + + return new Fix( + filePath: $violation->filePath, + beginOffset: $violation->beginOffset, + untilOffset: $violation->untilOffset, + replacement: "\n", + sniffCode: $violation->sniffCode, + expectedContent: $violation->content, + ); + } +} diff --git a/tests/Support/Fix/ToggleElementFixer.php b/tests/Support/Fix/ToggleElementFixer.php new file mode 100644 index 0000000..36fcad0 --- /dev/null +++ b/tests/Support/Fix/ToggleElementFixer.php @@ -0,0 +1,35 @@ +content === null) { + throw FixerException::cannotFixMissingContent(); + } + + $replacement = match ($violation->content) { + '' => '', + '' => '', + default => throw FixerException::cannotFixInvalidContent($violation), + }; + + return new Fix( + filePath: $violation->filePath, + beginOffset: $violation->beginOffset, + untilOffset: $violation->untilOffset, + replacement: $replacement, + sniffCode: $violation->sniffCode, + expectedContent: $violation->content, + ); + } +} diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 275288a..654bd48 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\DiffChangeset; use DocbookCS\Diff\DiffParser; use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; use DocbookCS\Config\SniffEntry; +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,9 +26,17 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Runner\EntityPreprocessor; -use DocbookCS\Runner\SniffRunner; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; +use DocbookCS\Runner\SourceScope; +use DocbookCS\Runner\ViolationScopeFilter; use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlProcessingResult; use DocbookCS\Sniff\ExceptionNameSniff; +use DocbookCS\Source\File; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -47,11 +59,24 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunCoordinator::class), + CoversClass(RunMode::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), - CoversClass(SniffRunner::class), CoversClass(XmlFileProcessor::class), - UsesClass(Diff::class), + // + UsesClass(DiffChangeset::class), + UsesClass(DiffPathLoader::class), + UsesClass(File::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(NativeProcessRunner::class), + UsesClass(ProcessResult::class), + UsesClass(RunScopeResolver::class), + UsesClass(SourceScope::class), + UsesClass(ViolationScopeFilter::class), + UsesClass(XmlProcessingResult::class), ] final class ApplicationTest extends TestCase { @@ -87,7 +112,7 @@ private function readStream(mixed $stream): string return stream_get_contents($stream) ?: ''; } - #[Test] + #[Test] // TODO: should be feature public function itPrintsHelpAndExitsWithZero(): void { $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); @@ -99,7 +124,7 @@ public function itPrintsHelpAndExitsWithZero(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itPrintsVersionAndExitsWithZero(): void { $app = new Application(['docbook-cs', '--version'], $this->stdout, $this->stderr); @@ -111,7 +136,7 @@ public function itPrintsVersionAndExitsWithZero(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itReturnsErrorWhenConfigCannotBeLoaded(): void { $app = new Application( @@ -126,7 +151,7 @@ public function itReturnsErrorWhenConfigCannotBeLoaded(): void self::assertStringContainsString('Error:', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itHandlesSeparateConfigArgument(): void { $app = new Application( @@ -141,7 +166,7 @@ public function itHandlesSeparateConfigArgument(): void self::assertStringContainsString('Error:', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itAcceptsPathsWithoutCrashing(): void { $app = new Application( @@ -155,7 +180,7 @@ public function itAcceptsPathsWithoutCrashing(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] + #[Test] // TODO: should be feature public function itSupportsQuietFlag(): void { $app = new Application(['docbook-cs', '--quiet'], $this->stdout, $this->stderr); @@ -165,7 +190,7 @@ public function itSupportsQuietFlag(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] + #[Test] // TODO: should be feature public function itSupportsReportFormats(): void { foreach (['console', 'json', 'checkstyle'] as $format) { @@ -181,7 +206,7 @@ public function itSupportsReportFormats(): void } } - #[Test] + #[Test] // TODO: should be feature public function itSupportsColorFlags(): void { foreach (['--colors', '--no-colors'] as $flag) { @@ -197,7 +222,7 @@ public function itSupportsColorFlags(): void } } - #[Test] + #[Test] // TODO: should be feature public function helpShortCircuitsExecution(): void { $app = new Application( @@ -213,7 +238,7 @@ public function helpShortCircuitsExecution(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function versionShortCircuitsExecution(): void { $app = new Application( @@ -229,7 +254,7 @@ public function versionShortCircuitsExecution(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itResolvesRelativeOverridePathsAgainstCwd(): void { $app = new Application( @@ -245,11 +270,11 @@ public function itResolvesRelativeOverridePathsAgainstCwd(): void self::assertNotSame(2, $exitCode); } - #[Test] + #[Test] // TODO: should be feature 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, ); @@ -260,7 +285,7 @@ public function itCatchesRuntimeErrorFromRunner(): void self::assertStringContainsString('Runtime error:', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itSupportsSeparateReportArgument(): void { $app = new Application( @@ -274,7 +299,7 @@ public function itSupportsSeparateReportArgument(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] + #[Test] // TODO: should be feature public function itPassesThroughAbsoluteOverridePaths(): void { $app = new Application( @@ -288,115 +313,7 @@ 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] + #[Test] // TODO: should be feature public function itSuppressesProgressWhenQuietFlagIsSet(): void { $app = new Application( @@ -411,7 +328,7 @@ public function itSuppressesProgressWhenQuietFlagIsSet(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] + #[Test] // TODO: should be feature public function itSuppressesProgressForStructuredReportFormats(): void { foreach (['json', 'checkstyle'] as $format) { @@ -434,7 +351,7 @@ public function itSuppressesProgressForStructuredReportFormats(): void } } - #[Test] + #[Test] // TODO: should be feature public function itShowsPerformanceWhenPerfFlagIsEnabled(): void { $app = new Application( @@ -457,7 +374,7 @@ public function itShowsPerformanceWhenPerfFlagIsEnabled(): void self::assertStringContainsString('PERFORMANCE', $output); } - #[Test] + #[Test] // TODO: should be feature public function itDoesNotShowPerformanceByDefault(): void { $app = new Application( diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index ad3e5d2..b1bd988 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -4,7 +4,7 @@ namespace DocbookCS\Tests\Unit\Diff; -use DocbookCS\Diff\Diff; +use DocbookCS\Diff\DiffChangeset; use DocbookCS\Diff\DiffParser; use DocbookCS\Diff\FileChange; use PHPUnit\Framework\Attributes\CoversClass; @@ -14,7 +14,8 @@ #[ CoversClass(DiffParser::class), - UsesClass(Diff::class), + // + UsesClass(DiffChangeset::class), UsesClass(FileChange::class), ] final class DiffParserTest extends TestCase @@ -176,6 +177,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 +216,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 { @@ -237,7 +278,7 @@ public function itHandlesHunkWithNoContext(): void // TODO: avoids test diff churn; remove when fixers merged /** @return array> */ - private function lineNumbersByFile(Diff $diff): array + private function lineNumbersByFile(DiffChangeset $diff): array { $lineNumbersByFile = []; diff --git a/tests/Unit/Fix/FixPlanTest.php b/tests/Unit/Fix/FixPlanTest.php new file mode 100644 index 0000000..490938c --- /dev/null +++ b/tests/Unit/Fix/FixPlanTest.php @@ -0,0 +1,110 @@ +x'; + $source = new File('file.xml', $content); + $plan = new FixPlan( + new Fix('file.xml', 1, 5, 'simpara', 'Sniff', 'para'), + new Fix('file.xml', 9, 13, 'simpara', 'Sniff', 'para'), + ); + + $result = new FixApplier()->apply($source, [$plan]); + + self::assertSame('x', $result->file->content); + self::assertSame(1, $result->applied); + self::assertSame(0, $result->skipped); + } + + #[Test] + public function itAllowsAnIndependentFixBetweenAPlanRanges(): void + { + $content = 'x'; + $source = new File('file.xml', $content); + $plan = new FixPlan( + new Fix('file.xml', 1, 5, 'simpara', 'ElementSniff', 'para'), + new Fix('file.xml', 9, 13, 'simpara', 'ElementSniff', 'para'), + ); + $textFix = new Fix('file.xml', 6, 7, 'y', 'TextSniff', 'x'); + + $result = new FixApplier()->apply($source, [$plan, $textFix]); + + self::assertSame('y', $result->file->content); + self::assertSame(2, $result->applied); + self::assertSame(0, $result->skipped); + } + + #[Test] + public function itSkipsAWholePlanWhenOneRangeIsStale(): void + { + $content = 'x'; + $source = new File('file.xml', $content); + $plan = new FixPlan( + new Fix('file.xml', 1, 5, 'simpara', 'Sniff', 'para'), + new Fix('file.xml', 9, 13, 'simpara', 'Sniff', 'para'), + ); + + $result = new FixApplier()->apply($source, [$plan]); + + self::assertSame($content, $result->file->content); + self::assertSame(0, $result->applied); + self::assertSame(1, $result->skipped); + } + + #[Test] + public function itSkipsAWholePlanWhenOneRangeConflicts(): void + { + $content = 'x'; + $source = new File('file.xml', $content); + $openingTagFix = new Fix('file.xml', 1, 5, 'other', 'FirstSniff', 'para'); + $plan = new FixPlan( + new Fix('file.xml', 1, 5, 'simpara', 'SecondSniff', 'para'), + new Fix('file.xml', 9, 13, 'simpara', 'SecondSniff', 'para'), + ); + + $result = new FixApplier()->apply($source, [$openingTagFix, $plan]); + + self::assertSame('x', $result->file->content); + self::assertSame(1, $result->applied); + self::assertSame(1, $result->skipped); + } + + #[Test] + public function itSkipsFixesForAnotherSource(): void + { + $content = 'x'; + $source = new File('file.xml', $content); + $fix = new Fix('other.xml', 1, 5, 'simpara', 'Sniff', 'para'); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame($content, $result->file->content); + self::assertSame(0, $result->applied); + self::assertSame(1, $result->skipped); + } +} 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..bff8881 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -4,18 +4,24 @@ namespace DocbookCS\Tests\Unit\Report; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ CoversClass(FileReport::class), + CoversClass(RelativePath::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(SourceRange::class), ] final class ReportTest extends TestCase { @@ -26,7 +32,7 @@ private function createViolation( Severity $severity = Severity::ERROR, string $filePath = 'file.xml', ): Violation { - return new Violation($sniffCode, $filePath, $line, $message, $severity); + return new Violation($sniffCode, $filePath, $line, 0, 0, $message, severity: $severity); } #[Test] @@ -68,6 +74,16 @@ public function itAddsFileReport(): void self::assertSame($fileReport, $report->getFileReports()['src/chapter.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 itKeysFileReportsByFilePath(): void { diff --git a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php index 87cbfb1..4e72cb3 100644 --- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php +++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php @@ -4,13 +4,16 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\CheckstyleReporter; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +21,9 @@ CoversClass(FileReport::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), ] final class CheckstyleReporterTest extends TestCase { @@ -34,7 +40,7 @@ private function createViolation( string $sniffCode = 'DocbookCS.Test', Severity $severity = Severity::ERROR, ): Violation { - return new Violation($sniffCode, 'filepath.xml', $line, $message, $severity); + return new Violation($sniffCode, 'filepath.xml', $line, 0, 0, $message, severity: $severity); } private function parseOutput(string $xml): \DOMDocument @@ -113,6 +119,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..4b29bf9 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -4,13 +4,16 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\ConsoleReporter; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +21,9 @@ CoversClass(FileReport::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), ] final class ConsoleReporterTest extends TestCase { @@ -35,7 +41,7 @@ private function createViolation( Severity $severity = Severity::ERROR, string $filePath = 'filepath.xml', ): Violation { - return new Violation($sniffCode, $filePath, $line, $message, $severity); + return new Violation($sniffCode, $filePath, $line, 0, 0, $message, severity: $severity); } #[Test] @@ -90,6 +96,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..dda9faf 100644 --- a/tests/Unit/Report/Reporter/JsonReporterTest.php +++ b/tests/Unit/Report/Reporter/JsonReporterTest.php @@ -4,13 +4,16 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\JsonReporter; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +21,9 @@ CoversClass(JsonReporter::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), ] final class JsonReporterTest extends TestCase { @@ -34,7 +40,7 @@ private function createViolation( string $sniffCode = 'DocbookCS.Test', Severity $severity = Severity::ERROR, ): Violation { - return new Violation($sniffCode, 'filepath.xml', $line, $message, $severity); + return new Violation($sniffCode, 'filepath.xml', $line, 0, 0, $message, severity: $severity); } #[Test] @@ -312,6 +318,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..8515f25 --- /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..3e37cda 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -6,8 +6,10 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; -use DocbookCS\Diff\Diff; +use DocbookCS\Diff\DiffChangeset; use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; @@ -15,12 +17,23 @@ use DocbookCS\Progress\ProgressInterface; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; +use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; -use DocbookCS\Runner\SniffRunner; +use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; +use DocbookCS\Runner\SourceScope; +use DocbookCS\Runner\ViolationScopeFilter; use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlProcessingResult; use DocbookCS\Sniff\SniffInterface; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -35,12 +48,26 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunCoordinator::class), + CoversClass(RunMode::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), - CoversClass(SniffRunner::class), CoversClass(Violation::class), CoversClass(XmlFileProcessor::class), - UsesClass(Diff::class), + // + UsesClass(DiffChangeset::class), + UsesClass(DiffPathLoader::class), + UsesClass(EntityExpansionMarker::class), + UsesClass(File::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(Line::class), + UsesClass(RunScopeResolver::class), + UsesClass(SourceRange::class), + UsesClass(SourceScope::class), + UsesClass(ViolationScopeFilter::class), + UsesClass(XmlProcessingResult::class), ] final class SniffRunnerTest extends TestCase { @@ -59,31 +86,34 @@ private function createConfig(array $sniffs = []): ConfigData ); } - #[Test] + #[Test] // TODO: should be integration public function itProcessesFilesWithoutViolations(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); - $report = $runner->run($config); + $runner = new RunCoordinator(); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); self::assertCount(0, $report->getFileReports()); } - #[Test] + #[Test] // TODO: should be integration public function itUsesOverridePathsWhenProvided(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); - $report = $runner->run($config, [self::FIXTURE_DIR . '/../override']); + $runner = new RunCoordinator(); + $report = $runner->run($this->planPaths( + $config, + [self::FIXTURE_DIR . '/../override'], + )); self::assertSame(1, $report->getFilesScanned()); } - #[Test] + #[Test] // TODO: should be integration public function itCallsProgressMethods(): void { $progress = $this->createMock(ProgressInterface::class); @@ -100,26 +130,32 @@ public function itCallsProgressMethods(): void $config = $this->createConfig(); - $runner = new SniffRunner($progress); - $runner->run($config); + $runner = new RunCoordinator($progress); + $runner->run($this->planPaths($config)); } - #[Test] + #[Test] // TODO: should be integration public function itAddsFileReportsForFilesWithViolations(): void { - $sniff = new class implements SniffInterface { - public function getCode(): string + $sniff = new class (RunMode::Sniff) implements SniffInterface { + public function __construct(public RunMode $mode) + { + } + + public static function getCode(): string { return 'Test.ViolatingSniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return [ new Violation( sniffCode: 'Test.ViolatingSniff', - filePath: $filePath, + filePath: $file->path, line: 1, + beginOffset: 0, + untilOffset: 0, message: 'Test violation message', severity: Severity::WARNING, ), @@ -133,30 +169,36 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); - $runner = new SniffRunner(); - $report = $runner->run($config); + $runner = new RunCoordinator(); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertCount(2, $report->getFileReports()); self::assertTrue($report->hasViolations()); } - #[Test] - public function itStoresRelativePathsInFileReports(): void + #[Test] // TODO: should be integration + public function itStoresAbsolutePathsInFileReports(): void { - $sniff = new class implements SniffInterface { - public function getCode(): string + $sniff = new class (RunMode::Sniff) implements SniffInterface { + public function __construct(public RunMode $mode) + { + } + + public static function getCode(): string { return 'Test.ViolatingSniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return [ new Violation( sniffCode: 'Test.ViolatingSniff', - filePath: $filePath, + filePath: $file->path, line: 1, + beginOffset: 0, + untilOffset: 0, message: 'Test violation', severity: Severity::WARNING, ), @@ -170,34 +212,40 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); - $runner = new SniffRunner(); - $report = $runner->run($config); + $runner = new RunCoordinator(); + $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, ); } } - #[Test] + #[Test] // TODO: should be integration public function itPassesPropertiesToSniffs(): void { - $sniffClass = new class implements SniffInterface { + $sniffClass = new class (RunMode::Sniff) implements SniffInterface { public static string $captured = ''; + public static RunMode $capturedMode = RunMode::Sniff; + + public function __construct(public RunMode $mode) + { + self::$capturedMode = $mode; + } public function setProperty(string $name, string $value): void { self::$captured = $value; } - public function getCode(): string + public static function getCode(): string { return 'Test.ConfigurableSniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return []; } @@ -205,103 +253,155 @@ 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 = new RunCoordinator(); + $runner->run($this->planPaths($config, mode: RunMode::Fix)); self::assertSame('someValue', $sniffClass::$captured); + self::assertSame(RunMode::Fix, $sniffClass::$capturedMode); } - #[Test] + #[Test] // TODO: should be integration public function itThrowsWhenSniffClassDoesNotExist(): void { $config = $this->createConfig(sniffs: [new SniffEntry('NonExistent\\FakeSniff')]); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('does not exist'); + $this->expectExceptionMessageIsOrContains('does not exist'); - $runner->run($config); + $runner->run($this->planPaths($config)); } - #[Test] + #[Test] // TODO: should be integration public function itThrowsWhenClassDoesNotImplementSniffInterface(): void { $config = $this->createConfig(sniffs: [new SniffEntry(\stdClass::class)]); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('does not implement'); + $this->expectExceptionMessageIsOrContains('does not implement'); - $runner->run($config); + $runner->run($this->planPaths($config)); } - #[Test] + #[Test] // TODO: should be integration public function itFiltersFilesToOnlyThoseInTheDiff(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [1])]); - $report = $runner->run($config, null, $diff); + $diff = new DiffChangeset([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [1])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } - #[Test] + #[Test] // TODO: should be integration public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); - $diff = new Diff([new FileChange('completely/different/file.xml', [1, 2, 3])]); - $report = $runner->run($config, null, $diff); + $diff = new DiffChangeset([new FileChange('completely/different/file.xml', [1, 2, 3])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(0, $report->getFilesScanned()); } - #[Test] + #[Test] // TODO: should be integration public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); $discoveredPath = self::FIXTURE_DIR . '/file_a.xml'; - $diff = new Diff([new FileChange($discoveredPath, [1])]); - $report = $runner->run($config, null, $diff); + $diff = new DiffChangeset([new FileChange($discoveredPath, [1])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } - #[Test] + #[Test] // TODO: should be integration public function itScansAllFilesWhenNoDiffIsGiven(): void { $config = $this->createConfig(); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); } - #[Test] + #[Test] // TODO: should be integration + 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( + mode: RunMode::Sniff, + sniffs: [], + targets: $resolver->resolvePaths([$directory . '/.']), + entities: [ + 'bridge' => '⌖', + 'target' => '', + ], + ); + + $report = new RunCoordinator()->run($plan); + + self::assertSame(2, $report->getFilesScanned()); + } finally { + @unlink($sourceFile); + @unlink($targetFile); + @unlink($entityFile); + @rmdir($directory); + } + } + + #[Test] // TODO: should be integration public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { - $sniff = new class implements SniffInterface { - public function getCode(): string + $sniff = new class (RunMode::Sniff) implements SniffInterface { + public function __construct(public RunMode $mode) + { + } + + public static function getCode(): string { return 'Test.ViolatingSniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return [ new Violation( sniffCode: 'Test.ViolatingSniff', - filePath: $filePath, + filePath: $file->path, line: 1, + beginOffset: 0, + untilOffset: 0, message: 'Test violation', severity: Severity::WARNING, ), @@ -314,12 +414,23 @@ public function setProperty(string $name, string $value): void }; $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [])]); - $report = $runner->run($config, null, $diff); + $diff = new DiffChangeset([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, RunMode $mode = RunMode::Sniff): RunPlan + { + return new RunPlanner($config, $mode)->planPaths($paths ?? $config->getIncludePaths()); + } + + private function planDiff(ConfigData $config, DiffChangeset $diff, RunMode $mode = RunMode::Sniff): RunPlan + { + return new RunPlanner($config, $mode)->planDiff($diff); + } } diff --git a/tests/Unit/Runner/SourceScopeTest.php b/tests/Unit/Runner/SourceScopeTest.php new file mode 100644 index 0000000..27f5328 --- /dev/null +++ b/tests/Unit/Runner/SourceScopeTest.php @@ -0,0 +1,140 @@ +isWholeFile()); + self::assertSame([2], $scope->lineNumbers($file)); + self::assertTrue($scope->includes($this->violation(4, 7, 2))); + self::assertFalse($scope->includes($this->violation(8, 13, 3))); + } + + #[Test] + public function wholeFileScopeIncludesEveryLine(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = SourceScope::wholeFile(); + + self::assertTrue($scope->isWholeFile()); + self::assertSame([1, 2, 3], $scope->lineNumbers($file)); + } + + #[Test] + public function anEmptyChangedLineSetSelectsNoLines(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = SourceScope::fromFileChange($file, new FileChange('file.xml', [])); + + self::assertSame([], $scope->lineNumbers($file)); + } + + #[Test] + public function itKeepsScopeAlignedAfterAnInsertionBeforeIt(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = SourceScope::fromFileChange($file, new FileChange('file.xml', [2])); + $scope = $scope->after([ + new Fix('file.xml', 0, 0, "x\n", 'Test'), + ]); + $file = $file->withContent("x\none\ntwo\nthree"); + + self::assertSame([3], $scope->lineNumbers($file)); + self::assertFalse($scope->includes($this->violation(4, 5, 2))); + self::assertTrue($scope->includes($this->violation(6, 9, 3))); + self::assertFalse($scope->includes($this->violation(10, 15, 4))); + } + + #[Test] + public function itIncludesContentInsertedIntoAnEmptySelectedLine(): void + { + $file = new File('file.xml', "root\n"); + $scope = SourceScope::fromFileChange($file, new FileChange('file.xml', [2])); + $scope = $scope->after([ + new Fix('file.xml', 5, 5, 'value', 'Test'), + ]); + + self::assertTrue($scope->includes($this->violation(5, 10, 2))); + } + + #[Test] + public function itSortsFixesBeforeMappingScopeOffsets(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = SourceScope::fromFileChange($file, new FileChange('file.xml', [2])); + $scope = $scope->after([ + new Fix('file.xml', 13, 13, "\nfour", 'Test'), + new Fix('file.xml', 0, 0, "zero\n", 'Test'), + ]); + $file = $file->withContent("zero\none\ntwo\nthree\nfour"); + + self::assertSame([3], $scope->lineNumbers($file)); + } + + #[Test] + public function itIncludesAViolationContainingADeletionLocation(): void + { + $file = new File('file.xml', "\n\nText\n\n"); + $scope = SourceScope::fromFileChange( + $file, + new FileChange('file.xml', [], deletionAnchors: [3]), + ); + + self::assertTrue($scope->includes($this->violation(7, 27, 2))); + self::assertFalse($scope->includes($this->violation(0, 6, 1))); + } + + #[Test] + public function itAnchorsADeletionAtTheEndOfTheFile(): void + { + $file = new File('file.xml', "\n"); + $scope = SourceScope::fromFileChange( + $file, + new FileChange('file.xml', [], deletionAnchors: [2]), + ); + + self::assertTrue($scope->includes($this->violation(0, strlen($file->content) + 1, 1))); + } + + private function violation(int $beginOffset, int $untilOffset, int $line): Violation + { + return new Violation( + sniffCode: 'Test', + filePath: 'file.xml', + line: $line, + beginOffset: $beginOffset, + untilOffset: $untilOffset, + message: 'Test violation.', + ); + } +} diff --git a/tests/Unit/Runner/ViolationScopeFilterTest.php b/tests/Unit/Runner/ViolationScopeFilterTest.php new file mode 100644 index 0000000..5a42559 --- /dev/null +++ b/tests/Unit/Runner/ViolationScopeFilterTest.php @@ -0,0 +1,51 @@ +'); + $document = new \DOMDocument(); + self::assertTrue($document->loadXML($file->content)); + $violations = [new Violation( + sniffCode: 'Test.Stub', + filePath: $file->path, + line: 1, + beginOffset: 0, + untilOffset: 0, + message: 'violation at line 1', + )]; + + $result = new ViolationScopeFilter()->filter( + $violations, + $document, + $file, + SourceScope::wholeFile(), + ); + + self::assertSame($violations, $result); + } +} diff --git a/tests/Unit/Runner/XmlFileProcessorTest.php b/tests/Unit/Runner/XmlFileProcessorTest.php index 901835a..0eb1d7a 100644 --- a/tests/Unit/Runner/XmlFileProcessorTest.php +++ b/tests/Unit/Runner/XmlFileProcessorTest.php @@ -4,15 +4,27 @@ namespace DocbookCS\Tests\Unit\Runner; +use DocbookCS\Diff\FileChange; +use DocbookCS\Fix\Fixer\AttributeOrderFixer; +use DocbookCS\Fix\FixerException; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\SourceScope; +use DocbookCS\Runner\ViolationScopeFilter; use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlProcessingResult; +use DocbookCS\Sniff\Fixable; use DocbookCS\Sniff\SniffInterface; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\Severity; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -20,36 +32,49 @@ CoversClass(FileReport::class), CoversClass(Report::class), CoversClass(Violation::class), + CoversClass(ViolationScopeFilter::class), CoversClass(XmlFileProcessor::class), + // + UsesClass(AttributeOrderFixer::class), + UsesClass(File::class), + UsesClass(FileChange::class), + UsesClass(FixerException::class), + UsesClass(Line::class), + UsesClass(RunMode::class), + UsesClass(SourceRange::class), + UsesClass(SourceScope::class), + UsesClass(XmlProcessingResult::class), ] final class XmlFileProcessorTest extends TestCase { #[Test] public function itReportsParseErrors(): void { - $report = $this->processor()->processString('', 'bad.xml'); + $report = $this->process($this->processor(), '', 'bad.xml'); $this->assertInternalError($report, 'XML parse error'); } #[Test] - public function itReportsMissingFiles(): void + public function itReportsParseErrorsOutsideChangedSourceRanges(): void { - $report = $this->processor()->processFile('/nonexistent/path/file.xml'); + $report = $this->process( + $this->processor(), + '', + 'bad.xml', + new FileChange('bad.xml', [99]), + ); - $this->assertInternalError($report, 'Could not read file'); + $this->assertInternalError($report, 'XML parse error'); } #[Test] - public function itPrefersReportPathOverFilePath(): void + public function itStoresTheProvidedFilePathInFileReports(): void { - $report = $this->processor()->processFile( - '/nonexistent/path/file.xml', - [], - 'relative/file.xml' - ); + $filePath = (getcwd() ?: '') . '/nonexistent/path/file.xml'; + $report = $this->process($this->processor(), '', $filePath); - self::assertSame('relative/file.xml', $report->filePath); + self::assertSame($filePath, $report->filePath); } #[Test] @@ -57,12 +82,12 @@ public function itAcceptsValidXmlWithoutViolations(): void { $xml = $this->xml('ok'); - $report = $this->processor()->processString($xml); + $report = $this->process($this->processor(), $xml); self::assertFalse($report->hasViolations()); } - #[Test] + #[Test] // TODO: should be integration public function itHandlesEntitiesWithoutParseErrors(): void { $xml = $this->xml( @@ -77,7 +102,7 @@ public function itHandlesEntitiesWithoutParseErrors(): void 'php.ini' => '', ])); - $report = $processor->processString($xml); + $report = $this->process($processor, $xml); self::assertCount( 0, @@ -88,7 +113,7 @@ public function itHandlesEntitiesWithoutParseErrors(): void ); } - #[Test] + #[Test] // TODO: should be integration public function itUsesCustomPreprocessor(): void { $processor = $this->processor([], new EntityPreprocessor([ @@ -97,7 +122,7 @@ public function itUsesCustomPreprocessor(): void $xml = $this->xml('&custom.entity;'); - $report = $processor->processString($xml); + $report = $this->process($processor, $xml); self::assertCount( 0, @@ -113,7 +138,7 @@ public function itReturnsZeroViolationsWithoutSniffs(): void { $xml = $this->xml('Hello'); - $report = $this->processor()->processString($xml); + $report = $this->process($this->processor(), $xml); self::assertSame(0, $report->getViolationCount()); } @@ -131,7 +156,7 @@ public function itReturnsAllViolationsWithoutDiffFiltering(): void ' ); - $report = $this->processor([$sniff])->processString($xml); + $report = $this->process($this->processor([$sniff]), $xml); self::assertSame(2, $report->getViolationCount()); } @@ -149,7 +174,12 @@ public function itFiltersViolationsByChangedLines(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'f.xml', [3]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'f.xml', + new FileChange('f.xml', [3]), + ); self::assertSame(1, $report->getViolationCount()); self::assertSame(3, $report->getViolations()[0]->line); @@ -170,7 +200,12 @@ public function itExpandsElementSpanForNestedChanges(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [6]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [6]), + ); self::assertSame(1, $report->getViolationCount()); } @@ -186,7 +221,12 @@ public function itDropsViolationsWhoseLineHasNoElement(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [3]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [3]), + ); self::assertSame(0, $report->getViolationCount()); } @@ -204,7 +244,12 @@ public function itMatchesChangesInElementOwnTextContent(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [4]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [4]), + ); self::assertSame(1, $report->getViolationCount()); } @@ -223,7 +268,12 @@ public function itBoundsChildSpanByNextSibling(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [4]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [4]), + ); self::assertSame(1, $report->getViolationCount()); } @@ -245,7 +295,12 @@ public function itIgnoresChangesInNonDirectDescendants(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [6]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [6]), + ); self::assertSame(0, $report->getViolationCount()); } @@ -262,20 +317,16 @@ public function itIgnoresChangesOutsideElementSpan(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'x.xml', [7]); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'x.xml', + new FileChange('x.xml', [7]), + ); self::assertSame(0, $report->getViolationCount()); } - #[Test] - public function itKeepsInternalErrorsEvenWithDiffFiltering(): void - { - $report = $this->processor()->processFile('/nonexistent/path/file.xml', [42]); - - self::assertTrue($report->hasViolations()); - self::assertSame('DocbookCS.Internal', $report->getViolations()[0]->sniffCode); - } - #[Test] public function itReportsNoViolationsInDiffModeWhenNoLinesWereAdded(): void { @@ -289,32 +340,124 @@ public function itReportsNoViolationsInDiffModeWhenNoLinesWereAdded(): void ' ); - $report = $this->processor([$sniff])->processString($xml, 'f.xml', []); + $report = $this->process( + $this->processor([$sniff]), + $xml, + 'f.xml', + new FileChange('f.xml', []), + ); self::assertSame(0, $report->getViolationCount()); } + #[Test] + public function itDoesNotFixViolationsFromNonFixableSniffs(): void + { + $sniff = new class (RunMode::Sniff) implements SniffInterface { + public function __construct(public RunMode $mode) + { + } + + public static function getCode(): string + { + return 'Test.NonFixable'; + } + + public function process(\DOMDocument $document, File $file): array + { + return [ + new Violation( + sniffCode: self::getCode(), + filePath: $file->path, + line: 2, + beginOffset: 0, + untilOffset: 7, + message: 'Reported only.', + content: '', + severity: Severity::ERROR, + ), + ]; + } + + public function setProperty(string $name, string $value): void + { + } + }; + + $report = $this->process($this->processor([$sniff]), $this->xml('')); + + self::assertSame(1, $report->getViolationCount()); + } + + #[Test] + public function itThrowsWhenFixableSniffReportsViolationWithoutContentInFixMode(): void + { + $sniff = new class (RunMode::Fix) implements Fixable { + public function __construct(public RunMode $mode) + { + } + + public static function getCode(): string + { + return 'Test.BrokenFixable'; + } + + public static function fixerClassName(): string + { + return AttributeOrderFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + return [ + new Violation( + sniffCode: self::getCode(), + filePath: $file->path, + line: 1, + beginOffset: 0, + untilOffset: 7, + message: 'Missing source content.', + severity: Severity::ERROR, + ), + ]; + } + + public function setProperty(string $name, string $value): void + { + } + }; + + $this->expectException(FixerException::class); + $this->expectExceptionMessageIsOrContains('Violations cannot be content-less when passed to a fixer.'); + + $this->process($this->processor([$sniff]), $this->xml('')); + } + /** @param list $lines */ private function sniff(array $lines): SniffInterface { - return new class ($lines) implements SniffInterface { - /** @param list $lines */ - public function __construct(private readonly array $lines) + $sniff = new class (RunMode::Sniff) implements SniffInterface { + /** @var list */ + public array $lines = []; + + public function __construct(public RunMode $mode) { } - public function getCode(): string + public static function getCode(): string { return 'Test.Stub'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return array_map( fn(int $line) => new Violation( - sniffCode: $this->getCode(), - filePath: $filePath, + sniffCode: self::getCode(), + filePath: $file->path, line: $line, + beginOffset: 0, + untilOffset: 0, message: "violation at line {$line}", severity: Severity::WARNING ), @@ -326,6 +469,19 @@ public function setProperty(string $name, string $value): void { } }; + + $sniff->lines = $lines; + + return $sniff; + } + + private function process( + XmlFileProcessor $processor, + string $content, + string $path = 'input.xml', + ?FileChange $fileChange = null, + ): FileReport { + return $processor->process(new File($path, $content), $fileChange)->fileReport; } /** @param list $sniffs */ diff --git a/tests/Unit/Runner/XmlProcessingResultTest.php b/tests/Unit/Runner/XmlProcessingResultTest.php new file mode 100644 index 0000000..e06f8e0 --- /dev/null +++ b/tests/Unit/Runner/XmlProcessingResultTest.php @@ -0,0 +1,73 @@ +'), + new File('input.xml', ''), + ); + + self::assertFalse($result->isModified()); + } + + #[Test] + public function itHasPendingFixesForModifiedContent(): void + { + $result = new XmlProcessingResult( + new FileReport('input.xml'), + new File('input.xml', ''), + new File('input.xml', ''), + ); + + self::assertTrue($result->isModified()); + } + + #[Test] + public function itThrowsWhenReadingFixedContentWithoutFixApplication(): void + { + $result = new XmlProcessingResult( + new FileReport('input.xml'), + new File('input.xml', ''), + new File('input.xml', ''), + ); + + $this->expectException(FixerException::class); + $this->expectExceptionMessageIsOrContains('Cannot read fixed content when no fix application was attempted.'); + + $result->fixedContent(); + } + + #[Test] + public function itReturnsFixedContentWhenFixApplicationExists(): void + { + $result = new XmlProcessingResult( + new FileReport('input.xml'), + new File('input.xml', ''), + new File('input.xml', ''), + ); + + self::assertSame('', $result->fixedContent()); + } +} diff --git a/tests/Unit/Sniff/AbstractSniffTest.php b/tests/Unit/Sniff/AbstractSniffTest.php index 8c9c5a4..0f8e834 100644 --- a/tests/Unit/Sniff/AbstractSniffTest.php +++ b/tests/Unit/Sniff/AbstractSniffTest.php @@ -5,6 +5,7 @@ namespace DocbookCS\Tests\Unit\Sniff; use DocbookCS\Sniff\AbstractSniff; +use DocbookCS\Source\File; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -17,12 +18,12 @@ final class AbstractSniffTest extends TestCase private function createSniff(): AbstractSniff { return new class extends AbstractSniff { - public function getCode(): string + public static function getCode(): string { return 'test.sniff'; } - public function process(\DOMDocument $document, string $content, string $filePath): array + public function process(\DOMDocument $document, File $file): array { return []; } diff --git a/tests/Unit/Sniff/AffectedRangesTest.php b/tests/Unit/Sniff/AffectedRangesTest.php new file mode 100644 index 0000000..c3c1bef --- /dev/null +++ b/tests/Unit/Sniff/AffectedRangesTest.php @@ -0,0 +1,71 @@ +\nText\n"; + $violation = new SimparaSniff()->process( + $this->document($content), + new File('file.xml', $content), + )[0]; + + self::assertSame('Text', $violation->content); + self::assertSame((int) strpos($content, ''), $violation->beginOffset); + self::assertEquals([ + new SourceRange(2, 8, 12), + new SourceRange(2, 19, 23), + ], $violation->affectedRanges); + } + + #[Test] + public function exceptionNameIdentifiesElementNamesOnDifferentLines(): void + { + $content = "\nRuntimeException\n\n"; + $violation = new ExceptionNameSniff()->process( + $this->document($content), + new File('file.xml', $content), + )[0]; + + self::assertSame("RuntimeException\n", $violation->content); + self::assertEquals([ + new SourceRange(2, 8, 17), + new SourceRange(3, 37, 46), + ], $violation->affectedRanges); + } + + private function document(string $content): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($content); + + return $document; + } +} diff --git a/tests/Unit/Sniff/AttributeOrderSniffTest.php b/tests/Unit/Sniff/AttributeOrderSniffTest.php index da2e993..173fe44 100644 --- a/tests/Unit/Sniff/AttributeOrderSniffTest.php +++ b/tests/Unit/Sniff/AttributeOrderSniffTest.php @@ -4,15 +4,23 @@ namespace DocbookCS\Tests\Unit\Sniff; -use DocbookCS\Report\Violation; use DocbookCS\Sniff\AttributeOrderSniff; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ CoversClass(AttributeOrderSniff::class), CoversClass(Violation::class), + // + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), ] final class AttributeOrderSniffTest extends TestCase { @@ -34,7 +42,7 @@ public function itIgnoresElementsWithoutRelevantAttributes(): void self::assertSame( [], - $sniff->process($doc, $content, 'test.xml') + $sniff->process($doc, new File('test.xml', $content)) ); } @@ -48,7 +56,7 @@ public function itDoesNotFlagWhenXmlIdComesFirst(): void self::assertSame( [], - $sniff->process($doc, $content, 'test.xml') + $sniff->process($doc, new File('test.xml', $content)) ); } @@ -60,7 +68,7 @@ public function itFlagsWhenXmlIdComesAfterXmlns(): void $content = ''; $doc = $this->createDocument(''); - $violations = $sniff->process($doc, $content, 'file.xml'); + $violations = $sniff->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertStringContainsString( @@ -79,7 +87,7 @@ public function itHandlesXmlnsPrefixedAttributes(): void self::assertCount( 1, - $sniff->process($doc, $content, 'file.xml') + $sniff->process($doc, new File('file.xml', $content)) ); } @@ -97,7 +105,7 @@ public function itHandlesMultipleElements(): void $doc = $this->createDocument(''); - $violations = $sniff->process($doc, $content, 'file.xml'); + $violations = $sniff->process($doc, new File('file.xml', $content)); self::assertCount(2, $violations); } @@ -113,7 +121,7 @@ public function itReportsCorrectLineNumber(): void $doc = $this->createDocument(''); - $violations = $sniff->process($doc, $content, 'file.xml'); + $violations = $sniff->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertSame(2, $violations[0]->line); @@ -128,7 +136,37 @@ public function itReturnsEmptyWhenContentIsEmpty(): void self::assertSame( [], - $sniff->process($doc, '', 'file.xml') + $sniff->process($doc, new File('file.xml', '')) ); } + + #[Test] + public function itAddsSourceContent(): void + { + $sniff = new AttributeOrderSniff(); + + $content = ''; + $doc = $this->createDocument(''); + + $violations = $sniff->process($doc, new File('file.xml', $content)); + + self::assertCount(1, $violations); + self::assertSame('', $violations[0]->content); + self::assertSame(0, $violations[0]->beginOffset); + self::assertSame(38, $violations[0]->untilOffset); + self::assertSame(1, $violations[0]->line); + } + + #[Test] + public function itIgnoresElementsInsideComments(): void + { + $content = '' + . ''; + $doc = $this->createDocument($content); + + $violations = new AttributeOrderSniff()->process($doc, new File('file.xml', $content)); + + self::assertCount(1, $violations); + self::assertSame('', $violations[0]->content); + } } diff --git a/tests/Unit/Sniff/ExceptionNameSniffTest.php b/tests/Unit/Sniff/ExceptionNameSniffTest.php index 5e749c6..ab3abf4 100644 --- a/tests/Unit/Sniff/ExceptionNameSniffTest.php +++ b/tests/Unit/Sniff/ExceptionNameSniffTest.php @@ -4,9 +4,12 @@ namespace DocbookCS\Tests\Unit\Sniff; -use DocbookCS\Report\Violation; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Sniff\ExceptionNameSniff; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -17,6 +20,9 @@ CoversClass(Violation::class), // UsesClass(EntityExpansionMarker::class), + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), ] final class ExceptionNameSniffTest extends TestCase { @@ -33,7 +39,7 @@ public function itReturnsEmptyWhenNoClassnameNodes(): void { $content = ''; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'test.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('test.xml', $content)); self::assertSame([], $violations); } @@ -43,7 +49,7 @@ public function itIgnoresEmptyClassname(): void { $content = ' '; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'test.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('test.xml', $content)); self::assertSame([], $violations); } @@ -53,7 +59,7 @@ public function itDoesNotFlagRegularClassnames(): void { $content = 'MyService'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'test.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('test.xml', $content)); self::assertSame([], $violations); } @@ -63,7 +69,7 @@ public function itFlagsExceptionSuffix(): void { $content = 'RuntimeException'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertStringContainsString( @@ -77,7 +83,7 @@ public function itFlagsErrorSuffix(): void { $content = 'TypeError'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertStringContainsString( @@ -91,7 +97,7 @@ public function itFlagsThrowableSuffix(): void { $content = 'CustomThrowable'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertStringContainsString( @@ -105,7 +111,7 @@ public function itHandlesNamespacedClassnames(): void { $content = 'Foo\Bar\BazException'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertStringContainsString( @@ -119,7 +125,7 @@ public function itOnlyChecksBaseNameInNamespace(): void { $content = 'Exception\ButNotActually'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertSame([], $violations); } @@ -135,7 +141,7 @@ public function itHandlesMultipleClassnames(): void '; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertCount(2, $violations); } @@ -145,7 +151,7 @@ public function itDoesNotFlagClassnameInsideOoclass(): void { $content = 'RuntimeException'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); self::assertSame([], $violations); } @@ -155,9 +161,57 @@ public function itIncludesFilePathInViolation(): void { $content = 'RuntimeException'; $doc = $this->createDocument($content); - $violations = new ExceptionNameSniff()->process($doc, $content, 'my-file.xml'); + $violations = new ExceptionNameSniff()->process($doc, new File('my-file.xml', $content)); self::assertCount(1, $violations); self::assertSame('my-file.xml', $violations[0]->filePath); } + + #[Test] + public function itAddsSourceContent(): void + { + $content = 'RuntimeException'; + $doc = $this->createDocument($content); + + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); + + $beginOffset = (int) strpos($content, ''); + $sourceContent = 'RuntimeException'; + + self::assertCount(1, $violations); + self::assertSame($sourceContent, $violations[0]->content); + self::assertSame($beginOffset, $violations[0]->beginOffset); + self::assertSame($beginOffset + strlen($sourceContent), $violations[0]->untilOffset); + self::assertSame(1, $violations[0]->line); + } + + #[Test] + public function itKeepsSourceContentAlignedAfterRegularClassnames(): void + { + $content = 'RegularClassRuntimeException'; + $doc = $this->createDocument($content); + + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); + + $sourceContent = 'RuntimeException'; + $beginOffset = (int) strpos($content, $sourceContent); + + self::assertCount(1, $violations); + self::assertSame($sourceContent, $violations[0]->content); + self::assertSame($beginOffset, $violations[0]->beginOffset); + self::assertSame($beginOffset + strlen($sourceContent), $violations[0]->untilOffset); + } + + #[Test] + public function itIgnoresClassnamesInsideCommentsWhenMappingSource(): void + { + $content = '' + . 'RuntimeException'; + $doc = $this->createDocument($content); + + $violations = new ExceptionNameSniff()->process($doc, new File('file.xml', $content)); + + self::assertCount(1, $violations); + self::assertSame('RuntimeException', $violations[0]->content); + } } diff --git a/tests/Unit/Sniff/SimparaSniffTest.php b/tests/Unit/Sniff/SimparaSniffTest.php index 036d374..5542d00 100644 --- a/tests/Unit/Sniff/SimparaSniffTest.php +++ b/tests/Unit/Sniff/SimparaSniffTest.php @@ -4,9 +4,12 @@ namespace DocbookCS\Tests\Unit\Sniff; -use DocbookCS\Report\Violation; use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Sniff\SimparaSniff; +use DocbookCS\Source\File; +use DocbookCS\Source\Line; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -17,6 +20,9 @@ CoversClass(Violation::class), // UsesClass(EntityExpansionMarker::class), + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), ] final class SimparaSniffTest extends TestCase { @@ -31,29 +37,36 @@ private function createDocument(string $xml): \DOMDocument #[Test] public function itFlagsPlainTextPara(): void { - $doc = $this->createDocument('Text'); + $doc = $this->createDocument($content = + 'Text' + ); + + $violations = new SimparaSniff()->process($doc, new File('file.xml', $content)); - self::assertCount(1, new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertCount(1, $violations); + self::assertSame('Text', $violations[0]->content); + self::assertSame((int) strpos($content, ''), $violations[0]->beginOffset); + self::assertSame((int) strpos($content, '') + strlen(''), $violations[0]->untilOffset); } #[Test] public function itFlagsAllowedInlineElements(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = 'Text inline' ); - self::assertCount(1, new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertCount(1, new SimparaSniff()->process($doc, new File('file.xml', $content))); } #[Test] public function itDoesNotFlagUnknownElement(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = '' ); - self::assertSame([], new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } #[Test] @@ -61,7 +74,7 @@ public function itHandlesMultipleParas(): void { $sniff = new SimparaSniff(); - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Inline @@ -69,7 +82,7 @@ public function itHandlesMultipleParas(): void ' ); - $violations = $sniff->process($doc, '', 'file.xml'); + $violations = $sniff->process($doc, new File('file.xml', $content)); self::assertCount(2, $violations); } @@ -80,41 +93,62 @@ public function itSupportsAdditionalInlineElements(): void $sniff = new SimparaSniff(); $sniff->setProperty('additionalInlineElements', 'custom'); - $doc = $this->createDocument( + $doc = $this->createDocument($content = '' ); - self::assertCount(1, $sniff->process($doc, '', 'file.xml')); + self::assertCount(1, $sniff->process($doc, new File('file.xml', $content))); } #[Test] public function itDoesNotFlagWhenCustomElementNotAllowed(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = '' ); - self::assertSame([], new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } #[Test] public function itReportsCorrectLineNumber(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = '' . PHP_EOL . ' Text' . PHP_EOL . '' ); - $violations = new SimparaSniff()->process($doc, '', 'file.xml'); + $violations = new SimparaSniff()->process($doc, new File('file.xml', $content)); self::assertSame(2, $violations[0]->line); } + #[Test] + public function itIgnoresTagShapedTextOutsideElementsWhenMappingSource(): void + { + $content = <<<'XML' + Declared">]> + + + CDATA]]> + Instruction?> + Source + + XML; + $doc = $this->createDocument($content); + + $violations = new SimparaSniff()->process($doc, new File('file.xml', $content)); + + self::assertCount(1, $violations); + self::assertSame('Source', $violations[0]->content); + self::assertSame(6, $violations[0]->line); + } + #[Test] public function itDoesNotFlagParaInsideFormalpara(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -123,13 +157,13 @@ public function itDoesNotFlagParaInsideFormalpara(): void ' ); - self::assertSame([], new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } #[Test] public function itDoesNotFlagParaWithInlineContentInsideFormalpara(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -138,13 +172,13 @@ public function itDoesNotFlagParaWithInlineContentInsideFormalpara(): void ' ); - self::assertSame([], new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } #[Test] public function itStillFlagsParasOutsideFormalparaWhenSiblingIsFormalpara(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -154,7 +188,7 @@ public function itStillFlagsParasOutsideFormalparaWhenSiblingIsFormalpara(): voi ' ); - $violations = new SimparaSniff()->process($doc, '', 'file.xml'); + $violations = new SimparaSniff()->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); } @@ -162,7 +196,7 @@ public function itStillFlagsParasOutsideFormalparaWhenSiblingIsFormalpara(): voi #[Test] public function itDoesNotFlagParaInFormalparaRegardlessOfCase(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -171,6 +205,6 @@ public function itDoesNotFlagParaInFormalparaRegardlessOfCase(): void ' ); - self::assertSame([], new SimparaSniff()->process($doc, '', 'file.xml')); + self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } } diff --git a/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php new file mode 100644 index 0000000..a1937e7 --- /dev/null +++ b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php @@ -0,0 +1,88 @@ + \r\n"; + $violations = new TrailingWhitespaceSniff()->process( + $this->createDocument($content), + new File('file.xml', $content), + ); + + self::assertCount(1, $violations); + self::assertSame('DocbookCS.TrailingWhitespace', $violations[0]->sniffCode); + self::assertSame('Trailing whitespace detected.', $violations[0]->message); + self::assertSame(' ', $violations[0]->content); + self::assertSame(strlen(''), $violations[0]->beginOffset); + self::assertSame(strlen(' '), $violations[0]->untilOffset); + self::assertSame(1, $violations[0]->line); + } + + #[Test] + public function itReportsOnlyMixedLeadingIndentationAsAffected(): void + { + $content = "\n \t\n"; + $lineOffset = strlen("\n"); + $violations = new MixedIndentationSniff()->process( + $this->createDocument($content), + new File('file.xml', $content), + ); + + self::assertCount(1, $violations); + self::assertSame('DocbookCS.MixedIndentation', $violations[0]->sniffCode); + self::assertSame('Mixed tabs and spaces in indentation.', $violations[0]->message); + self::assertSame(" \t", $violations[0]->content); + self::assertSame($lineOffset, $violations[0]->beginOffset); + self::assertSame($lineOffset + 2, $violations[0]->untilOffset); + self::assertSame(2, $violations[0]->line); + } + + #[Test] + public function itReportsDisjointConcernsOnTheSameLine(): void + { + $content = "\n \t \n"; + $document = $this->createDocument($content); + + $source = new File('file.xml', $content); + $indentation = new MixedIndentationSniff()->process($document, $source)[0]; + $trailing = new TrailingWhitespaceSniff()->process($document, $source)[0]; + + self::assertSame(2, $indentation->line); + self::assertSame(2, $trailing->line); + self::assertLessThanOrEqual($trailing->beginOffset, $indentation->untilOffset); + } + + private function createDocument(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} diff --git a/tests/Unit/Sniff/WhitespaceSniffTest.php b/tests/Unit/Sniff/WhitespaceSniffTest.php index 54e5583..e13fc34 100644 --- a/tests/Unit/Sniff/WhitespaceSniffTest.php +++ b/tests/Unit/Sniff/WhitespaceSniffTest.php @@ -4,15 +4,21 @@ namespace DocbookCS\Tests\Unit\Sniff; -use DocbookCS\Report\Violation; use DocbookCS\Sniff\WhitespaceSniff; +use DocbookCS\Source\File; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ CoversClass(Violation::class), CoversClass(WhitespaceSniff::class), + // + UsesClass(File::class), + UsesClass(SourceRange::class), ] final class WhitespaceSniffTest extends TestCase { @@ -32,7 +38,7 @@ public function itReturnsEmptyWhenNoViolations(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertSame([], $violations); } @@ -44,11 +50,14 @@ public function itDetectsTrailingWhitespace(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertSame('Trailing whitespace detected.', $violations[0]->message); self::assertSame(1, $violations[0]->line); + self::assertSame(' ', $violations[0]->content); + self::assertSame(0, $violations[0]->beginOffset); + self::assertSame(7, $violations[0]->untilOffset); } #[Test] @@ -59,11 +68,14 @@ public function itDetectsSpaceBeforeTab(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertSame('Mixed tabs and spaces in indentation.', $violations[0]->message); self::assertSame(2, $violations[0]->line); + self::assertSame(" \t", $violations[0]->content); + self::assertSame(strlen("" . PHP_EOL), $violations[0]->beginOffset); + self::assertSame(strlen("" . PHP_EOL . " \t"), $violations[0]->untilOffset); } #[Test] @@ -74,7 +86,7 @@ public function itDetectsMixedIndentationSpacesThenTabs(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertSame('Mixed tabs and spaces in indentation.', $violations[0]->message); @@ -88,7 +100,7 @@ public function itDetectsMixedIndentationTabsThenSpaces(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertCount(1, $violations); self::assertSame('Mixed tabs and spaces in indentation.', $violations[0]->message); @@ -103,7 +115,7 @@ public function itHandlesMultipleViolations(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('file.xml', $content)); self::assertCount(3, $violations); } @@ -115,7 +127,7 @@ public function itIncludesFilePathInViolation(): void ""; $doc = $this->createDocument($content); - $violations = (new WhitespaceSniff())->process($doc, $content, 'my-file.xml'); + $violations = (new WhitespaceSniff())->process($doc, new File('my-file.xml', $content)); self::assertCount(1, $violations); self::assertSame('my-file.xml', $violations[0]->filePath); diff --git a/tests/Unit/Source/FileTest.php b/tests/Unit/Source/FileTest.php new file mode 100644 index 0000000..ad7a524 --- /dev/null +++ b/tests/Unit/Source/FileTest.php @@ -0,0 +1,96 @@ +lines()); + + self::assertEquals([ + new Line(1, 'one', "\r\n", 0), + new Line(2, 'two', "\n", 5), + new Line(3, 'three', "\r", 9), + new Line(4, 'four', '', 15), + ], $lines); + self::assertSame([3, 8, 14, 19], array_map( + static fn(Line $line): int => $line->offsetAfterContent(), + $lines, + )); + self::assertSame([5, 9, 15, 19], array_map( + static fn(Line $line): int => $line->offsetAfterLine(), + $lines, + )); + } + + #[Test] + public function itResolvesOffsetsToLines(): void + { + $file = new File('file.xml', "one\r\ntwo\nthree"); + + self::assertSame(1, $file->lineAtOffset(0)->number); + self::assertSame(1, $file->lineAtOffset(3)->number); + self::assertSame(1, $file->lineAtOffset(4)->number); + self::assertSame(2, $file->lineAtOffset(5)->number); + self::assertSame(2, $file->lineAtOffset(8)->number); + self::assertSame(3, $file->lineAtOffset(9)->number); + self::assertSame(3, $file->lineAtOffset(14)->number); + } + + #[Test] + public function itIncludesAnEmptyLineAfterTheFinalLineEnding(): void + { + $file = new File('file.xml', "one\n"); + + self::assertEquals([ + new Line(1, 'one', "\n", 0), + new Line(2, '', '', 4), + ], iterator_to_array($file->lines())); + + self::assertSame(2, $file->lineAtOffset(4)->number); + } + + #[Test] + public function itCreatesANewRevisionWithoutChangingTheCurrentRevision(): void + { + $initialFile = new File('file.xml', ''); + + $modifiedFile = $initialFile->withContent(''); + + self::assertSame('', $initialFile->content); + self::assertSame('file.xml', $modifiedFile->path); + self::assertSame('', $modifiedFile->content); + } + + #[Test] + public function itReusesTheCurrentRevisionForUnchangedContent(): void + { + $file = new File('file.xml', ''); + + self::assertSame($file, $file->withContent('')); + } + + #[Test] + public function itRejectsOffsetsOutsideTheSource(): void + { + $file = new File('file.xml', 'one'); + + $this->expectException(\OutOfBoundsException::class); + $file->lineAtOffset(4); + } +} diff --git a/tests/Unit/Violation/AffectedRangesTest.php b/tests/Unit/Violation/AffectedRangesTest.php new file mode 100644 index 0000000..2fca9c0 --- /dev/null +++ b/tests/Unit/Violation/AffectedRangesTest.php @@ -0,0 +1,50 @@ +affectedRanges); + } + + #[Test] + public function relatedRangesDoNotChangeTheExistingFinding(): void + { + $ranges = [ + new SourceRange(3, 11, 15), + new SourceRange(8, 40, 44), + ]; + $violation = new Violation( + sniffCode: 'Test', + filePath: 'file.xml', + line: 3, + beginOffset: 10, + untilOffset: 45, + message: 'Message', + content: 'content', + affectedRanges: $ranges, + ); + + self::assertSame(10, $violation->beginOffset); + self::assertSame(45, $violation->untilOffset); + self::assertSame('content', $violation->content); + self::assertSame($ranges, $violation->affectedRanges); + } +}