diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aead8a9..967c6b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: - name: "Install dependencies" run: "composer install --no-interaction --prefer-dist" - - run: "mkdir -p .phpunit.cache" + - run: "mkdir -p var/.phpunit.cache" - name: "Run PHPUnit (console)" run: "vendor/bin/phpunit --configuration phpunit.xml.dist --colors=always" @@ -83,23 +83,23 @@ jobs: - name: "Generate coverage summary" if: ${{ ! cancelled() }} run: | - echo '```' > .phpunit.cache/coverage.txt + echo '```' > var/.phpunit.cache/coverage.txt vendor/bin/phpunit \ --configuration phpunit.xml.dist \ --colors=never \ - --coverage-text >> .phpunit.cache/coverage.txt - echo '```' >> .phpunit.cache/coverage.txt + --coverage-text >> var/.phpunit.cache/coverage.txt + echo '```' >> var/.phpunit.cache/coverage.txt - name: "Report test results" uses: "mikepenz/action-junit-report@v6" if: ${{ ! cancelled() }} with: - report_paths: ".phpunit.cache/junit.xml" + report_paths: "var/.phpunit.cache/junit.xml" annotate_only: true - name: "Post coverage summary" if: ${{ ! cancelled() }} - run: 'cat .phpunit.cache/coverage.txt >> "$GITHUB_STEP_SUMMARY"' + run: 'cat var/.phpunit.cache/coverage.txt >> "$GITHUB_STEP_SUMMARY"' - name: "Upload coverage report" uses: "actions/upload-artifact@v7" @@ -107,7 +107,7 @@ jobs: with: name: "coverage-report" path: | - .phpunit.cache/coverage-html - .phpunit.cache/coverage.xml - .phpunit.cache/junit.xml + var/.phpunit.cache/coverage-html + var/.phpunit.cache/coverage.xml + var/.phpunit.cache/junit.xml retention-days: 7 diff --git a/.gitignore b/.gitignore index efaa4ea..939b569 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,9 @@ /vendor/ -/.phpunit.cache/ -/.phpstan.cache/ /composer.lock -/.phpcs-cache.json + +# ------------------------------------------------------------------------------ +# Editor configuration directories +# ------------------------------------------------------------------------------ +/.idea/ +/.vscode/ +/.zed/ 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/composer.json b/composer.json index 3397bf0..ba5461a 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,8 @@ "phpunit/php-code-coverage": "^14.1", "squizlabs/php_codesniffer": "^4.0", "phpstan/phpstan-strict-rules": "^2.0", - "phpstan/phpstan-phpunit": "^2.0" + "phpstan/phpstan-phpunit": "^2.0", + "shipmonk/dead-code-detector": "^1.3" }, "autoload": { "psr-4": { @@ -28,6 +29,16 @@ "DocbookCS\\Tests\\": "tests/" } }, + "scripts": { + "test": "@php -r \"exit(extension_loaded('xdebug') || extension_loaded('pcov') ? 0 : 1);\" && XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration phpunit.xml.dist || ./vendor/bin/phpunit --configuration phpunit.xml.dist --no-coverage", + "phpstan": "./vendor/bin/phpstan analyse --no-progress", + "phpcs": "./vendor/bin/phpcs", + "quality": [ + "@phpcs", + "@phpstan", + "@test" + ] + }, "authors": [ { "name": "Jordi Kroon", diff --git a/phpcs.xml b/phpcs.xml index de0c8f6..3495fd9 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -2,8 +2,11 @@ DocbookCS Coding Standard + + tests/* + - + diff --git a/phpstan.neon b/phpstan.neon index 3e81ff1..a7c21c0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,14 +1,16 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon + - vendor/shipmonk/dead-code-detector/rules.neon parameters: level: max paths: + - bin/ - src/ - tests/ - tmpDir: .phpstan.cache + tmpDir: var/.phpstan.cache treatPhpDocTypesAsCertain: false diff --git a/phpunit.xml.dist b/phpunit.xml.dist index da429f3..540a1a7 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -3,7 +3,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="vendor/autoload.php" - cacheDirectory=".phpunit.cache" + cacheDirectory="var/.phpunit.cache" executionOrder="depends,defects" requireCoverageMetadata="true" beStrictAboutCoverageMetadata="true" @@ -18,6 +18,12 @@ tests/Unit + + tests/Integration + + + tests/Feature + @@ -28,13 +34,13 @@ - - + + - + diff --git a/src/Application.php b/src/Application.php index 45a0a0a..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); - } - - $diffLines = null; - - if ($options['diff'] !== null) { - try { - $diffContent = $this->readDiff($options['diff']); - $diffLines = (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, $diffLines); + $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/DiffChangeset.php b/src/Diff/DiffChangeset.php new file mode 100644 index 0000000..c6f5319 --- /dev/null +++ b/src/Diff/DiffChangeset.php @@ -0,0 +1,31 @@ + $fileChanges */ + public function __construct(public array $fileChanges) + { + } + + public function changeFor(string $filePath): ?FileChange + { + $normalisedPath = str_replace('\\', '/', $filePath); + + foreach ($this->fileChanges as $fileChange) { + $normalisedDiffPath = str_replace('\\', '/', $fileChange->filePath); + + if ( + $normalisedPath === $normalisedDiffPath + || str_ends_with($normalisedPath, '/' . ltrim($normalisedDiffPath, '/')) + ) { + return $fileChange; + } + } + + return null; + } +} diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 5d04cc8..b43c2c3 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -6,19 +6,29 @@ final class DiffParser { - /** @return array> */ - public function parse(string $diff): array + private const string NO_FINAL_LINE_MARKER = '\ No newline at end of file'; + + public function parse(string $diff): DiffChangeset { - $result = []; + /** @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 ')) { $currentFile = null; $deleted = false; $newLineNumber = 0; + $inHunk = false; + $previousLineWasDeletion = false; continue; } @@ -34,8 +44,10 @@ public function parse(string $diff): array $path = substr($path, 2); } $currentFile = $path !== '/dev/null' ? $path : null; - if ($currentFile !== null && !isset($result[$currentFile])) { - $result[$currentFile] = []; + $inHunk = false; + if ($currentFile !== null && !isset($changedLinesByFile[$currentFile])) { + $changedLinesByFile[$currentFile] = []; + $deletionAnchorsByFile[$currentFile] = []; } continue; } @@ -46,24 +58,56 @@ public function parse(string $diff): array // Hunk header: @@ -old_start[,old_count] +new_start[,new_count] @@ if (str_starts_with($line, '@@ ')) { - if (preg_match('/\+(\d+)(?:,\d+)?/', $line, $m)) { - $newLineNumber = (int) $m[1]; + if (preg_match('/^@@ -\d+(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/', $line, $m, PREG_UNMATCHED_AS_NULL)) { + $oldLinesRemaining = isset($m[1]) ? (int) $m[1] : 1; + $newLineNumber = (int) $m[2]; + $newLinesRemaining = isset($m[3]) ? (int) $m[3] : 1; + $inHunk = true; + $previousLineWasDeletion = false; } continue; } - if (str_starts_with($line, '+')) { - $result[$currentFile][] = $newLineNumber; - $newLineNumber++; + if (!$inHunk || $line === self::NO_FINAL_LINE_MARKER) { continue; } - if (!str_starts_with($line, '-')) { + if (str_starts_with($line, '+')) { + $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; } } - return $result; + $fileChanges = []; + + foreach ($changedLinesByFile as $filePath => $lineNumbers) { + $fileChanges[] = new FileChange( + $filePath, + $lineNumbers, + $deletionAnchorsByFile[$filePath], + ); + } + + 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 $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 @@ -' + . $content + . ''; + } + + public static function contains(\DOMNode $node): bool + { + for ($current = $node; $current->parentNode !== null; $current = $current->parentNode) { + if (self::isBetweenMarkers($current)) { + return true; + } + } + + return false; + } + + private static function isBetweenMarkers(\DOMNode $node): bool + { + $nestedMarkers = 0; + + for ($sibling = $node->previousSibling; $sibling !== null; $sibling = $sibling->previousSibling) { + if (self::isEnd($sibling)) { + $nestedMarkers++; + continue; + } + + if (!self::isStart($sibling)) { + continue; + } + + if ($nestedMarkers === 0) { + return true; + } + + $nestedMarkers--; + } + + return false; + } + + private static function isStart(\DOMNode $node): bool + { + return $node instanceof \DOMComment && $node->textContent === self::START; + } + + private static function isEnd(\DOMNode $node): bool + { + return $node instanceof \DOMComment && $node->textContent === self::END; + } +} diff --git a/src/Runner/EntityPreprocessor.php b/src/Runner/EntityPreprocessor.php index a603684..61327a1 100644 --- a/src/Runner/EntityPreprocessor.php +++ b/src/Runner/EntityPreprocessor.php @@ -25,7 +25,14 @@ public function process(string $xml): string return $this->expandEntities($xml); } - private function expandEntities(string $content): string + public function processForParsing(string $xml): string + { + $xml = $this->stripDoctype($xml); + + return $this->expandEntities($xml, markXmlExpansions: true); + } + + private function expandEntities(string $content, bool $markXmlExpansions = false): string { $maxDepth = 20; @@ -33,13 +40,23 @@ private function expandEntities(string $content): string $changed = false; $content = preg_replace_callback( - '/|' . self::ENTITY_PATTERN . '/', - function (array $matches) use (&$changed): string { - // If this is a comment, return as is + '/||<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/', + function (array $matches) use (&$changed, $markXmlExpansions): string { + // Entity-like text inside comments is literal. if (str_starts_with($matches[0], '&value;'; + + self::assertSame( + 'expanded', + $preprocessor->process($source), + ); + } + + private function parse(string $xml): \DOMDocument + { + $document = new \DOMDocument(); + $document->loadXML($xml); + + return $document; + } +} 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..1830e32 --- /dev/null +++ b/tests/Integration/Runner/RunScopeResolverTest.php @@ -0,0 +1,137 @@ +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); + } + + 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 new file mode 100644 index 0000000..886a3f8 --- /dev/null +++ b/tests/Integration/Sniff/EntityExpandedSniffTest.php @@ -0,0 +1,67 @@ +Source&expanded;'; + $document = $this->processedDocument($source, 'Expanded'); + + $violations = new SimparaSniff()->process($document, new File('file.xml', $source)); + + self::assertCount(1, $violations); + self::assertSame('Source', $violations[0]->content); + } + + #[Test] + public function exceptionNameIgnoresExpandedElements(): void + { + $source = 'RuntimeException&expanded;'; + $document = $this->processedDocument($source, 'ExpandedException'); + + $violations = new ExceptionNameSniff()->process($document, new File('file.xml', $source)); + + self::assertCount(1, $violations); + self::assertSame('RuntimeException', $violations[0]->content); + } + + private function processedDocument(string $source, string $expanded): \DOMDocument + { + $content = new EntityPreprocessor(['expanded' => $expanded])->processForParsing($source); + $document = new \DOMDocument(); + $document->loadXML($content); + + return $document; + } +} 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 6e24e79..1ebef99 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -6,13 +6,19 @@ use DocbookCS\Application; use DocbookCS\Config\ConfigData; +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; @@ -20,32 +26,58 @@ 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; use PHPUnit\Framework\TestCase; -#[CoversClass(Application::class)] -#[CoversClass(DiffParser::class)] -#[CoversClass(ConfigParser::class)] -#[CoversClass(ConfigParserException::class)] -#[CoversClass(ConfigData::class)] -#[CoversClass(SniffEntry::class)] -#[CoversClass(PathLoader::class)] -#[CoversClass(PathMatcher::class)] -#[CoversClass(NullProgress::class)] -#[CoversClass(Report::class)] -#[CoversClass(ConsoleReporter::class)] -#[CoversClass(EntityPreprocessor::class)] -#[CoversClass(SniffRunner::class)] -#[CoversClass(XmlFileProcessor::class)] -#[CoversClass(CheckstyleReporter::class)] -#[CoversClass(JsonReporter::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(ExceptionNameSniff::class)] -#[CoversClass(EntityResolver::class)] +#[ + CoversClass(Application::class), + CoversClass(CheckstyleReporter::class), + CoversClass(ConfigData::class), + CoversClass(ConfigParser::class), + CoversClass(ConfigParserException::class), + CoversClass(ConsoleReporter::class), + CoversClass(DiffParser::class), + CoversClass(EntityPreprocessor::class), + CoversClass(EntityResolver::class), + CoversClass(ExceptionNameSniff::class), + CoversClass(FileReport::class), + CoversClass(JsonReporter::class), + CoversClass(NullProgress::class), + 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(XmlFileProcessor::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 { private const string FIXTURE_DIR = __DIR__ . '/../fixtures/application'; @@ -80,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); @@ -92,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); @@ -104,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( @@ -119,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( @@ -134,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( @@ -148,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); @@ -158,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) { @@ -174,7 +206,7 @@ public function itSupportsReportFormats(): void } } - #[Test] + #[Test] // TODO: should be feature public function itSupportsColorFlags(): void { foreach (['--colors', '--no-colors'] as $flag) { @@ -190,7 +222,7 @@ public function itSupportsColorFlags(): void } } - #[Test] + #[Test] // TODO: should be feature public function helpShortCircuitsExecution(): void { $app = new Application( @@ -206,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( @@ -222,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( @@ -238,7 +270,7 @@ public function itResolvesRelativeOverridePathsAgainstCwd(): void self::assertNotSame(2, $exitCode); } - #[Test] + #[Test] // TODO: should be feature public function itCatchesRuntimeErrorFromRunner(): void { $app = new Application( @@ -253,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( @@ -267,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( @@ -281,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( @@ -404,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) { @@ -427,7 +351,7 @@ public function itSuppressesProgressForStructuredReportFormats(): void } } - #[Test] + #[Test] // TODO: should be feature public function itShowsPerformanceWhenPerfFlagIsEnabled(): void { $app = new Application( @@ -450,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/Config/ConfigParserTest.php b/tests/Unit/Config/ConfigParserTest.php index df56a0b..ca490d2 100644 --- a/tests/Unit/Config/ConfigParserTest.php +++ b/tests/Unit/Config/ConfigParserTest.php @@ -12,10 +12,12 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(ConfigParser::class)] -#[CoversClass(ConfigData::class)] -#[CoversClass(SniffEntry::class)] -#[CoversClass(ConfigParserException::class)] +#[ + CoversClass(ConfigData::class), + CoversClass(ConfigParser::class), + CoversClass(ConfigParserException::class), + CoversClass(SniffEntry::class), +] final class ConfigParserTest extends TestCase { private ConfigParser $parser; @@ -138,7 +140,7 @@ public function itResolvesAbsolutePathsAsIs(): void public function itThrowsOnMissingFile(): void { $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('not found'); + $this->expectExceptionMessageIsOrContains('not found'); $this->parser->parseFile('/nonexistent/docbookcs.xml'); } @@ -147,7 +149,7 @@ public function itThrowsOnMissingFile(): void public function itThrowsOnInvalidXml(): void { $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('Invalid XML'); + $this->expectExceptionMessageIsOrContains('Invalid XML'); $this->parser->parseString('expectException(ConfigParserException::class); - $this->expectExceptionMessage(''); + $this->expectExceptionMessageIsOrContains(''); $this->parser->parseString($xml, '/base'); } @@ -187,7 +189,7 @@ public function itThrowsWhenSniffHasNoClassAttribute(): void XML; $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('class'); + $this->expectExceptionMessageIsOrContains('class'); $this->parser->parseString($xml, '/base'); } @@ -210,7 +212,7 @@ public function itThrowsWhenPropertyHasNoNameAttribute(): void XML; $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('name'); + $this->expectExceptionMessageIsOrContains('name'); $this->parser->parseString($xml, '/base'); } @@ -314,7 +316,7 @@ public function itThrowsWhenFileContainsInvalidXml(): void $fixturePath = __DIR__ . '/../../fixtures/invalid.xml'; $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('Invalid XML'); + $this->expectExceptionMessageIsOrContains('Invalid XML'); $this->parser->parseFile($fixturePath); } diff --git a/tests/Unit/Config/SniffEntryTest.php b/tests/Unit/Config/SniffEntryTest.php index 95e47ab..f464340 100644 --- a/tests/Unit/Config/SniffEntryTest.php +++ b/tests/Unit/Config/SniffEntryTest.php @@ -9,14 +9,16 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(SniffEntry::class)] +#[ + CoversClass(SniffEntry::class), +] final class SniffEntryTest extends TestCase { #[Test] public function itThrowsExceptionForEmptyClassName(): void { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('Sniff class name must not be empty.'); + $this->expectExceptionMessageIs('Sniff class name must not be empty.'); new SniffEntry(''); } diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 5581ff9..b1bd988 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -4,12 +4,20 @@ namespace DocbookCS\Tests\Unit\Diff; +use DocbookCS\Diff\DiffChangeset; use DocbookCS\Diff\DiffParser; +use DocbookCS\Diff\FileChange; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -#[CoversClass(DiffParser::class)] +#[ + CoversClass(DiffParser::class), + // + UsesClass(DiffChangeset::class), + UsesClass(FileChange::class), +] final class DiffParserTest extends TestCase { private DiffParser $parser; @@ -22,7 +30,7 @@ protected function setUp(): void #[Test] public function itReturnsEmptyArrayForEmptyDiff(): void { - self::assertSame([], $this->parser->parse('')); + self::assertSame([], $this->lineNumbersByFile($this->parser->parse(''))); } #[Test] @@ -39,7 +47,7 @@ public function itParsesAddedLineNumbers(): void line3 DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertArrayHasKey('reference/file.xml', $result); self::assertSame([2], $result['reference/file.xml']); @@ -60,7 +68,7 @@ public function itParsesMultipleAddedLines(): void last line DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertSame([6, 7], $result['doc/chapter.xml']); } @@ -77,7 +85,7 @@ public function itStripsTheBPrefix(): void +added DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertArrayHasKey('src/file.xml', $result); self::assertArrayNotHasKey('b/src/file.xml', $result); @@ -97,7 +105,7 @@ public function itExcludesDeletedFiles(): void -line3 DIFF; - self::assertSame([], $this->parser->parse($diff)); + self::assertSame([], $this->lineNumbersByFile($this->parser->parse($diff))); } #[Test] @@ -114,7 +122,7 @@ public function itHandlesNewlyCreatedFiles(): void +line3 DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertArrayHasKey('new.xml', $result); self::assertSame([1, 2, 3], $result['new.xml']); @@ -140,7 +148,7 @@ public function itHandlesMultipleFilesInOneDiff(): void unchanged DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertArrayHasKey('first.xml', $result); self::assertArrayHasKey('second.xml', $result); @@ -162,13 +170,72 @@ public function itIgnoresRemovedLines(): void line3 DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); // No lines added, so the changed set is empty (not absent — the file is tracked). self::assertArrayHasKey('file.xml', $result); 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 + { + $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; + + $result = $this->lineNumbersByFile($this->parser->parse($diff)); + + 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 { @@ -188,7 +255,7 @@ public function itTracksLineNumbersAcrossMultipleHunks(): void line12 DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertSame([2, 12], $result['file.xml']); } @@ -204,8 +271,21 @@ public function itHandlesHunkWithNoContext(): void +only line DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertSame([1], $result['file.xml']); } + + // TODO: avoids test diff churn; remove when fixers merged + /** @return array> */ + private function lineNumbersByFile(DiffChangeset $diff): array + { + $lineNumbersByFile = []; + + foreach ($diff->fileChanges as $fileChange) { + $lineNumbersByFile[$fileChange->filePath] = $fileChange->addedLineNumbers; + } + + return $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/Path/EntityResolverTest.php b/tests/Unit/Path/EntityResolverTest.php index 8f616a2..82832ac 100644 --- a/tests/Unit/Path/EntityResolverTest.php +++ b/tests/Unit/Path/EntityResolverTest.php @@ -11,8 +11,11 @@ use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -#[CoversClass(EntityResolver::class)] -#[UsesClass(EntityPreprocessor::class)] +#[ + CoversClass(EntityResolver::class), + // + UsesClass(EntityPreprocessor::class), +] final class EntityResolverTest extends TestCase { private string $fixtureRoot; diff --git a/tests/Unit/Path/PathLoaderTest.php b/tests/Unit/Path/PathLoaderTest.php index 5a86056..0118b9b 100644 --- a/tests/Unit/Path/PathLoaderTest.php +++ b/tests/Unit/Path/PathLoaderTest.php @@ -10,8 +10,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(PathLoader::class)] -#[CoversClass(PathMatcher::class)] +#[ + CoversClass(PathLoader::class), + CoversClass(PathMatcher::class), +] final class PathLoaderTest extends TestCase { private const string FIXTURE_ROOT = __DIR__ . '/../../fixtures/sample_tree'; diff --git a/tests/Unit/Path/PathMatcherTest.php b/tests/Unit/Path/PathMatcherTest.php index 9ed9f42..5ab521c 100644 --- a/tests/Unit/Path/PathMatcherTest.php +++ b/tests/Unit/Path/PathMatcherTest.php @@ -10,7 +10,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(PathMatcher::class)] +#[ + CoversClass(PathMatcher::class), +] final class PathMatcherTest extends TestCase { /** diff --git a/tests/Unit/Progress/ConsoleProgressTest.php b/tests/Unit/Progress/ConsoleProgressTest.php index 2946fb6..9bdc5fa 100644 --- a/tests/Unit/Progress/ConsoleProgressTest.php +++ b/tests/Unit/Progress/ConsoleProgressTest.php @@ -9,7 +9,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(ConsoleProgress::class)] +#[ + CoversClass(ConsoleProgress::class), +] final class ConsoleProgressTest extends TestCase { /** @var resource */ diff --git a/tests/Unit/Progress/NullProgressTest.php b/tests/Unit/Progress/NullProgressTest.php index a51eb3d..9ad02bb 100644 --- a/tests/Unit/Progress/NullProgressTest.php +++ b/tests/Unit/Progress/NullProgressTest.php @@ -9,7 +9,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(NullProgress::class)] +#[ + CoversClass(NullProgress::class), +] final class NullProgressTest extends TestCase { #[Test] diff --git a/tests/Unit/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php index b9085fd..bff8881 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -4,17 +4,25 @@ 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(Report::class)] -#[CoversClass(Violation::class)] -#[CoversClass(FileReport::class)] +#[ + CoversClass(FileReport::class), + CoversClass(RelativePath::class), + CoversClass(Report::class), + CoversClass(Violation::class), + // + UsesClass(SourceRange::class), +] final class ReportTest extends TestCase { private function createViolation( @@ -24,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] @@ -66,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 91a46e8..4e72cb3 100644 --- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php +++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php @@ -4,19 +4,27 @@ 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; -#[CoversClass(CheckstyleReporter::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(Report::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(CheckstyleReporter::class), + CoversClass(FileReport::class), + CoversClass(Report::class), + CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), +] final class CheckstyleReporterTest extends TestCase { private CheckstyleReporter $reporter; @@ -32,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 @@ -111,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 1dd96b8..4b29bf9 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -4,19 +4,27 @@ 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; -#[CoversClass(ConsoleReporter::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(Report::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(ConsoleReporter::class), + CoversClass(FileReport::class), + CoversClass(Report::class), + CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), +] final class ConsoleReporterTest extends TestCase { private ConsoleReporter $reporter; @@ -33,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] @@ -88,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 1e488fa..dda9faf 100644 --- a/tests/Unit/Report/Reporter/JsonReporterTest.php +++ b/tests/Unit/Report/Reporter/JsonReporterTest.php @@ -4,19 +4,27 @@ 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; -#[CoversClass(JsonReporter::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(Report::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(FileReport::class), + CoversClass(JsonReporter::class), + CoversClass(Report::class), + CoversClass(Violation::class), + // + UsesClass(RelativePath::class), + UsesClass(SourceRange::class), +] final class JsonReporterTest extends TestCase { private JsonReporter $reporter; @@ -32,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] @@ -310,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/EntityPreprocessorTest.php b/tests/Unit/Runner/EntityPreprocessorTest.php index 936b378..bd8d0db 100644 --- a/tests/Unit/Runner/EntityPreprocessorTest.php +++ b/tests/Unit/Runner/EntityPreprocessorTest.php @@ -10,7 +10,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(EntityPreprocessor::class)] +#[ + CoversClass(EntityPreprocessor::class), +] final class EntityPreprocessorTest extends TestCase { #[Test] 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 f6801a3..db78127 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -6,6 +6,10 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; +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; @@ -13,28 +17,56 @@ use DocbookCS\Progress\ProgressInterface; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; -use DocbookCS\Report\Violation; 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; use PHPUnit\Framework\TestCase; -#[CoversClass(SniffRunner::class)] -#[CoversClass(ConfigData::class)] -#[CoversClass(PathLoader::class)] -#[CoversClass(PathMatcher::class)] -#[CoversClass(NullProgress::class)] -#[CoversClass(EntityPreprocessor::class)] -#[CoversClass(EntityResolver::class)] -#[CoversClass(XmlFileProcessor::class)] -#[CoversClass(Report::class)] -#[CoversClass(SniffEntry::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(ConfigData::class), + CoversClass(EntityPreprocessor::class), + CoversClass(EntityResolver::class), + CoversClass(FileReport::class), + CoversClass(NullProgress::class), + 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(Violation::class), + CoversClass(XmlFileProcessor::class), + // + UsesClass(DiffChangeset::class), + UsesClass(DiffPathLoader::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 { private const string FIXTURE_DIR = __DIR__ . '/../../fixtures/sniff_runner/default'; @@ -52,31 +84,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); @@ -93,26 +128,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, ), @@ -126,30 +167,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, ), @@ -163,34 +210,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 []; } @@ -198,103 +251,110 @@ 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(); - $diffLines = ['sniff_runner/default/file_a.xml' => [1]]; - $report = $runner->run($config, null, $diffLines); + $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(); - $diffLines = ['completely/different/file.xml' => [1, 2, 3]]; - $report = $runner->run($config, null, $diffLines); + $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'; - $diffLines = [$discoveredPath => [1]]; - $report = $runner->run($config, null, $diffLines); + $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 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, ), @@ -307,12 +367,23 @@ public function setProperty(string $name, string $value): void }; $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); - $runner = new SniffRunner(); + $runner = new RunCoordinator(); - $diffLines = ['sniff_runner/default/file_a.xml' => []]; - $report = $runner->run($config, null, $diffLines); + $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 6bd60b4..0eb1d7a 100644 --- a/tests/Unit/Runner/XmlFileProcessorTest.php +++ b/tests/Unit/Runner/XmlFileProcessorTest.php @@ -4,50 +4,77 @@ 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; -#[CoversClass(EntityPreprocessor::class)] -#[CoversClass(FileReport::class)] -#[CoversClass(Violation::class)] -#[CoversClass(XmlFileProcessor::class)] -#[CoversClass(Report::class)] +#[ + CoversClass(EntityPreprocessor::class), + 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] @@ -55,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( @@ -75,7 +102,7 @@ public function itHandlesEntitiesWithoutParseErrors(): void 'php.ini' => '', ])); - $report = $processor->processString($xml); + $report = $this->process($processor, $xml); self::assertCount( 0, @@ -86,7 +113,7 @@ public function itHandlesEntitiesWithoutParseErrors(): void ); } - #[Test] + #[Test] // TODO: should be integration public function itUsesCustomPreprocessor(): void { $processor = $this->processor([], new EntityPreprocessor([ @@ -95,7 +122,7 @@ public function itUsesCustomPreprocessor(): void $xml = $this->xml('&custom.entity;'); - $report = $processor->processString($xml); + $report = $this->process($processor, $xml); self::assertCount( 0, @@ -111,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()); } @@ -129,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()); } @@ -147,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); @@ -168,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()); } @@ -184,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()); } @@ -202,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()); } @@ -221,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()); } @@ -243,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()); } @@ -260,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 { @@ -287,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 ), @@ -324,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 26b9262..0f8e834 100644 --- a/tests/Unit/Sniff/AbstractSniffTest.php +++ b/tests/Unit/Sniff/AbstractSniffTest.php @@ -5,22 +5,25 @@ 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; -#[CoversClass(AbstractSniff::class)] +#[ + CoversClass(AbstractSniff::class), +] 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 0673283..4a89fbe 100644 --- a/tests/Unit/Sniff/AttributeOrderSniffTest.php +++ b/tests/Unit/Sniff/AttributeOrderSniffTest.php @@ -4,14 +4,24 @@ 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)] +#[ + CoversClass(AttributeOrderSniff::class), + CoversClass(Violation::class), + // + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), +] final class AttributeOrderSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument @@ -32,7 +42,7 @@ public function itIgnoresElementsWithoutRelevantAttributes(): void self::assertSame( [], - $sniff->process($doc, $content, 'test.xml') + $sniff->process($doc, new File('test.xml', $content)) ); } @@ -46,7 +56,7 @@ public function itDoesNotFlagWhenXmlIdComesFirst(): void self::assertSame( [], - $sniff->process($doc, $content, 'test.xml') + $sniff->process($doc, new File('test.xml', $content)) ); } @@ -58,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( @@ -77,7 +87,7 @@ public function itHandlesXmlnsPrefixedAttributes(): void self::assertCount( 1, - $sniff->process($doc, $content, 'file.xml') + $sniff->process($doc, new File('file.xml', $content)) ); } @@ -95,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); } @@ -111,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); @@ -126,7 +136,24 @@ 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); + } } diff --git a/tests/Unit/Sniff/ExceptionNameSniffTest.php b/tests/Unit/Sniff/ExceptionNameSniffTest.php index de6600d..b4c8355 100644 --- a/tests/Unit/Sniff/ExceptionNameSniffTest.php +++ b/tests/Unit/Sniff/ExceptionNameSniffTest.php @@ -4,14 +4,26 @@ 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; use PHPUnit\Framework\TestCase; -#[CoversClass(ExceptionNameSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(ExceptionNameSniff::class), + CoversClass(Violation::class), + // + UsesClass(EntityExpansionMarker::class), + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), +] final class ExceptionNameSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument @@ -27,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); } @@ -37,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); } @@ -47,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); } @@ -57,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( @@ -71,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( @@ -85,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( @@ -99,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( @@ -113,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); } @@ -129,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); } @@ -139,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); } @@ -149,9 +161,44 @@ 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); + } } diff --git a/tests/Unit/Sniff/SimparaSniffTest.php b/tests/Unit/Sniff/SimparaSniffTest.php index bb8cdde..b0004f0 100644 --- a/tests/Unit/Sniff/SimparaSniffTest.php +++ b/tests/Unit/Sniff/SimparaSniffTest.php @@ -4,14 +4,26 @@ 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; use PHPUnit\Framework\TestCase; -#[CoversClass(SimparaSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(SimparaSniff::class), + CoversClass(Violation::class), + // + UsesClass(EntityExpansionMarker::class), + UsesClass(File::class), + UsesClass(Line::class), + UsesClass(SourceRange::class), +] final class SimparaSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument @@ -25,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] @@ -55,7 +74,7 @@ public function itHandlesMultipleParas(): void { $sniff = new SimparaSniff(); - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Inline @@ -63,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); } @@ -74,33 +93,33 @@ 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); } @@ -108,7 +127,7 @@ public function itReportsCorrectLineNumber(): void #[Test] public function itDoesNotFlagParaInsideFormalpara(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -117,13 +136,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 @@ -132,13 +151,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 @@ -148,7 +167,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); } @@ -156,7 +175,7 @@ public function itStillFlagsParasOutsideFormalparaWhenSiblingIsFormalpara(): voi #[Test] public function itDoesNotFlagParaInFormalparaRegardlessOfCase(): void { - $doc = $this->createDocument( + $doc = $this->createDocument($content = ' Title @@ -165,6 +184,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 17d81c0..e13fc34 100644 --- a/tests/Unit/Sniff/WhitespaceSniffTest.php +++ b/tests/Unit/Sniff/WhitespaceSniffTest.php @@ -4,14 +4,22 @@ 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(WhitespaceSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(Violation::class), + CoversClass(WhitespaceSniff::class), + // + UsesClass(File::class), + UsesClass(SourceRange::class), +] final class WhitespaceSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument @@ -30,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); } @@ -42,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] @@ -57,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] @@ -72,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); @@ -86,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); @@ -101,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); } @@ -113,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); + } +} diff --git a/var/.gitignore b/var/.gitignore new file mode 100644 index 0000000..d6b7ef3 --- /dev/null +++ b/var/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore