From d9b5951734e43ba4923c44b3f89e34cc628b6afc Mon Sep 17 00:00:00 2001 From: NickSdot <32384907+NickSdot@users.noreply.github.com> Date: Mon, 20 Jul 2026 20:53:49 +0700 Subject: [PATCH 01/14] Improve CI tooling (#25) --- .github/workflows/ci.yml | 18 +++++++++--------- .gitignore | 10 +++++++--- composer.json | 13 ++++++++++++- phpcs.xml | 2 +- phpstan.neon | 3 ++- phpunit.xml.dist | 8 ++++---- var/.gitignore | 2 ++ 7 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 var/.gitignore 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/composer.json b/composer.json index 3397bf0..6bd6998 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": "./vendor/bin/phpunit --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..93f3b2f 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -3,7 +3,7 @@ DocbookCS Coding Standard - + diff --git a/phpstan.neon b/phpstan.neon index 3e81ff1..a88585b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,6 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon + - vendor/shipmonk/dead-code-detector/rules.neon parameters: level: max @@ -8,7 +9,7 @@ parameters: - src/ - tests/ - tmpDir: .phpstan.cache + tmpDir: var/.phpstan.cache treatPhpDocTypesAsCertain: false diff --git a/phpunit.xml.dist b/phpunit.xml.dist index da429f3..f173b58 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" @@ -28,13 +28,13 @@ - - + + - + 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 From e624a5b91a77e1e351edf5084c774a374ba0f1d8 Mon Sep 17 00:00:00 2001 From: NickSdot <32384907+NickSdot@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:02:53 +0700 Subject: [PATCH 02/14] fix: ignore missing newline markers (#26) --- src/Diff/DiffParser.php | 32 ++++++++++++++++++++++++------ tests/Unit/Diff/DiffParserTest.php | 19 ++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 5d04cc8..fc29ef2 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -6,6 +6,8 @@ final class DiffParser { + private const string NO_FINAL_LINE_MARKER = '\ No newline at end of file'; + /** @return array> */ public function parse(string $diff): array { @@ -13,12 +15,16 @@ public function parse(string $diff): array $currentFile = null; $deleted = false; $newLineNumber = 0; + $oldLinesRemaining = 0; + $newLinesRemaining = 0; + $inHunk = false; foreach (explode("\n", $diff) as $line) { if (str_starts_with($line, 'diff --git ')) { $currentFile = null; $deleted = false; $newLineNumber = 0; + $inHunk = false; continue; } @@ -34,6 +40,7 @@ public function parse(string $diff): array $path = substr($path, 2); } $currentFile = $path !== '/dev/null' ? $path : null; + $inHunk = false; if ($currentFile !== null && !isset($result[$currentFile])) { $result[$currentFile] = []; } @@ -46,21 +53,34 @@ 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; } 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, '+')) { + $result[$currentFile][] = $newLineNumber; + $newLineNumber++; + $newLinesRemaining--; + } elseif (str_starts_with($line, '-')) { + $oldLinesRemaining--; + } elseif (str_starts_with($line, ' ')) { // Context line — present in both old and new file. $newLineNumber++; + $oldLinesRemaining--; + $newLinesRemaining--; + } + + if ($oldLinesRemaining === 0 && $newLinesRemaining === 0) { + $inHunk = false; } } diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 5581ff9..8afed41 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -169,6 +169,25 @@ public function itIgnoresRemovedLines(): void self::assertSame([], $result['file.xml']); } + #[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->parser->parse($diff); + + self::assertSame([1, 2], $result['file.xml']); + } + #[Test] public function itTracksLineNumbersAcrossMultipleHunks(): void { From 273595622748c1ffdccf7c677cadfa6e40a577b0 Mon Sep 17 00:00:00 2001 From: NickSdot <32384907+NickSdot@users.noreply.github.com> Date: Tue, 21 Jul 2026 02:07:19 +0700 Subject: [PATCH 03/14] quality: add bin/ to PHPStan paths (#27) --- phpstan.neon | 1 + 1 file changed, 1 insertion(+) diff --git a/phpstan.neon b/phpstan.neon index a88585b..a7c21c0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,6 +6,7 @@ parameters: level: max paths: + - bin/ - src/ - tests/ From 78bce0a47211f28fa8f99375bca4fc4034b89954 Mon Sep 17 00:00:00 2001 From: NickSdot <32384907+NickSdot@users.noreply.github.com> Date: Tue, 21 Jul 2026 03:37:27 +0700 Subject: [PATCH 04/14] test: replace deprecated PHPUnit assertions (#28) * test: replace deprecated PHPUnit assertions Co-authored-by: Jordi Kroon --- tests/Unit/Config/ConfigParserTest.php | 12 ++++++------ tests/Unit/Config/SniffEntryTest.php | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Unit/Config/ConfigParserTest.php b/tests/Unit/Config/ConfigParserTest.php index df56a0b..721d5eb 100644 --- a/tests/Unit/Config/ConfigParserTest.php +++ b/tests/Unit/Config/ConfigParserTest.php @@ -138,7 +138,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 +147,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 +187,7 @@ public function itThrowsWhenSniffHasNoClassAttribute(): void XML; $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('class'); + $this->expectExceptionMessageIsOrContains('class'); $this->parser->parseString($xml, '/base'); } @@ -210,7 +210,7 @@ public function itThrowsWhenPropertyHasNoNameAttribute(): void XML; $this->expectException(ConfigParserException::class); - $this->expectExceptionMessage('name'); + $this->expectExceptionMessageIsOrContains('name'); $this->parser->parseString($xml, '/base'); } @@ -314,7 +314,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..c5ae22b 100644 --- a/tests/Unit/Config/SniffEntryTest.php +++ b/tests/Unit/Config/SniffEntryTest.php @@ -16,7 +16,7 @@ final class SniffEntryTest extends TestCase 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(''); } From ff1b96d014b5d8c488b8fcac4e0f1774422effb8 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 01:49:04 +0700 Subject: [PATCH 05/14] refactor: model file changes as value objects --- src/Application.php | 6 +-- src/Diff/Diff.php | 31 ++++++++++++ src/Diff/DiffParser.php | 20 +++++--- src/Diff/FileChange.php | 15 ++++++ src/Runner/SniffRunner.php | 71 +++------------------------ tests/Unit/Diff/DiffParserTest.php | 38 +++++++++----- tests/Unit/Runner/SniffRunnerTest.php | 18 ++++--- 7 files changed, 106 insertions(+), 93 deletions(-) create mode 100644 src/Diff/Diff.php create mode 100644 src/Diff/FileChange.php diff --git a/src/Application.php b/src/Application.php index 45a0a0a..9827b56 100644 --- a/src/Application.php +++ b/src/Application.php @@ -83,12 +83,12 @@ public function run(): int $overridePaths = $this->resolveOverridePaths($overridePaths); } - $diffLines = null; + $diff = null; if ($options['diff'] !== null) { try { $diffContent = $this->readDiff($options['diff']); - $diffLines = (new DiffParser())->parse($diffContent); + $diff = (new DiffParser())->parse($diffContent); } catch (\Throwable $e) { $this->writeError('Error reading diff: ' . $e->getMessage() . PHP_EOL); @@ -100,7 +100,7 @@ public function run(): int try { $runner = new SniffRunner($progress); - $report = $runner->run($config, $overridePaths, $diffLines); + $report = $runner->run($config, $overridePaths, $diff); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); diff --git a/src/Diff/Diff.php b/src/Diff/Diff.php new file mode 100644 index 0000000..83b3bd9 --- /dev/null +++ b/src/Diff/Diff.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 fc29ef2..9dbd2fe 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -8,10 +8,10 @@ final class DiffParser { private const string NO_FINAL_LINE_MARKER = '\ No newline at end of file'; - /** @return array> */ - public function parse(string $diff): array + public function parse(string $diff): Diff { - $result = []; + /** @var array> $changedLinesByFile */ + $changedLinesByFile = []; $currentFile = null; $deleted = false; $newLineNumber = 0; @@ -41,8 +41,8 @@ public function parse(string $diff): array } $currentFile = $path !== '/dev/null' ? $path : null; $inHunk = false; - if ($currentFile !== null && !isset($result[$currentFile])) { - $result[$currentFile] = []; + if ($currentFile !== null && !isset($changedLinesByFile[$currentFile])) { + $changedLinesByFile[$currentFile] = []; } continue; } @@ -67,7 +67,7 @@ public function parse(string $diff): array } if (str_starts_with($line, '+')) { - $result[$currentFile][] = $newLineNumber; + $changedLinesByFile[$currentFile][] = $newLineNumber; $newLineNumber++; $newLinesRemaining--; } elseif (str_starts_with($line, '-')) { @@ -84,6 +84,12 @@ public function parse(string $diff): array } } - return $result; + $fileChanges = []; + + foreach ($changedLinesByFile as $filePath => $lineNumbers) { + $fileChanges[] = new FileChange($filePath, $lineNumbers); + } + + return new Diff($fileChanges); } } diff --git a/src/Diff/FileChange.php b/src/Diff/FileChange.php new file mode 100644 index 0000000..6b31ca2 --- /dev/null +++ b/src/Diff/FileChange.php @@ -0,0 +1,15 @@ + $addedLineNumbers */ + public function __construct( + public string $filePath, + public array $addedLineNumbers, + ) { + } +} diff --git a/src/Runner/SniffRunner.php b/src/Runner/SniffRunner.php index 15e7e9a..2884155 100644 --- a/src/Runner/SniffRunner.php +++ b/src/Runner/SniffRunner.php @@ -6,6 +6,7 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; +use DocbookCS\Diff\Diff; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; @@ -25,11 +26,10 @@ public function __construct(?ProgressInterface $progress = null) /** * @param list|null $overridePaths - * @param array>|null $diffLines * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. * @throws \UnexpectedValueException if no files are found to scan. */ - public function run(ConfigData $config, ?array $overridePaths = null, ?array $diffLines = null): Report + public function run(ConfigData $config, ?array $overridePaths = null, ?Diff $diff = null): Report { $startTime = microtime(true); @@ -45,8 +45,11 @@ public function run(ConfigData $config, ?array $overridePaths = null, ?array $di $pathLoader = new PathLoader($includePaths, $matcher); $files = $pathLoader->loadPaths(); - if ($diffLines !== null) { - $files = $this->filterByDiff($files, array_keys($diffLines)); + if ($diff !== null) { + $files = array_values(array_filter( + $files, + static fn(string $file): bool => $diff->changeFor($file) !== null, + )); } $report = new Report(); @@ -60,9 +63,7 @@ public function run(ConfigData $config, ?array $overridePaths = null, ?array $di foreach ($files as $index => $file) { $report->incrementFilesScanned(); - $changedLines = $diffLines !== null - ? $this->getChangedLinesForFile($file, $diffLines) - : null; + $changedLines = $diff?->changeFor($file)?->addedLineNumbers; $fileReport = $processor->processFile( $file, @@ -125,40 +126,6 @@ private function instantiateSniffs(array $entries): array return $sniffs; } - /** - * @param list $files - * @param list $diffPaths - * @return list - */ - private function filterByDiff(array $files, array $diffPaths): array - { - return array_values( - array_filter( - $files, - fn(string $file) => $this->matchesDiffPath($file, $diffPaths), - ) - ); - } - - /** @param list $diffPaths */ - private function matchesDiffPath(string $absolutePath, array $diffPaths): bool - { - $normalized = str_replace('\\', '/', $absolutePath); - - foreach ($diffPaths as $diffPath) { - $normalizedDiff = str_replace('\\', '/', $diffPath); - - if ( - $normalized === $normalizedDiff - || str_ends_with($normalized, '/' . ltrim($normalizedDiff, '/')) - ) { - return true; - } - } - - return false; - } - private function makeRelative(string $absolutePath): string { $cwd = getcwd(); @@ -175,26 +142,4 @@ private function makeRelative(string $absolutePath): string return $absolutePath; // @codeCoverageIgnore } - - /** - * @param array> $diffLines - * @return list - */ - private function getChangedLinesForFile(string $absolutePath, array $diffLines): array - { - $normalized = str_replace('\\', '/', $absolutePath); - - foreach ($diffLines as $diffPath => $lines) { - $normalizedDiff = str_replace('\\', '/', $diffPath); - - if ( - $normalized === $normalizedDiff - || str_ends_with($normalized, '/' . ltrim($normalizedDiff, '/')) - ) { - return $lines; - } - } - - return []; // @codeCoverageIgnore - } } diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 8afed41..047c9bb 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -4,6 +4,7 @@ namespace DocbookCS\Tests\Unit\Diff; +use DocbookCS\Diff\Diff; use DocbookCS\Diff\DiffParser; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -22,7 +23,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 +40,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 +61,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 +78,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 +98,7 @@ public function itExcludesDeletedFiles(): void -line3 DIFF; - self::assertSame([], $this->parser->parse($diff)); + self::assertSame([], $this->lineNumbersByFile($this->parser->parse($diff))); } #[Test] @@ -114,7 +115,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 +141,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,9 +163,9 @@ 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). + // No lines added, so the changed set is empty self::assertArrayHasKey('file.xml', $result); self::assertSame([], $result['file.xml']); } @@ -183,7 +184,7 @@ public function itIgnoresTheMissingFinalNewlineMarker(): void +second DIFF; - $result = $this->parser->parse($diff); + $result = $this->lineNumbersByFile($this->parser->parse($diff)); self::assertSame([1, 2], $result['file.xml']); } @@ -207,7 +208,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']); } @@ -223,8 +224,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(Diff $diff): array + { + $lineNumbersByFile = []; + + foreach ($diff->fileChanges as $fileChange) { + $lineNumbersByFile[$fileChange->filePath] = $fileChange->addedLineNumbers; + } + + return $lineNumbersByFile; + } } diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index f6801a3..c04ff8b 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -6,6 +6,8 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; +use DocbookCS\Diff\Diff; +use DocbookCS\Diff\FileChange; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; @@ -236,8 +238,8 @@ public function itFiltersFilesToOnlyThoseInTheDiff(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $diffLines = ['sniff_runner/default/file_a.xml' => [1]]; - $report = $runner->run($config, null, $diffLines); + $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [1])]); + $report = $runner->run($config, null, $diff); self::assertSame(1, $report->getFilesScanned()); } @@ -248,8 +250,8 @@ public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $diffLines = ['completely/different/file.xml' => [1, 2, 3]]; - $report = $runner->run($config, null, $diffLines); + $diff = new Diff([new FileChange('completely/different/file.xml', [1, 2, 3])]); + $report = $runner->run($config, null, $diff); self::assertSame(0, $report->getFilesScanned()); } @@ -262,8 +264,8 @@ public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void $discoveredPath = self::FIXTURE_DIR . '/file_a.xml'; - $diffLines = [$discoveredPath => [1]]; - $report = $runner->run($config, null, $diffLines); + $diff = new Diff([new FileChange($discoveredPath, [1])]); + $report = $runner->run($config, null, $diff); self::assertSame(1, $report->getFilesScanned()); } @@ -309,8 +311,8 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $diffLines = ['sniff_runner/default/file_a.xml' => []]; - $report = $runner->run($config, null, $diffLines); + $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [])]); + $report = $runner->run($config, null, $diff); self::assertSame(1, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); From f3067befa65897c11118780fdb08b04566ba8d91 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 02:56:00 +0700 Subject: [PATCH 06/14] chore: restored original comment --- tests/Unit/Diff/DiffParserTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 047c9bb..937fac2 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -165,7 +165,7 @@ public function itIgnoresRemovedLines(): void $result = $this->lineNumbersByFile($this->parser->parse($diff)); - // No lines added, so the changed set is empty + // 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']); } From bfd6da72c203d8496518dcd54445f74925ed7500 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 03:08:35 +0700 Subject: [PATCH 07/14] test: declare diff value object usage --- tests/Unit/ApplicationTest.php | 5 +++++ tests/Unit/Diff/DiffParserTest.php | 4 ++++ tests/Unit/Runner/SniffRunnerTest.php | 3 +++ 3 files changed, 12 insertions(+) diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 6e24e79..99ddd46 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -6,7 +6,9 @@ use DocbookCS\Application; use DocbookCS\Config\ConfigData; +use DocbookCS\Diff\Diff; use DocbookCS\Diff\DiffParser; +use DocbookCS\Diff\FileChange; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; use DocbookCS\Config\SniffEntry; @@ -25,6 +27,7 @@ use DocbookCS\Sniff\ExceptionNameSniff; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[CoversClass(Application::class)] @@ -46,6 +49,8 @@ #[CoversClass(FileReport::class)] #[CoversClass(ExceptionNameSniff::class)] #[CoversClass(EntityResolver::class)] +#[UsesClass(Diff::class)] +#[UsesClass(FileChange::class)] final class ApplicationTest extends TestCase { private const string FIXTURE_DIR = __DIR__ . '/../fixtures/application'; diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 937fac2..39f55eb 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -6,11 +6,15 @@ use DocbookCS\Diff\Diff; 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)] +#[UsesClass(Diff::class)] +#[UsesClass(FileChange::class)] final class DiffParserTest extends TestCase { private DiffParser $parser; diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index c04ff8b..7b2b302 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -23,6 +23,7 @@ use DocbookCS\Sniff\SniffInterface; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[CoversClass(SniffRunner::class)] @@ -37,6 +38,8 @@ #[CoversClass(SniffEntry::class)] #[CoversClass(FileReport::class)] #[CoversClass(Violation::class)] +#[UsesClass(Diff::class)] +#[UsesClass(FileChange::class)] final class SniffRunnerTest extends TestCase { private const string FIXTURE_DIR = __DIR__ . '/../../fixtures/sniff_runner/default'; From edc283eb9db71a0c69a86deba874e9e9f37e16fe Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 04:40:00 +0700 Subject: [PATCH 08/14] test: group coverage attributes --- tests/Unit/ApplicationTest.php | 45 ++++++++++--------- tests/Unit/Config/ConfigParserTest.php | 10 +++-- tests/Unit/Config/SniffEntryTest.php | 4 +- tests/Unit/Diff/DiffParserTest.php | 9 ++-- tests/Unit/Path/EntityResolverTest.php | 7 ++- tests/Unit/Path/PathLoaderTest.php | 6 ++- tests/Unit/Path/PathMatcherTest.php | 4 +- tests/Unit/Progress/ConsoleProgressTest.php | 4 +- tests/Unit/Progress/NullProgressTest.php | 4 +- tests/Unit/Report/ReportTest.php | 8 ++-- .../Reporter/CheckstyleReporterTest.php | 10 +++-- .../Report/Reporter/ConsoleReporterTest.php | 10 +++-- .../Unit/Report/Reporter/JsonReporterTest.php | 10 +++-- tests/Unit/Runner/EntityPreprocessorTest.php | 4 +- tests/Unit/Runner/SniffRunnerTest.php | 31 +++++++------ tests/Unit/Runner/XmlFileProcessorTest.php | 12 ++--- tests/Unit/Sniff/AbstractSniffTest.php | 4 +- tests/Unit/Sniff/AttributeOrderSniffTest.php | 6 ++- tests/Unit/Sniff/ExceptionNameSniffTest.php | 6 ++- tests/Unit/Sniff/SimparaSniffTest.php | 6 ++- tests/Unit/Sniff/WhitespaceSniffTest.php | 6 ++- 21 files changed, 126 insertions(+), 80 deletions(-) diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 99ddd46..35eaa53 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -30,27 +30,30 @@ 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)] -#[UsesClass(Diff::class)] -#[UsesClass(FileChange::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(SniffEntry::class), + CoversClass(SniffRunner::class), + CoversClass(XmlFileProcessor::class), + // + UsesClass(Diff::class), + UsesClass(FileChange::class), +] final class ApplicationTest extends TestCase { private const string FIXTURE_DIR = __DIR__ . '/../fixtures/application'; diff --git a/tests/Unit/Config/ConfigParserTest.php b/tests/Unit/Config/ConfigParserTest.php index 721d5eb..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; diff --git a/tests/Unit/Config/SniffEntryTest.php b/tests/Unit/Config/SniffEntryTest.php index c5ae22b..f464340 100644 --- a/tests/Unit/Config/SniffEntryTest.php +++ b/tests/Unit/Config/SniffEntryTest.php @@ -9,7 +9,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(SniffEntry::class)] +#[ + CoversClass(SniffEntry::class), +] final class SniffEntryTest extends TestCase { #[Test] diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 39f55eb..4fa72f6 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -12,9 +12,12 @@ use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -#[CoversClass(DiffParser::class)] -#[UsesClass(Diff::class)] -#[UsesClass(FileChange::class)] +#[ + CoversClass(DiffParser::class), + // + UsesClass(Diff::class), + UsesClass(FileChange::class), +] final class DiffParserTest extends TestCase { private DiffParser $parser; 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..86eca0c 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -12,9 +12,11 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(Report::class)] -#[CoversClass(Violation::class)] -#[CoversClass(FileReport::class)] +#[ + CoversClass(FileReport::class), + CoversClass(Report::class), + CoversClass(Violation::class), +] final class ReportTest extends TestCase { private function createViolation( diff --git a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php index 91a46e8..87cbfb1 100644 --- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php +++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php @@ -13,10 +13,12 @@ use PHPUnit\Framework\Attributes\Test; 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), +] final class CheckstyleReporterTest extends TestCase { private CheckstyleReporter $reporter; diff --git a/tests/Unit/Report/Reporter/ConsoleReporterTest.php b/tests/Unit/Report/Reporter/ConsoleReporterTest.php index 1dd96b8..ad2ba58 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -13,10 +13,12 @@ use PHPUnit\Framework\Attributes\Test; 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), +] final class ConsoleReporterTest extends TestCase { private ConsoleReporter $reporter; diff --git a/tests/Unit/Report/Reporter/JsonReporterTest.php b/tests/Unit/Report/Reporter/JsonReporterTest.php index 1e488fa..3cbbe72 100644 --- a/tests/Unit/Report/Reporter/JsonReporterTest.php +++ b/tests/Unit/Report/Reporter/JsonReporterTest.php @@ -13,10 +13,12 @@ use PHPUnit\Framework\Attributes\Test; 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), +] final class JsonReporterTest extends TestCase { private JsonReporter $reporter; 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/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 7b2b302..1db646a 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -26,20 +26,23 @@ 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)] -#[UsesClass(Diff::class)] -#[UsesClass(FileChange::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(SniffEntry::class), + CoversClass(SniffRunner::class), + CoversClass(Violation::class), + CoversClass(XmlFileProcessor::class), + // + UsesClass(Diff::class), + UsesClass(FileChange::class), +] final class SniffRunnerTest extends TestCase { private const string FIXTURE_DIR = __DIR__ . '/../../fixtures/sniff_runner/default'; diff --git a/tests/Unit/Runner/XmlFileProcessorTest.php b/tests/Unit/Runner/XmlFileProcessorTest.php index 6bd60b4..901835a 100644 --- a/tests/Unit/Runner/XmlFileProcessorTest.php +++ b/tests/Unit/Runner/XmlFileProcessorTest.php @@ -15,11 +15,13 @@ use PHPUnit\Framework\Attributes\Test; 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(XmlFileProcessor::class), +] final class XmlFileProcessorTest extends TestCase { #[Test] diff --git a/tests/Unit/Sniff/AbstractSniffTest.php b/tests/Unit/Sniff/AbstractSniffTest.php index 26b9262..8c9c5a4 100644 --- a/tests/Unit/Sniff/AbstractSniffTest.php +++ b/tests/Unit/Sniff/AbstractSniffTest.php @@ -9,7 +9,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(AbstractSniff::class)] +#[ + CoversClass(AbstractSniff::class), +] final class AbstractSniffTest extends TestCase { private function createSniff(): AbstractSniff diff --git a/tests/Unit/Sniff/AttributeOrderSniffTest.php b/tests/Unit/Sniff/AttributeOrderSniffTest.php index 0673283..da2e993 100644 --- a/tests/Unit/Sniff/AttributeOrderSniffTest.php +++ b/tests/Unit/Sniff/AttributeOrderSniffTest.php @@ -10,8 +10,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(AttributeOrderSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(AttributeOrderSniff::class), + CoversClass(Violation::class), +] final class AttributeOrderSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument diff --git a/tests/Unit/Sniff/ExceptionNameSniffTest.php b/tests/Unit/Sniff/ExceptionNameSniffTest.php index de6600d..99ecd73 100644 --- a/tests/Unit/Sniff/ExceptionNameSniffTest.php +++ b/tests/Unit/Sniff/ExceptionNameSniffTest.php @@ -10,8 +10,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(ExceptionNameSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(ExceptionNameSniff::class), + CoversClass(Violation::class), +] final class ExceptionNameSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument diff --git a/tests/Unit/Sniff/SimparaSniffTest.php b/tests/Unit/Sniff/SimparaSniffTest.php index bb8cdde..d6c616a 100644 --- a/tests/Unit/Sniff/SimparaSniffTest.php +++ b/tests/Unit/Sniff/SimparaSniffTest.php @@ -10,8 +10,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(SimparaSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(SimparaSniff::class), + CoversClass(Violation::class), +] final class SimparaSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument diff --git a/tests/Unit/Sniff/WhitespaceSniffTest.php b/tests/Unit/Sniff/WhitespaceSniffTest.php index 17d81c0..54e5583 100644 --- a/tests/Unit/Sniff/WhitespaceSniffTest.php +++ b/tests/Unit/Sniff/WhitespaceSniffTest.php @@ -10,8 +10,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -#[CoversClass(WhitespaceSniff::class)] -#[CoversClass(Violation::class)] +#[ + CoversClass(Violation::class), + CoversClass(WhitespaceSniff::class), +] final class WhitespaceSniffTest extends TestCase { private function createDocument(string $xml): \DOMDocument From b080668ffe5183519752c094658845297be3b7ce Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 04:50:14 +0700 Subject: [PATCH 09/14] test: added coverage to test command --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6bd6998..ba5461a 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,7 @@ } }, "scripts": { - "test": "./vendor/bin/phpunit --no-coverage", + "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": [ From fcb8c679c5295fae74a8264eb36614eea194de08 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 05:44:05 +0700 Subject: [PATCH 10/14] fix: ignore expanded entity nodes in source-backed sniffs --- phpunit.xml.dist | 3 + src/Runner/EntityExpansionMarker.php | 63 ++++++++++++++++ src/Runner/EntityPreprocessor.php | 31 ++++++-- src/Runner/XmlFileProcessor.php | 2 +- src/Sniff/AbstractSniff.php | 6 ++ src/Sniff/ExceptionNameSniff.php | 4 ++ src/Sniff/SimparaSniff.php | 4 ++ .../Runner/EntityExpansionMarkerTest.php | 72 +++++++++++++++++++ .../Sniff/EntityExpandedSniffTest.php | 62 ++++++++++++++++ tests/Unit/Sniff/ExceptionNameSniffTest.php | 4 ++ tests/Unit/Sniff/SimparaSniffTest.php | 4 ++ 11 files changed, 247 insertions(+), 8 deletions(-) create mode 100644 src/Runner/EntityExpansionMarker.php create mode 100644 tests/Integration/Runner/EntityExpansionMarkerTest.php create mode 100644 tests/Integration/Sniff/EntityExpandedSniffTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f173b58..eaa454f 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,9 @@ tests/Unit + + tests/Integration + diff --git a/src/Runner/EntityExpansionMarker.php b/src/Runner/EntityExpansionMarker.php new file mode 100644 index 0000000..841fe67 --- /dev/null +++ b/src/Runner/EntityExpansionMarker.php @@ -0,0 +1,63 @@ +' + . $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..f7d80b8 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,10 +40,13 @@ 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 - if (str_starts_with($matches[0], '||<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/', + function (array $matches) use (&$changed, $markXmlExpansions): string { + 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/Sniff/EntityExpandedSniffTest.php b/tests/Integration/Sniff/EntityExpandedSniffTest.php new file mode 100644 index 0000000..a6b1925 --- /dev/null +++ b/tests/Integration/Sniff/EntityExpandedSniffTest.php @@ -0,0 +1,62 @@ +Source&expanded;'; + $document = $this->processedDocument($source, 'Expanded'); + + $violations = new SimparaSniff()->process($document, $source, 'file.xml'); + + self::assertCount(1, $violations); + self::assertSame(1, $violations[0]->line); + } + + #[Test] + public function exceptionNameIgnoresExpandedElements(): void + { + $source = 'RuntimeException&expanded;'; + $document = $this->processedDocument( + $source, + 'ExpandedException', + ); + + $violations = new ExceptionNameSniff()->process($document, $source, 'file.xml'); + + self::assertCount(1, $violations); + self::assertSame(1, $violations[0]->line); + } + + 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/Unit/Sniff/ExceptionNameSniffTest.php b/tests/Unit/Sniff/ExceptionNameSniffTest.php index 99ecd73..5e749c6 100644 --- a/tests/Unit/Sniff/ExceptionNameSniffTest.php +++ b/tests/Unit/Sniff/ExceptionNameSniffTest.php @@ -5,14 +5,18 @@ namespace DocbookCS\Tests\Unit\Sniff; use DocbookCS\Report\Violation; +use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Sniff\ExceptionNameSniff; 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), + // + UsesClass(EntityExpansionMarker::class), ] final class ExceptionNameSniffTest extends TestCase { diff --git a/tests/Unit/Sniff/SimparaSniffTest.php b/tests/Unit/Sniff/SimparaSniffTest.php index d6c616a..036d374 100644 --- a/tests/Unit/Sniff/SimparaSniffTest.php +++ b/tests/Unit/Sniff/SimparaSniffTest.php @@ -5,14 +5,18 @@ namespace DocbookCS\Tests\Unit\Sniff; use DocbookCS\Report\Violation; +use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Sniff\SimparaSniff; 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), + // + UsesClass(EntityExpansionMarker::class), ] final class SimparaSniffTest extends TestCase { From ce2dbcec4394308c0816679715335082dc796c44 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 05:55:27 +0700 Subject: [PATCH 11/14] refactor: reworked command flags, scopes, and diff detection --- README.md | 20 ++ bin/docbook-cs | 2 +- phpunit.xml.dist | 3 + src/Application.php | 153 ++++++-------- src/Diff/DiffParser.php | 16 +- src/Diff/DiffProviderInterface.php | 10 + src/Diff/FileChange.php | 6 +- src/Diff/GitDiffProvider.php | 92 +++++++++ src/Path/DiffPathLoader.php | 101 ++++++++++ src/Path/EntityResolver.php | 59 +++++- src/Process/NativeProcessRunner.php | 38 ++++ src/Process/ProcessResult.php | 15 ++ src/Process/ProcessRunnerInterface.php | 14 ++ src/Runner/RunPlan.php | 23 +++ src/Runner/RunPlanner.php | 79 ++++++++ src/Runner/RunScopeResolver.php | 186 ++++++++++++++++++ src/Runner/SniffRunner.php | 41 +--- tests/Feature/ApplicationInputTest.php | 99 ++++++++++ .../Integration/Diff/GitDiffProviderTest.php | 112 +++++++++++ tests/Integration/Path/DiffPathLoaderTest.php | 80 ++++++++ .../Process/NativeProcessRunnerTest.php | 33 ++++ .../Runner/RunScopeResolverTest.php | 125 ++++++++++++ tests/Unit/ApplicationTest.php | 128 ++---------- tests/Unit/Diff/DiffParserTest.php | 40 ++++ tests/Unit/Path/EntityResolverPathsTest.php | 38 ++++ tests/Unit/Runner/RunPlannerTest.php | 67 +++++++ tests/Unit/Runner/SniffRunnerTest.php | 51 +++-- 27 files changed, 1374 insertions(+), 257 deletions(-) create mode 100644 src/Diff/DiffProviderInterface.php create mode 100644 src/Diff/GitDiffProvider.php create mode 100644 src/Path/DiffPathLoader.php create mode 100644 src/Process/NativeProcessRunner.php create mode 100644 src/Process/ProcessResult.php create mode 100644 src/Process/ProcessRunnerInterface.php create mode 100644 src/Runner/RunPlan.php create mode 100644 src/Runner/RunPlanner.php create mode 100644 src/Runner/RunScopeResolver.php create mode 100644 tests/Feature/ApplicationInputTest.php create mode 100644 tests/Integration/Diff/GitDiffProviderTest.php create mode 100644 tests/Integration/Path/DiffPathLoaderTest.php create mode 100644 tests/Integration/Process/NativeProcessRunnerTest.php create mode 100644 tests/Integration/Runner/RunScopeResolverTest.php create mode 100644 tests/Unit/Path/EntityResolverPathsTest.php create mode 100644 tests/Unit/Runner/RunPlannerTest.php diff --git a/README.md b/README.md index 5ee464c..4be5799 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,26 @@ Register it in your config: ``` +## CLI Scope + +By default, DocbookCS checks the current Git diff from its upstream branch point +through the working tree. Alternatively, a unified diff can be piped or file and +directory paths passed. The inspection scope is limited to the given diff or the +full contents of the given file paths. + +XML references are expanded by default, but reported violations remain limited +to the given scope. With `--wide`, every file inferred from paths or a diff is +checked as a whole, and referenced `SYSTEM` XML files are recursively included. + +| Input | `--wide` | Full File(s) | References | +|------------|---------:|-------------:|-----------:| +| none | no | no | no | +| none | yes | yes | yes | +| path | no | yes | no | +| path | yes | yes | yes | +| piped diff | no | no | no | +| piped diff | yes | yes | yes | + ## License Apache 2.0 diff --git a/bin/docbook-cs b/bin/docbook-cs index 45e5abf..4b72e67 100644 --- a/bin/docbook-cs +++ b/bin/docbook-cs @@ -25,4 +25,4 @@ use DocbookCS\Application; exit(2); })(); -new Application($argv ?? [])->run(); +exit(Application::fromGlobals($argv ?? [])->run()); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index eaa454f..540a1a7 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -21,6 +21,9 @@ tests/Integration + + tests/Feature + diff --git a/src/Application.php b/src/Application.php index 9827b56..bb36f76 100644 --- a/src/Application.php +++ b/src/Application.php @@ -7,7 +7,6 @@ use DocbookCS\Config\ConfigData; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; -use DocbookCS\Diff\DiffParser; use DocbookCS\Progress\ConsoleProgress; use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; @@ -15,6 +14,7 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Report\Reporter\ReporterInterface; +use DocbookCS\Runner\RunPlanner; use DocbookCS\Runner\SniffRunner; final class Application @@ -32,21 +32,50 @@ final class Application /** @var resource */ private $stderr; - /** @var resource */ - private $stdin; + private ?string $stdin; + + /** + * @param list $argv + * @throws \RuntimeException if redirected stdin cannot be read. + * @api + */ + public static function fromGlobals(array $argv): self + { + $stdin = null; + $stat = fstat(STDIN); + + if ($stat === false) { + return new self($argv, stdin: $stdin); + } + + $type = $stat['mode'] & 0170000; + + if ($type === 0010000 || $type === 0100000) { + $stdin = stream_get_contents(STDIN); + + if ($stdin === false) { + throw new \RuntimeException('Could not read diff from stdin.'); + } + } + + return new self($argv, stdin: $stdin); + } /** * @param list $argv * @param ?resource $stdout * @param ?resource $stderr - * @param ?resource $stdin */ - public function __construct(array $argv, mixed $stdout = null, mixed $stderr = null, mixed $stdin = null) - { + public function __construct( + array $argv, + mixed $stdout = null, + mixed $stderr = null, + ?string $stdin = null, + ) { $this->argv = $argv; $this->stdout = $stdout ?? STDOUT; $this->stderr = $stderr ?? STDERR; - $this->stdin = $stdin ?? STDIN; + $this->stdin = $stdin; } /** @@ -54,7 +83,13 @@ public function __construct(array $argv, mixed $stdout = null, mixed $stderr = n */ public function run(): int { - $options = $this->parseArgv(); + try { + $options = $this->parseArgv(); + } catch (\InvalidArgumentException $e) { + $this->writeError('Error: ' . $e->getMessage() . PHP_EOL); + + return 2; + } if ($options['help']) { $this->printHelp(); @@ -76,31 +111,19 @@ public function run(): int return 2; } - $overridePaths = $options['paths'] !== [] ? $options['paths'] : null; - - // If override paths are relative, resolve them against cwd. - if ($overridePaths !== null) { - $overridePaths = $this->resolveOverridePaths($overridePaths); - } - - $diff = null; - - if ($options['diff'] !== null) { - try { - $diffContent = $this->readDiff($options['diff']); - $diff = (new DiffParser())->parse($diffContent); - } catch (\Throwable $e) { - $this->writeError('Error reading diff: ' . $e->getMessage() . PHP_EOL); + try { + $runPlan = new RunPlanner($config, $options['wide'])->plan($options['paths'], $this->stdin); + } catch (\Throwable $e) { + $this->writeError('Error resolving input: ' . $e->getMessage() . PHP_EOL); - return 2; - } + return 2; } $progress = $this->createProgress($options); try { $runner = new SniffRunner($progress); - $report = $runner->run($config, $overridePaths, $diff); + $report = $runner->run($runPlan); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); @@ -118,27 +141,6 @@ public function run(): int return (int) $report->hasViolations(); } - /** - * @param list $paths - * @return list - */ - private function resolveOverridePaths(array $paths): array - { - $cwd = getcwd() ?: '.'; - $resolved = []; - - foreach ($paths as $path) { - if (str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $path)) { - $resolved[] = $path; - continue; - } - - $resolved[] = $cwd . '/' . $path; - } - - return $resolved; - } - /** * @return array{ * help: bool, @@ -148,9 +150,10 @@ private function resolveOverridePaths(array $paths): array * colors: bool, * quiet: bool, * paths: list, - * diff: string|null, + * wide: bool, * perf: bool, * } + * @throws \InvalidArgumentException for unsupported options. */ private function parseArgv(): array { @@ -162,7 +165,7 @@ private function parseArgv(): array 'colors' => $this->detectColorSupport(), 'quiet' => false, 'paths' => [], - 'diff' => null, + 'wide' => false, 'perf' => false, ]; @@ -227,23 +230,14 @@ private function parseArgv(): array continue; } - // --diff = read from stdin - // --diff=FILE = read from file - // --diff=- = read from stdin (explicit) - if ($arg === '--diff') { - $result['diff'] = ''; - $i++; - continue; - } - - if (str_starts_with($arg, '--diff=')) { - $result['diff'] = substr($arg, 7); + if ($arg === '--perf') { + $result['perf'] = true; $i++; continue; } - if ($arg === '--perf') { - $result['perf'] = true; + if ($arg === '--wide') { + $result['wide'] = true; $i++; continue; } @@ -251,33 +245,16 @@ private function parseArgv(): array // Anything else is a path to scan. if (!str_starts_with($arg, '-')) { $result['paths'][] = $arg; + $i++; + continue; } - $i++; + throw new \InvalidArgumentException(sprintf('Unknown option: %s', $arg)); } return $result; } - /** @throws \RuntimeException if the source cannot be read. */ - private function readDiff(string $source): string - { - if ($source === '' || $source === '-') { - $content = stream_get_contents($this->stdin); - if ($content === false) { - throw new \RuntimeException('Could not read diff from stdin.'); // @codeCoverageIgnore - } - return $content; - } - - $content = @file_get_contents($source); - if ($content === false) { - throw new \RuntimeException(sprintf('Could not read diff file: %s', $source)); - } - - return $content; - } - /** @param array{report: string, quiet: bool, colors: bool} $options */ private function createProgress(array $options): ProgressInterface { @@ -382,22 +359,20 @@ private function printHelp(): void --report= Output format: console (default), checkstyle, json. --colors Force ANSI color output. --no-colors Disable ANSI color output. - --diff[=] Restrict analysis to files changed in a unified diff. - Omit the value or pass "-" to read the diff from stdin. - Violations are only reported when the violating element - is on or contains a changed line (parent-context aware). + --wide Check whole selected files and recursively include + referenced XML files. Arguments: One or more files or directories to scan. - If omitted, the paths from the config file are used. + Paths cannot be combined with diff input. Examples: docbook-cs docbook-cs --config=myconfig.xml reference/ docbook-cs --report=checkstyle --no-colors > report.xml docbook-cs reference/strings/functions/strlen.xml - git diff HEAD | docbook-cs --diff --report=checkstyle - docbook-cs --diff=changes.patch --report=json + git diff HEAD | docbook-cs + git diff HEAD | docbook-cs --wide --report=checkstyle HELP; diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 9dbd2fe..0c62f3a 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -12,12 +12,15 @@ public function parse(string $diff): Diff { /** @var array> $changedLinesByFile */ $changedLinesByFile = []; + /** @var array> $deletionAnchorsByFile */ + $deletionAnchorsByFile = []; $currentFile = null; $deleted = false; $newLineNumber = 0; $oldLinesRemaining = 0; $newLinesRemaining = 0; $inHunk = false; + $previousLineWasDeletion = false; foreach (explode("\n", $diff) as $line) { if (str_starts_with($line, 'diff --git ')) { @@ -25,6 +28,7 @@ public function parse(string $diff): Diff $deleted = false; $newLineNumber = 0; $inHunk = false; + $previousLineWasDeletion = false; continue; } @@ -43,6 +47,7 @@ public function parse(string $diff): Diff $inHunk = false; if ($currentFile !== null && !isset($changedLinesByFile[$currentFile])) { $changedLinesByFile[$currentFile] = []; + $deletionAnchorsByFile[$currentFile] = []; } continue; } @@ -58,6 +63,7 @@ public function parse(string $diff): Diff $newLineNumber = (int) $m[2]; $newLinesRemaining = isset($m[3]) ? (int) $m[3] : 1; $inHunk = true; + $previousLineWasDeletion = false; } continue; } @@ -70,24 +76,32 @@ public function parse(string $diff): Diff $changedLinesByFile[$currentFile][] = $newLineNumber; $newLineNumber++; $newLinesRemaining--; + $previousLineWasDeletion = false; } elseif (str_starts_with($line, '-')) { + if (!$previousLineWasDeletion) { + $deletionAnchorsByFile[$currentFile][] = max(1, $newLineNumber); + } + $oldLinesRemaining--; + $previousLineWasDeletion = true; } elseif (str_starts_with($line, ' ')) { // Context line — present in both old and new file. $newLineNumber++; $oldLinesRemaining--; $newLinesRemaining--; + $previousLineWasDeletion = false; } if ($oldLinesRemaining === 0 && $newLinesRemaining === 0) { $inHunk = false; + $previousLineWasDeletion = false; } } $fileChanges = []; foreach ($changedLinesByFile as $filePath => $lineNumbers) { - $fileChanges[] = new FileChange($filePath, $lineNumbers); + $fileChanges[] = new FileChange($filePath, $lineNumbers, $deletionAnchorsByFile[$filePath]); } return new Diff($fileChanges); diff --git a/src/Diff/DiffProviderInterface.php b/src/Diff/DiffProviderInterface.php new file mode 100644 index 0000000..418a7b4 --- /dev/null +++ b/src/Diff/DiffProviderInterface.php @@ -0,0 +1,10 @@ + $addedLineNumbers */ + /** + * @param list $addedLineNumbers + * @param list $deletionAnchors + */ public function __construct( public string $filePath, public array $addedLineNumbers, + public array $deletionAnchors = [], ) { } } diff --git a/src/Diff/GitDiffProvider.php b/src/Diff/GitDiffProvider.php new file mode 100644 index 0000000..3e7c146 --- /dev/null +++ b/src/Diff/GitDiffProvider.php @@ -0,0 +1,92 @@ +runOrThrow( + ['git', 'rev-parse', '--show-toplevel'], + $workingDirectory, + 'Could not find Git repository.', + )); + + $baseReference = $this->resolveBaseReference($repositoryRoot); + $mergeBase = $this->runOrThrow( + ['git', 'merge-base', 'HEAD', $baseReference], + $repositoryRoot, + sprintf('Unclear where HEAD branched from %s.', $baseReference), + ); + + return $this->runOrThrow( + ['git', 'diff', '--no-ext-diff', '--no-color', trim($mergeBase), '--'], + $repositoryRoot, + 'Could not read diff.', + ); + } + + /** @throws \RuntimeException if no default branch reference exists. */ + private function resolveBaseReference(string $repositoryRoot): string + { + $candidates = []; + + foreach (['upstream', 'origin'] as $remote) { + $result = $this->processRunner->run( + ['git', 'symbolic-ref', '--quiet', sprintf('refs/remotes/%s/HEAD', $remote)], + $repositoryRoot, + ); + + if ($result->exitCode === 0) { + $candidates[] = trim($result->stdout); + } + + $candidates[] = sprintf('refs/remotes/%s/main', $remote); + $candidates[] = sprintf('refs/remotes/%s/master', $remote); + } + + $candidates[] = 'refs/heads/main'; + $candidates[] = 'refs/heads/master'; + + foreach (array_unique($candidates) as $candidate) { + $result = $this->processRunner->run( + ['git', 'rev-parse', '--verify', '--quiet', $candidate . '^{commit}'], + $repositoryRoot, + ); + + if ($result->exitCode === 0) { + return $candidate; + } + } + + throw new \RuntimeException('Could not determine the upstream default branch for the contribution diff.'); + } + + /** + * @param list $command + * @throws \RuntimeException if the command fails. + */ + private function runOrThrow(array $command, string $workingDirectory, string $error): string + { + $result = $this->processRunner->run($command, $workingDirectory); + + if ($result->exitCode === 0) { + return $result->stdout; + } + + $detail = trim($result->stderr); + + throw new \RuntimeException($detail !== '' ? "$error $detail" : $error); + } +} diff --git a/src/Path/DiffPathLoader.php b/src/Path/DiffPathLoader.php new file mode 100644 index 0000000..22706e0 --- /dev/null +++ b/src/Path/DiffPathLoader.php @@ -0,0 +1,101 @@ + $projectRoots */ + public function __construct( + private Diff $diff, + private string $workingDirectory, + private string $basePath, + private array $projectRoots, + private PathMatcher $matcher, + ) { + } + + public function load(): Diff + { + $changes = []; + + foreach ($this->diff->fileChanges as $fileChange) { + foreach ($this->candidates($fileChange->filePath) as $candidate) { + if ( + is_file($candidate) + && str_ends_with(strtolower($candidate), '.xml') + && $this->matcher->isIncluded($candidate) + ) { + $changes[$candidate] = new FileChange( + $candidate, + $fileChange->addedLineNumbers, + $fileChange->deletionAnchors, + ); + break; + } + } + } + + ksort($changes); + + return new Diff(array_values($changes)); + } + + /** @return list */ + private function candidates(string $path): array + { + $path = str_replace('\\', '/', $path); + + if ($this->isAbsolute($path)) { + return [$path]; + } + + $candidates = [ + $this->workingDirectory . '/' . $path, + $this->basePath . '/' . $path, + ]; + + foreach ($this->projectRoots as $root => $directory) { + $candidates[] = $root . '/' . $path; + + $prefix = trim(str_replace('\\', '/', $directory), '/') . '/'; + if ($prefix !== '/' && str_starts_with($path, $prefix)) { + $candidates[] = $root . '/' . substr($path, strlen($prefix)); + } + } + + return array_map($this->normalize(...), $candidates) + |> array_unique(...) + |> array_values(...); + } + + private function isAbsolute(string $path): bool + { + return str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:/#', $path) === 1; + } + + private function normalize(string $path): string + { + $prefix = str_starts_with($path, '/') ? '/' : ''; + $segments = []; + + foreach (explode('/', str_replace('\\', '/', $path)) as $segment) { + if ($segment === '' || $segment === '.') { + continue; + } + + if ($segment === '..' && $segments !== [] && end($segments) !== '..') { + array_pop($segments); + continue; + } + + $segments[] = $segment; + } + + return $prefix . implode('/', $segments); + } +} diff --git a/src/Path/EntityResolver.php b/src/Path/EntityResolver.php index 64d2200..8dc3f29 100644 --- a/src/Path/EntityResolver.php +++ b/src/Path/EntityResolver.php @@ -8,6 +8,12 @@ final class EntityResolver { private string $extension; + /** @var array|null */ + private ?array $resolvedEntities = null; + + /** @var array|null */ + private ?array $resolvedPaths = null; + /** * @param array $projectRoots * @param list $entityPaths @@ -26,15 +32,41 @@ public function __construct( */ public function resolve(): array { + $this->resolveAll(); + + return $this->resolvedEntities ?? []; + } + + /** + * @return array + * @throws \UnexpectedValueException if the directory cannot be read. + */ + public function paths(): array + { + $this->resolveAll(); + + return $this->resolvedPaths ?? []; + } + + /** @throws \UnexpectedValueException if the directory cannot be read. */ + private function resolveAll(): void + { + if ($this->resolvedEntities !== null && $this->resolvedPaths !== null) { + return; + } + $entities = []; + $paths = []; foreach ($this->entityPaths as $path) { foreach ($this->getEntityFiles($path) as $file) { - $entities += $this->resolveFile($file); + $visited = []; + $entities += $this->resolveFile($file, $visited, $paths); } } - return $entities; + $this->resolvedEntities = $entities; + $this->resolvedPaths = $paths; } /** @@ -86,11 +118,13 @@ private function scanDirectory(string $directory): array /** * @param array $visited + * @param array $paths * @return array */ private function resolveFile( string $filePath, - array &$visited = [], + array &$visited, + array &$paths, ?string $originEntity = null ): array { if (isset($visited[$filePath]) || !is_readable($filePath)) { @@ -105,7 +139,7 @@ private function resolveFile( return []; // @codeCoverageIgnore } - $entities = $this->extractEntities($content, $filePath, $visited); + $entities = $this->extractEntities($content, $filePath, $visited, $paths); if ($originEntity !== null) { $entities[$originEntity] = $this->normalize($content); @@ -116,26 +150,30 @@ private function resolveFile( /** * @param array $visited + * @param array $paths * @return array */ private function extractEntities( string $content, string $filePath, - array &$visited + array &$visited, + array &$paths, ): array { return - $this->extractDtdEntities($content, $filePath, $visited) + $this->extractDtdEntities($content, $filePath, $visited, $paths) + $this->extractXmlEntities($content); } /** * @param array $visited + * @param array $paths * @return array */ private function extractDtdEntities( string $content, string $filePath, - array &$visited + array &$visited, + array &$paths, ): array { $result = []; @@ -161,7 +199,12 @@ private function extractDtdEntities( if ($type === 'SYSTEM') { $resolvedPath = $this->resolvePath($filePath, $value); - $result += $this->resolveFile($resolvedPath, $visited, $name); + + if (is_readable($resolvedPath)) { + $paths[$name] ??= $resolvedPath; + } + + $result += $this->resolveFile($resolvedPath, $visited, $paths, $name); continue; } diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php new file mode 100644 index 0000000..dce353c --- /dev/null +++ b/src/Process/NativeProcessRunner.php @@ -0,0 +1,38 @@ + $command + * @throws \RuntimeException if the process cannot be started. + */ + public function run(array $command, string $workingDirectory): ProcessResult; +} diff --git a/src/Runner/RunPlan.php b/src/Runner/RunPlan.php new file mode 100644 index 0000000..71917e1 --- /dev/null +++ b/src/Runner/RunPlan.php @@ -0,0 +1,23 @@ + $sniffs + * @param array $targets + * @param array $entities + */ + public function __construct( + public array $sniffs, + public array $targets, + public array $entities, + ) { + } +} diff --git a/src/Runner/RunPlanner.php b/src/Runner/RunPlanner.php new file mode 100644 index 0000000..b17c3cf --- /dev/null +++ b/src/Runner/RunPlanner.php @@ -0,0 +1,79 @@ +diffProvider = $diffProvider ?? new GitDiffProvider(); + $this->entityResolver = new EntityResolver( + $config->getProjectRoots(), + $config->getEntityPaths(), + ); + } + + /** + * @param list $paths + * @throws \InvalidArgumentException if paths and a piped diff are both provided. + * @throws \RuntimeException if the contribution diff cannot be determined. + * @throws \UnexpectedValueException if an entity or selected directory cannot be read. + */ + public function plan(array $paths, ?string $pipedDiff): RunPlan + { + if ($paths === []) { + return $this->planDiff(new DiffParser()->parse($pipedDiff ?? $this->diffProvider->for(getcwd() ?: '.'))); + } + + if ($pipedDiff !== null) { + throw new \InvalidArgumentException('Paths cannot be combined with diff input.'); + } + + return $this->planPaths($paths); + } + + /** + * @param list $paths + * @throws \UnexpectedValueException if an entity or selected directory cannot be read. + */ + public function planPaths(array $paths): RunPlan + { + return new RunPlan( + sniffs: $this->config->getSniffs(), + targets: $this->scopeResolver()->resolvePaths($paths), + entities: $this->entityResolver->resolve(), + ); + } + + /** @throws \UnexpectedValueException if an entity directory cannot be read. */ + public function planDiff(Diff $diff): RunPlan + { + return new RunPlan( + sniffs: $this->config->getSniffs(), + targets: $this->scopeResolver()->resolveDiff($diff), + entities: $this->entityResolver->resolve(), + ); + } + + /** @throws \UnexpectedValueException if an entity directory cannot be read. */ + private function scopeResolver(): RunScopeResolver + { + return new RunScopeResolver($this->config, $this->entityResolver->paths(), $this->wide); + } +} diff --git a/src/Runner/RunScopeResolver.php b/src/Runner/RunScopeResolver.php new file mode 100644 index 0000000..cd47f5d --- /dev/null +++ b/src/Runner/RunScopeResolver.php @@ -0,0 +1,186 @@ + $entityPaths */ + public function __construct( + private ConfigData $config, + private array $entityPaths, + private bool $wide = false, + ) { + $this->pathMatcher = new PathMatcher( + $config->getBasePath(), + $config->getExcludePatterns(), + ); + } + + /** + * @param list $paths + * @return array + * @throws \UnexpectedValueException if a selected directory cannot be read. + */ + public function resolvePaths(array $paths): array + { + $targets = []; + + foreach (new PathLoader($this->absolutePaths($paths), $this->pathMatcher)->loadPaths() as $file) { + $targets[$file] = null; + } + + return $this->finalize($targets); + } + + /** @return array */ + public function resolveDiff(Diff $diff): array + { + $resolvedDiff = new DiffPathLoader( + $diff, + getcwd() ?: '.', + $this->config->getBasePath(), + $this->config->getProjectRoots(), + $this->pathMatcher, + )->load(); + + $targets = []; + + foreach ($resolvedDiff->fileChanges as $fileChange) { + $targets[$fileChange->filePath] = $fileChange; + } + + return $this->finalize($targets); + } + + /** + * @param array $targets + * @return array + */ + private function finalize(array $targets): array + { + if ($this->wide) { + $targets = array_fill_keys(array_keys($targets), null); + $this->expandReferencedTargets($targets); + } + + ksort($targets); + + return $targets; + } + + /** + * @param list $paths + * @return list + */ + private function absolutePaths(array $paths): array + { + $workingDirectory = getcwd() ?: '.'; + $absolutePaths = []; + + foreach ($paths as $path) { + if (str_starts_with($path, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $path)) { + $absolutePaths[] = $path; + continue; + } + + $absolutePaths[] = $workingDirectory . '/' . $path; + } + + return $absolutePaths; + } + + /** @param array $targets */ + private function expandReferencedTargets(array &$targets): void + { + $pending = array_keys($targets); + $visitedFiles = []; + $visitedEntityPaths = []; + + for ($i = 0; isset($pending[$i]); $i++) { + $file = $pending[$i]; + + if (isset($visitedFiles[$file])) { + continue; + } + + $visitedFiles[$file] = true; + $content = @file_get_contents($file); + + if ($content === false) { + continue; + } + + foreach ($this->targetFilesFromContent($content, $visitedEntityPaths) as $targetFile) { + if (array_key_exists($targetFile, $targets)) { + continue; + } + + $targets[$targetFile] = null; + $pending[] = $targetFile; + } + } + } + + /** + * @param array $visitedEntityPaths + * @return list + */ + private function targetFilesFromContent(string $content, array &$visitedEntityPaths): array + { + if (!preg_match_all(self::ENTITY_PATTERN, $content, $matches)) { + return []; + } + + $files = []; + + foreach ($matches[1] as $name) { + if (!isset($this->entityPaths[$name])) { + continue; + } + + foreach ($this->expandEntityPath($this->entityPaths[$name], $visitedEntityPaths) as $file) { + $files[$file] = true; + } + } + + return array_keys($files); + } + + /** + * @param array $visited + * @return list + */ + private function expandEntityPath(string $path, array &$visited): array + { + $path = str_replace('\\', '/', $path); + + if (isset($visited[$path]) || !is_file($path)) { + return []; + } + + $visited[$path] = true; + + if (str_ends_with($path, '.xml')) { + return $this->pathMatcher->isIncluded($path) ? [$path] : []; + } + + $content = @file_get_contents($path); + + return $content !== false + ? $this->targetFilesFromContent($content, $visited) + : []; + } +} diff --git a/src/Runner/SniffRunner.php b/src/Runner/SniffRunner.php index 2884155..923919b 100644 --- a/src/Runner/SniffRunner.php +++ b/src/Runner/SniffRunner.php @@ -4,12 +4,7 @@ namespace DocbookCS\Runner; -use DocbookCS\Config\ConfigData; use DocbookCS\Config\SniffEntry; -use DocbookCS\Diff\Diff; -use DocbookCS\Path\EntityResolver; -use DocbookCS\Path\PathLoader; -use DocbookCS\Path\PathMatcher; use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; use DocbookCS\Report\Report; @@ -25,45 +20,29 @@ public function __construct(?ProgressInterface $progress = null) } /** - * @param list|null $overridePaths * @throws \RuntimeException if a sniff class cannot be found or does not implement SniffInterface. - * @throws \UnexpectedValueException if no files are found to scan. */ - public function run(ConfigData $config, ?array $overridePaths = null, ?Diff $diff = null): Report + public function run(RunPlan $plan): Report { $startTime = microtime(true); - $sniffs = $this->instantiateSniffs($config->getSniffs()); - - $matcher = new PathMatcher($config->getBasePath(), $config->getExcludePatterns()); - - $includePaths = $overridePaths ?? $config->getIncludePaths(); - - $entityResolver = new EntityResolver($config->getProjectRoots(), $config->getEntityPaths()); - $entities = $entityResolver->resolve(); - - $pathLoader = new PathLoader($includePaths, $matcher); - $files = $pathLoader->loadPaths(); - - if ($diff !== null) { - $files = array_values(array_filter( - $files, - static fn(string $file): bool => $diff->changeFor($file) !== null, - )); - } + $sniffs = $this->instantiateSniffs($plan->sniffs); $report = new Report(); - $preprocessor = new EntityPreprocessor($entities); + $preprocessor = new EntityPreprocessor($plan->entities); $processor = new XmlFileProcessor($sniffs, $preprocessor, $report); - $total = count($files); + $total = count($plan->targets); $this->progress->start($total); - foreach ($files as $index => $file) { + $index = 0; + foreach ($plan->targets as $file => $fileChange) { $report->incrementFilesScanned(); - $changedLines = $diff?->changeFor($file)?->addedLineNumbers; + $changedLines = $fileChange !== null + ? array_values(array_unique([...$fileChange->addedLineNumbers, ...$fileChange->deletionAnchors])) + : null; $fileReport = $processor->processFile( $file, @@ -77,7 +56,7 @@ public function run(ConfigData $config, ?array $overridePaths = null, ?Diff $dif $report->addFileReport($fileReport); } - $this->progress->advance($index + 1, $file, $violationCount); + $this->progress->advance(++$index, $file, $violationCount); } $this->progress->finish(); diff --git a/tests/Feature/ApplicationInputTest.php b/tests/Feature/ApplicationInputTest.php new file mode 100644 index 0000000..a68ac9a --- /dev/null +++ b/tests/Feature/ApplicationInputTest.php @@ -0,0 +1,99 @@ +stdout = $stdout; + $this->stderr = $stderr; + } + + #[Test] + public function itRejectsTheRemovedDiffOption(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff'], + $this->stdout, + $this->stderr, + ); + + self::assertSame(2, $app->run()); + self::assertStringContainsString('Unknown option: --diff', $this->readStream($this->stderr)); + } + + #[Test] + public function itDetectsAPipedDiffWithoutAFlag(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG], + $this->stdout, + $this->stderr, + stdin: '', + ); + + self::assertSame(0, $app->run()); + self::assertSame('', $this->readStream($this->stderr)); + } + + #[Test] + public function itRejectsPathsCombinedWithAPipedDiff(): void + { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG, self::SCAN_FILE], + $this->stdout, + $this->stderr, + stdin: '', + ); + + self::assertSame(2, $app->run()); + self::assertStringContainsString('Paths cannot be combined with diff input', $this->readStream($this->stderr)); + } + + #[Test] + public function itIncludesTheWideOptionInHelp(): void + { + $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); + + $app->run(); + + $output = $this->readStream($this->stdout); + + self::assertStringContainsString('--wide', $output); + self::assertStringNotContainsString('--diff', $output); + } + + /** @param resource $stream */ + private function readStream(mixed $stream): string + { + rewind($stream); + + return stream_get_contents($stream) ?: ''; + } +} diff --git a/tests/Integration/Diff/GitDiffProviderTest.php b/tests/Integration/Diff/GitDiffProviderTest.php new file mode 100644 index 0000000..3cf7dc9 --- /dev/null +++ b/tests/Integration/Diff/GitDiffProviderTest.php @@ -0,0 +1,112 @@ +processRunner = new NativeProcessRunner(); + + $tmpDir = sys_get_temp_dir() . '/docbook-cs-git-diff-' . bin2hex(random_bytes(6)); + mkdir($tmpDir); + $this->repository = $tmpDir; + } + + protected function tearDown(): void + { + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($this->repository, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST, + ); + + foreach ($files as $file) { + if (!$file instanceof \SplFileInfo) { + throw new \UnexpectedValueException('Unexpected directory entry.'); + } + + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + + rmdir($this->repository); + } + + #[Test] + public function itDiffsTheWorkingTreeFromTheUpstreamBranchPoint(): void + { + $this->git('init', '--quiet', '--initial-branch=main'); + $this->configureAuthor(); + + file_put_contents($this->repository . '/base.xml', "base\n"); + $this->git('add', 'base.xml'); + $this->git('commit', '--quiet', '-m', 'Base'); + + $base = $this->git('rev-parse', 'HEAD'); + $this->git('update-ref', 'refs/remotes/upstream/main', $base); + $this->git('symbolic-ref', 'refs/remotes/upstream/HEAD', 'refs/remotes/upstream/main'); + $this->git('switch', '--quiet', '-c', 'contribution'); + $this->git('branch', '--delete', '--force', 'main'); + + file_put_contents($this->repository . '/committed.xml', "committed\n"); + $this->git('add', 'committed.xml'); + $this->git('commit', '--quiet', '-m', 'Contribution'); + file_put_contents($this->repository . '/base.xml', "working tree\n"); + + $diff = new GitDiffProvider($this->processRunner)->for($this->repository); + + self::assertStringContainsString('diff --git a/base.xml b/base.xml', $diff); + self::assertStringContainsString('+working tree', $diff); + self::assertStringContainsString('diff --git a/committed.xml b/committed.xml', $diff); + self::assertStringContainsString('+committed', $diff); + } + + #[Test] + public function itFailsClearlyWhenNoUpstreamDefaultBranchCanBeFound(): void + { + $this->git('init', '--quiet', '--initial-branch=contribution'); + $this->configureAuthor(); + + file_put_contents($this->repository . '/base.xml', "base\n"); + $this->git('add', 'base.xml'); + $this->git('commit', '--quiet', '-m', 'Contribution'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageIsOrContains('Could not determine the upstream default branch'); + + new GitDiffProvider($this->processRunner)->for($this->repository); + } + + private function configureAuthor(): void + { + $this->git('config', 'user.name', 'DocbookCS Tests'); + $this->git('config', 'user.email', 'docbook-cs@example.invalid'); + } + + private function git(string ...$arguments): string + { + $result = $this->processRunner->run(array_values(['git', ...$arguments]), $this->repository); + + self::assertSame(0, $result->exitCode, $result->stderr); + + return trim($result->stdout); + } +} diff --git a/tests/Integration/Path/DiffPathLoaderTest.php b/tests/Integration/Path/DiffPathLoaderTest.php new file mode 100644 index 0000000..47ba2e4 --- /dev/null +++ b/tests/Integration/Path/DiffPathLoaderTest.php @@ -0,0 +1,80 @@ +directory = sys_get_temp_dir() . '/docbook-cs-diff-path-' . bin2hex(random_bytes(6)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + @unlink($this->directory . '/chapter.xml'); + @unlink($this->directory . '/excluded.xml'); + @rmdir($this->directory); + } + + #[Test] + public function itLoadsChangedXmlFilesWithoutScanningConfiguredPaths(): void + { + $file = $this->directory . '/chapter.xml'; + file_put_contents($file, ''); + + $loader = new DiffPathLoader( + new Diff([new FileChange('docs/chapter.xml', [1])]), + workingDirectory: dirname($this->directory), + basePath: $this->directory, + projectRoots: [$this->directory => 'docs'], + matcher: new PathMatcher($this->directory, []), + ); + + $change = $loader->load()->changeFor($file); + + self::assertNotNull($change); + self::assertSame([1], $change->addedLineNumbers); + } + + #[Test] + public function itIgnoresMissingNonXmlAndExcludedFiles(): void + { + $excluded = $this->directory . '/excluded.xml'; + file_put_contents($excluded, ''); + + $loader = new DiffPathLoader( + new Diff([ + new FileChange('excluded.xml', [1]), + new FileChange('notes.txt', [1]), + new FileChange('missing.xml', [1]), + ]), + workingDirectory: $this->directory, + basePath: $this->directory, + projectRoots: [], + matcher: new PathMatcher($this->directory, ['excluded.xml']), + ); + + self::assertSame([], $loader->load()->fileChanges); + } +} diff --git a/tests/Integration/Process/NativeProcessRunnerTest.php b/tests/Integration/Process/NativeProcessRunnerTest.php new file mode 100644 index 0000000..9b930fb --- /dev/null +++ b/tests/Integration/Process/NativeProcessRunnerTest.php @@ -0,0 +1,33 @@ +run( + [PHP_BINARY, '-r', 'fwrite(STDOUT, "out"); fwrite(STDERR, "err"); exit(7);'], + getcwd() ?: '.', + ); + + self::assertSame(7, $result->exitCode); + self::assertSame('out', $result->stdout); + self::assertSame('err', $result->stderr); + } +} diff --git a/tests/Integration/Runner/RunScopeResolverTest.php b/tests/Integration/Runner/RunScopeResolverTest.php new file mode 100644 index 0000000..9bde123 --- /dev/null +++ b/tests/Integration/Runner/RunScopeResolverTest.php @@ -0,0 +1,125 @@ +directory = sys_get_temp_dir() . '/docbook-cs-scope-' . bin2hex(random_bytes(6)); + mkdir($this->directory); + + $this->sourceFile = $this->directory . '/source.xml'; + $this->targetFile = $this->directory . '/target.xml'; + $this->entityFile = $this->directory . '/bridge.ent'; + + file_put_contents($this->sourceFile, '&bridge;'); + file_put_contents($this->targetFile, ''); + file_put_contents($this->entityFile, '⌖'); + } + + protected function tearDown(): void + { + @unlink($this->sourceFile); + @unlink($this->targetFile); + @unlink($this->entityFile); + @rmdir($this->directory); + } + + #[Test] + public function narrowScopeKeepsOnlySelectedFilesAndDiffLines(): void + { + $targets = $this->resolver()->resolveDiff(new Diff([new FileChange('source.xml', [2, 3])])); + + self::assertSame([2, 3], $targets[$this->sourceFile]?->addedLineNumbers); + self::assertCount(1, $targets); + } + + #[Test] + public function wideScopeWidensSelectedFilesAndFollowsReferencedTargets(): void + { + $targets = $this->resolver(wide: true)->resolveDiff(new Diff([new FileChange('source.xml', [2, 3])])); + + self::assertNull($targets[$this->sourceFile]); + self::assertNull($targets[$this->targetFile]); + self::assertCount(2, $targets); + } + + #[Test] + public function expandedScopeHonorsTargetExclusions(): void + { + $resolver = new RunScopeResolver( + $this->config(['target.xml']), + [ + 'bridge' => $this->entityFile, + 'target' => $this->targetFile, + ], + wide: true, + ); + + self::assertSame([$this->sourceFile => null], $resolver->resolvePaths([$this->sourceFile])); + } + + #[Test] + public function pathScopeResolvesRelativePathsAgainstTheWorkingDirectory(): void + { + $path = 'tests/fixtures/sniff_runner/default/file_a.xml'; + $absolutePath = (getcwd() ?: '.') . '/' . $path; + + self::assertSame([$absolutePath => null], $this->resolver()->resolvePaths([$path])); + } + + private function resolver(bool $wide = false): RunScopeResolver + { + return new RunScopeResolver( + $this->config(), + [ + 'bridge' => $this->entityFile, + 'target' => $this->targetFile, + ], + $wide, + ); + } + + /** @param list $excludePatterns */ + private function config(array $excludePatterns = []): ConfigData + { + return new ConfigData( + projectRoots: [], + sniffs: [], + includePaths: [$this->sourceFile], + excludePatterns: $excludePatterns, + entityPaths: [], + basePath: $this->directory, + ); + } +} diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 35eaa53..4a36809 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -6,15 +6,19 @@ use DocbookCS\Application; use DocbookCS\Config\ConfigData; -use DocbookCS\Diff\Diff; -use DocbookCS\Diff\DiffParser; -use DocbookCS\Diff\FileChange; use DocbookCS\Config\ConfigParser; use DocbookCS\Config\ConfigParserException; use DocbookCS\Config\SniffEntry; +use DocbookCS\Diff\Diff; +use DocbookCS\Diff\DiffParser; +use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; +use DocbookCS\Process\NativeProcessRunner; +use DocbookCS\Process\ProcessResult; use DocbookCS\Progress\NullProgress; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; @@ -22,6 +26,9 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\SniffRunner; use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Sniff\ExceptionNameSniff; @@ -47,12 +54,19 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), CoversClass(SniffRunner::class), CoversClass(XmlFileProcessor::class), // UsesClass(Diff::class), + UsesClass(DiffPathLoader::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(NativeProcessRunner::class), + UsesClass(ProcessResult::class), + UsesClass(RunScopeResolver::class), ] final class ApplicationTest extends TestCase { @@ -289,114 +303,6 @@ public function itPassesThroughAbsoluteOverridePaths(): void self::assertNotSame(2, $exitCode); } - #[Test] - public function itSupportsDiffFromFile(): void - { - $diffFile = tempnam(sys_get_temp_dir(), 'docbookcs_test_'); - self::assertIsString($diffFile); - - // Diff that references no XML files the config would normally scan. - file_put_contents($diffFile, <<<'DIFF' -diff --git a/nonexistent.xml b/nonexistent.xml ---- a/nonexistent.xml -+++ b/nonexistent.xml -@@ -1,1 +1,2 @@ - line1 -+line2 -DIFF); - - try { - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, "--diff={$diffFile}"], - $this->stdout, - $this->stderr, - ); - - $exitCode = $app->run(); - - // No matching files → no violations → exit 0. - self::assertSame(0, $exitCode); - self::assertSame('', $this->readStream($this->stderr)); - } finally { - unlink($diffFile); - } - } - - #[Test] - public function itSupportsDiffFromStdin(): void - { - $stdin = fopen('php://memory', 'rb+'); - self::assertIsResource($stdin); - - fwrite($stdin, <<<'DIFF' -diff --git a/nonexistent.xml b/nonexistent.xml ---- a/nonexistent.xml -+++ b/nonexistent.xml -@@ -1,1 +1,2 @@ - line1 -+line2 -DIFF); - rewind($stdin); - - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff'], - $this->stdout, - $this->stderr, - $stdin, - ); - - $exitCode = $app->run(); - - self::assertSame(0, $exitCode); - self::assertSame('', $this->readStream($this->stderr)); - } - - #[Test] - public function itSupportsDiffFromStdinWithExplicitDash(): void - { - $stdin = fopen('php://memory', 'rb+'); - self::assertIsResource($stdin); - - fwrite($stdin, ''); - rewind($stdin); - - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff=-'], - $this->stdout, - $this->stderr, - $stdin, - ); - - $exitCode = $app->run(); - - self::assertSame(0, $exitCode); - } - - #[Test] - public function itReturnsErrorWhenDiffFileCannotBeRead(): void - { - $app = new Application( - ['docbook-cs', '--config=' . self::VALID_CONFIG, '--diff=/nonexistent/path.patch'], - $this->stdout, - $this->stderr, - ); - - $exitCode = $app->run(); - - self::assertSame(2, $exitCode); - self::assertStringContainsString('Error reading diff', $this->readStream($this->stderr)); - } - - #[Test] - public function itIncludesDiffOptionInHelp(): void - { - $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); - - $app->run(); - - self::assertStringContainsString('--diff', $this->readStream($this->stdout)); - } - #[Test] public function itSuppressesProgressWhenQuietFlagIsSet(): void { diff --git a/tests/Unit/Diff/DiffParserTest.php b/tests/Unit/Diff/DiffParserTest.php index 4fa72f6..88cd560 100644 --- a/tests/Unit/Diff/DiffParserTest.php +++ b/tests/Unit/Diff/DiffParserTest.php @@ -177,6 +177,26 @@ public function itIgnoresRemovedLines(): void self::assertSame([], $result['file.xml']); } + #[Test] + public function itAnchorsRemovedLinesInTheResultingFile(): void + { + $diff = <<<'DIFF' +diff --git a/file.xml b/file.xml +--- a/file.xml ++++ b/file.xml +@@ -1,4 +1,3 @@ + line1 +-removed line + line2 + line3 +DIFF; + + $change = $this->parser->parse($diff)->changeFor('file.xml'); + self::assertNotNull($change); + + self::assertSame([2], $change->deletionAnchors); + } + #[Test] public function itIgnoresTheMissingFinalNewlineMarker(): void { @@ -196,6 +216,26 @@ public function itIgnoresTheMissingFinalNewlineMarker(): void self::assertSame([1, 2], $result['file.xml']); } + #[Test] + public function itAnchorsReplacedLinesWhenTheMissingFinalNewlineMarkerIsPresent(): void + { + $diff = <<<'DIFF' +diff --git a/file.xml b/file.xml +--- a/file.xml ++++ b/file.xml +@@ -1 +1,2 @@ +-old +\ No newline at end of file ++new ++second +DIFF; + + $change = $this->parser->parse($diff)->changeFor('file.xml'); + self::assertNotNull($change); + + self::assertSame([1], $change->deletionAnchors); + } + #[Test] public function itTracksLineNumbersAcrossMultipleHunks(): void { 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/Runner/RunPlannerTest.php b/tests/Unit/Runner/RunPlannerTest.php new file mode 100644 index 0000000..fa0a912 --- /dev/null +++ b/tests/Unit/Runner/RunPlannerTest.php @@ -0,0 +1,67 @@ +createMock(DiffProviderInterface::class); + $diffProvider + ->expects(self::once()) + ->method('for') + ->willReturn(<<<'DIFF' +diff --git a/nonexistent.xml b/nonexistent.xml +--- a/nonexistent.xml ++++ b/nonexistent.xml +@@ -1 +1 @@ +-old ++new +DIFF); + + $planner = new RunPlanner($config, diffProvider: $diffProvider); + + self::assertSame([], $planner->plan([], null)->targets); + } +} diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 1db646a..74bd4e9 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -8,6 +8,8 @@ use DocbookCS\Config\SniffEntry; use DocbookCS\Diff\Diff; use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; @@ -18,6 +20,9 @@ use DocbookCS\Report\Severity; use DocbookCS\Report\Violation; use DocbookCS\Runner\EntityPreprocessor; +use DocbookCS\Runner\RunPlan; +use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScopeResolver; use DocbookCS\Runner\SniffRunner; use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Sniff\SniffInterface; @@ -35,13 +40,18 @@ CoversClass(PathLoader::class), CoversClass(PathMatcher::class), CoversClass(Report::class), + CoversClass(RunPlan::class), + CoversClass(RunPlanner::class), CoversClass(SniffEntry::class), CoversClass(SniffRunner::class), CoversClass(Violation::class), CoversClass(XmlFileProcessor::class), // UsesClass(Diff::class), + UsesClass(DiffPathLoader::class), UsesClass(FileChange::class), + UsesClass(GitDiffProvider::class), + UsesClass(RunScopeResolver::class), ] final class SniffRunnerTest extends TestCase { @@ -66,7 +76,7 @@ public function itProcessesFilesWithoutViolations(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); @@ -79,7 +89,7 @@ public function itUsesOverridePathsWhenProvided(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config, [self::FIXTURE_DIR . '/../override']); + $report = $runner->run($this->planPaths($config, [self::FIXTURE_DIR . '/../override'])); self::assertSame(1, $report->getFilesScanned()); } @@ -102,7 +112,7 @@ public function itCallsProgressMethods(): void $config = $this->createConfig(); $runner = new SniffRunner($progress); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -135,7 +145,7 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); self::assertCount(2, $report->getFileReports()); @@ -172,7 +182,7 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); foreach ($report->getFileReports() as $fileReport) { self::assertFalse( @@ -207,7 +217,7 @@ public function process(\DOMDocument $document, string $content, string $filePat $config = $this->createConfig(sniffs: [new SniffEntry($sniffClass::class, ['someProp' => 'someValue'])]); $runner = new SniffRunner(); - $runner->run($config); + $runner->run($this->planPaths($config)); self::assertSame('someValue', $sniffClass::$captured); } @@ -222,7 +232,7 @@ public function itThrowsWhenSniffClassDoesNotExist(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('does not exist'); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -235,7 +245,7 @@ public function itThrowsWhenClassDoesNotImplementSniffInterface(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('does not implement'); - $runner->run($config); + $runner->run($this->planPaths($config)); } #[Test] @@ -244,8 +254,8 @@ public function itFiltersFilesToOnlyThoseInTheDiff(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [1])]); - $report = $runner->run($config, null, $diff); + $diff = new Diff([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [1])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } @@ -257,7 +267,7 @@ public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void $runner = new SniffRunner(); $diff = new Diff([new FileChange('completely/different/file.xml', [1, 2, 3])]); - $report = $runner->run($config, null, $diff); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(0, $report->getFilesScanned()); } @@ -271,7 +281,7 @@ public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void $discoveredPath = self::FIXTURE_DIR . '/file_a.xml'; $diff = new Diff([new FileChange($discoveredPath, [1])]); - $report = $runner->run($config, null, $diff); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); } @@ -282,7 +292,7 @@ public function itScansAllFilesWhenNoDiffIsGiven(): void $config = $this->createConfig(); $runner = new SniffRunner(); - $report = $runner->run($config); + $report = $runner->run($this->planPaths($config)); self::assertSame(2, $report->getFilesScanned()); } @@ -317,10 +327,21 @@ public function setProperty(string $name, string $value): void $config = $this->createConfig(sniffs: [new SniffEntry($sniff::class)]); $runner = new SniffRunner(); - $diff = new Diff([new FileChange('sniff_runner/default/file_a.xml', [])]); - $report = $runner->run($config, null, $diff); + $diff = new Diff([new FileChange(self::FIXTURE_DIR . '/file_a.xml', [])]); + $report = $runner->run($this->planDiff($config, $diff)); self::assertSame(1, $report->getFilesScanned()); self::assertFalse($report->hasViolations()); } + + /** @param list|null $paths */ + private function planPaths(ConfigData $config, ?array $paths = null): RunPlan + { + return new RunPlanner($config)->planPaths($paths ?? $config->getIncludePaths()); + } + + private function planDiff(ConfigData $config, Diff $diff): RunPlan + { + return new RunPlanner($config)->planDiff($diff); + } } From 29873cbb4a417a951406d4a6b9a8f9e3cc0b4f8b Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 05:58:38 +0700 Subject: [PATCH 12/14] refactor: preserve absolute paths until report rendering --- src/RelativePath.php | 23 +++++++++++++++++++ src/Report/Reporter/CheckstyleReporter.php | 3 ++- src/Report/Reporter/ConsoleReporter.php | 9 +++++--- src/Report/Reporter/JsonReporter.php | 3 ++- src/Runner/SniffRunner.php | 19 +-------------- tests/Unit/Report/ReportTest.php | 12 ++++++++++ .../Reporter/CheckstyleReporterTest.php | 21 +++++++++++++++++ .../Report/Reporter/ConsoleReporterTest.php | 18 +++++++++++++++ .../Unit/Report/Reporter/JsonReporterTest.php | 18 +++++++++++++++ tests/Unit/Runner/SniffRunnerTest.php | 6 ++--- 10 files changed, 106 insertions(+), 26 deletions(-) create mode 100644 src/RelativePath.php diff --git a/src/RelativePath.php b/src/RelativePath.php new file mode 100644 index 0000000..1f0689b --- /dev/null +++ b/src/RelativePath.php @@ -0,0 +1,23 @@ +createElement('file'); - $fileNode->setAttribute('name', $fileReport->filePath); + $fileNode->setAttribute('name', RelativePath::fromWorkingDirectory($fileReport->filePath)); foreach ($fileReport->getViolations() as $violation) { $errorNode = $dom->createElement('error'); diff --git a/src/Report/Reporter/ConsoleReporter.php b/src/Report/Reporter/ConsoleReporter.php index 2599bce..3e2a1bb 100644 --- a/src/Report/Reporter/ConsoleReporter.php +++ b/src/Report/Reporter/ConsoleReporter.php @@ -4,6 +4,7 @@ namespace DocbookCS\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\Report; use DocbookCS\Report\Severity; @@ -27,9 +28,11 @@ public function generate(Report $report): string continue; } + $filePath = RelativePath::fromWorkingDirectory($fileReport->filePath); + $output .= PHP_EOL; - $output .= $this->bold('FILE: ' . $fileReport->filePath) . PHP_EOL; - $output .= str_repeat('-', min(80, 6 + strlen($fileReport->filePath))) . PHP_EOL; + $output .= $this->bold('FILE: ' . $filePath) . PHP_EOL; + $output .= str_repeat('-', min(80, 6 + strlen($filePath))) . PHP_EOL; foreach ($fileReport->getViolations() as $violation) { $output .= sprintf( @@ -41,7 +44,7 @@ public function generate(Report $report): string ) . PHP_EOL; } - $output .= str_repeat('-', min(80, 6 + strlen($fileReport->filePath))) . PHP_EOL; + $output .= str_repeat('-', min(80, 6 + strlen($filePath))) . PHP_EOL; } $output .= PHP_EOL; diff --git a/src/Report/Reporter/JsonReporter.php b/src/Report/Reporter/JsonReporter.php index 0e52b0f..0eb3e0f 100644 --- a/src/Report/Reporter/JsonReporter.php +++ b/src/Report/Reporter/JsonReporter.php @@ -4,6 +4,7 @@ namespace DocbookCS\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\Report; final class JsonReporter implements ReporterInterface @@ -38,7 +39,7 @@ public function generate(Report $report): string ]; } - $data['files'][$fileReport->filePath] = [ + $data['files'][RelativePath::fromWorkingDirectory($fileReport->filePath)] = [ 'violations' => count($violations), 'messages' => $violations, ]; diff --git a/src/Runner/SniffRunner.php b/src/Runner/SniffRunner.php index 923919b..eee358d 100644 --- a/src/Runner/SniffRunner.php +++ b/src/Runner/SniffRunner.php @@ -47,7 +47,7 @@ public function run(RunPlan $plan): Report $fileReport = $processor->processFile( $file, $changedLines, - $this->makeRelative($file), + $file, ); $violationCount = $fileReport->getViolationCount(); @@ -104,21 +104,4 @@ private function instantiateSniffs(array $entries): array return $sniffs; } - - private function makeRelative(string $absolutePath): string - { - $cwd = getcwd(); - if ($cwd === false) { - return $absolutePath; // @codeCoverageIgnore - } - - $prefix = rtrim(str_replace('\\', '/', $cwd), '/') . '/'; - $normalized = str_replace('\\', '/', $absolutePath); - - if (str_starts_with($normalized, $prefix)) { - return substr($normalized, strlen($prefix)); - } - - return $absolutePath; // @codeCoverageIgnore - } } diff --git a/tests/Unit/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php index 86eca0c..c15648f 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -4,6 +4,7 @@ namespace DocbookCS\Tests\Unit\Report; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Severity; @@ -14,6 +15,7 @@ #[ CoversClass(FileReport::class), + CoversClass(RelativePath::class), CoversClass(Report::class), CoversClass(Violation::class), ] @@ -94,6 +96,16 @@ public function itOverwritesFileReportWithSamePath(): void self::assertSame($second, $report->getFileReports()['file.xml']); } + #[Test] + public function itKeepsTheFileReportPathWhileRenderingItRelativeToWorkingDirectory(): void + { + $filePath = (getcwd() ?: '') . '/src/chapter.xml'; + $fileReport = new FileReport($filePath); + + self::assertSame($filePath, $fileReport->filePath); + self::assertSame('src/chapter.xml', RelativePath::fromWorkingDirectory($fileReport->filePath)); + } + #[Test] public function itReturnsTotalViolationsAcrossAllFiles(): void { diff --git a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php index 87cbfb1..ec30288 100644 --- a/tests/Unit/Report/Reporter/CheckstyleReporterTest.php +++ b/tests/Unit/Report/Reporter/CheckstyleReporterTest.php @@ -4,6 +4,7 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\CheckstyleReporter; @@ -11,6 +12,7 @@ use DocbookCS\Report\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +20,8 @@ CoversClass(FileReport::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), ] final class CheckstyleReporterTest extends TestCase { @@ -113,6 +117,23 @@ public function itIncludesFileNodeWithNameAttribute(): void self::assertSame('src/broken.xml', $fileNodes->item(0)?->getAttribute('name')); } + #[Test] + public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void + { + $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml'); + $fileReport->addViolation($this->createViolation()); + + $report = new Report(); + $report->addFileReport($fileReport); + + $dom = $this->parseOutput($this->reporter->generate($report)); + + self::assertSame( + 'src/broken.xml', + $dom->getElementsByTagName('file')->item(0)?->getAttribute('name'), + ); + } + #[Test] public function itSetsLineAttribute(): void { diff --git a/tests/Unit/Report/Reporter/ConsoleReporterTest.php b/tests/Unit/Report/Reporter/ConsoleReporterTest.php index ad2ba58..34f0bac 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -4,6 +4,7 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\ConsoleReporter; @@ -11,6 +12,7 @@ use DocbookCS\Report\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +20,8 @@ CoversClass(FileReport::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), ] final class ConsoleReporterTest extends TestCase { @@ -90,6 +94,20 @@ public function itShowsFilePathInHeader(): void self::assertStringContainsString('FILE: src/broken.xml', $output); } + #[Test] + public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void + { + $fileReport = new FileReport((getcwd() ?: '') . '/src/broken.xml'); + $fileReport->addViolation($this->createViolation()); + + $report = new Report(); + $report->addFileReport($fileReport); + + $output = $this->reporter->generate($report); + + self::assertStringContainsString('FILE: src/broken.xml', $output); + } + #[Test] public function itShowsDashSeparatorAfterFileHeader(): void { diff --git a/tests/Unit/Report/Reporter/JsonReporterTest.php b/tests/Unit/Report/Reporter/JsonReporterTest.php index 3cbbe72..0a7915c 100644 --- a/tests/Unit/Report/Reporter/JsonReporterTest.php +++ b/tests/Unit/Report/Reporter/JsonReporterTest.php @@ -4,6 +4,7 @@ namespace DocbookCS\Tests\Unit\Report\Reporter; +use DocbookCS\RelativePath; use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\JsonReporter; @@ -11,6 +12,7 @@ use DocbookCS\Report\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; #[ @@ -18,6 +20,8 @@ CoversClass(JsonReporter::class), CoversClass(Report::class), CoversClass(Violation::class), + // + UsesClass(RelativePath::class), ] final class JsonReporterTest extends TestCase { @@ -312,6 +316,20 @@ public function itDoesNotEscapeSlashesInOutput(): void self::assertStringNotContainsString('path\/to\/file.xml', $output); } + #[Test] + public function itRendersAbsoluteFilePathRelativeToWorkingDirectory(): void + { + $fileReport = new FileReport((getcwd() ?: '') . '/path/to/file.xml'); + $fileReport->addViolation($this->createViolation()); + + $report = new Report(); + $report->addFileReport($fileReport); + + $data = $this->parseOutput($this->reporter->generate($report)); + + self::assertArrayHasKey('path/to/file.xml', $data['files']); + } + #[Test] public function itUsesPrettyPrintedJson(): void { diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 74bd4e9..3e7f15f 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -153,7 +153,7 @@ public function setProperty(string $name, string $value): void } #[Test] - public function itStoresRelativePathsInFileReports(): void + public function itStoresAbsolutePathsInFileReports(): void { $sniff = new class implements SniffInterface { public function getCode(): string @@ -185,9 +185,9 @@ public function setProperty(string $name, string $value): void $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, ); } } From 884386aa4be30067a396bb50b920e2148f9ed84e Mon Sep 17 00:00:00 2001 From: NickSdot Date: Tue, 21 Jul 2026 06:01:13 +0700 Subject: [PATCH 13/14] feat: add automatic fixers --- README.md | 12 +- bin/docbook-cs | 11 +- phpcs.xml | 3 + src/Application.php | 35 +- src/Diff/DiffParser.php | 6 +- src/Diff/GitDiffProvider.php | 13 +- src/Fix/Fix.php | 18 + src/Fix/FixApplier.php | 205 ++++++++++ src/Fix/FixPlan.php | 34 ++ src/Fix/FixResult.php | 19 + src/Fix/Fixer/AttributeOrderFixer.php | 80 ++++ src/Fix/Fixer/ExceptionNameFixer.php | 54 +++ src/Fix/Fixer/Fixer.php | 16 + src/Fix/Fixer/MixedIndentationFixer.php | 39 ++ src/Fix/Fixer/SimparaFixer.php | 47 +++ src/Fix/Fixer/TrailingWhitespaceFixer.php | 35 ++ src/Fix/Fixer/WhitespaceFixer.php | 43 ++ src/Fix/FixerException.php | 49 +++ src/Path/DiffPathLoader.php | 8 +- src/Path/EntityResolver.php | 27 +- src/Report/FileReport.php | 11 + src/Report/Report.php | 2 + src/Report/Reporter/ConsoleReporter.php | 2 +- src/Report/Violation.php | 17 - src/Runner/EntityPreprocessor.php | 17 +- .../{SniffRunner.php => RunCoordinator.php} | 64 +-- src/Runner/RunMode.php | 21 + src/Runner/RunPlan.php | 3 +- src/Runner/RunPlanner.php | 10 +- src/Runner/SourceScope.php | 219 ++++++++++ src/Runner/ViolationScopeFilter.php | 129 ++++++ src/Runner/XmlFileProcessor.php | 270 +++++-------- src/Runner/XmlProcessingResult.php | 34 ++ src/Sniff/AbstractSniff.php | 26 +- src/Sniff/AttributeOrderSniff.php | 68 ++-- src/Sniff/ExceptionNameSniff.php | 110 +++-- src/Sniff/Fixable.php | 13 + src/Sniff/MixedIndentationSniff.php | 52 +++ src/Sniff/SimparaSniff.php | 148 ++++++- src/Sniff/SniffInterface.php | 17 +- src/Sniff/TrailingWhitespaceSniff.php | 50 +++ src/Sniff/WhitespaceSniff.php | 56 ++- src/Source/File.php | 116 ++++++ src/Source/Line.php | 26 ++ src/{Report => Violation}/Severity.php | 2 +- src/Violation/SourceRange.php | 15 + src/Violation/Violation.php | 30 ++ tests/Feature/ApplicationInputTest.php | 6 +- .../Integration/Diff/GitDiffProviderTest.php | 3 +- .../Fix/AttributeOrderFixerTest.php | 72 ++++ .../Fix/ExceptionNameFixerTest.php | 128 ++++++ tests/Integration/Fix/SimparaFixerTest.php | 111 ++++++ .../Fix/WhitespaceConcernFixersTest.php | 73 ++++ tests/Integration/Fix/WhitespaceFixerTest.php | 75 ++++ .../Process/NativeProcessRunnerTest.php | 19 +- .../Integration/Runner/FixConvergenceTest.php | 375 ++++++++++++++++++ .../Runner/RunCoordinatorFileFailureTest.php | 107 +++++ .../Runner/RunScopeResolverTest.php | 20 +- tests/Integration/Runner/RunScopeTest.php | 227 +++++++++++ .../Runner/SourceRangeScopeTest.php | 126 ++++++ .../Runner/XmlFileProcessorPipelineTest.php | 101 +++++ .../Sniff/EntityExpandedSniffTest.php | 29 +- tests/Support/Fix/LineBreakFixer.php | 29 ++ tests/Support/Fix/ToggleElementFixer.php | 35 ++ tests/Unit/ApplicationTest.php | 56 +-- tests/Unit/Fix/FixPlanTest.php | 110 +++++ tests/Unit/Report/ReportTest.php | 30 +- .../Reporter/CheckstyleReporterTest.php | 8 +- .../Report/Reporter/ConsoleReporterTest.php | 8 +- .../Unit/Report/Reporter/JsonReporterTest.php | 8 +- tests/Unit/Runner/RunPlannerTest.php | 2 +- tests/Unit/Runner/SniffRunnerTest.php | 148 ++++--- tests/Unit/Runner/SourceScopeTest.php | 140 +++++++ .../Unit/Runner/ViolationScopeFilterTest.php | 51 +++ tests/Unit/Runner/XmlFileProcessorTest.php | 244 ++++++++++-- tests/Unit/Runner/XmlProcessingResultTest.php | 73 ++++ tests/Unit/Sniff/AbstractSniffTest.php | 5 +- tests/Unit/Sniff/AffectedRangesTest.php | 71 ++++ tests/Unit/Sniff/AttributeOrderSniffTest.php | 41 +- tests/Unit/Sniff/ExceptionNameSniffTest.php | 65 ++- tests/Unit/Sniff/SimparaSniffTest.php | 59 +-- .../Sniff/WhitespaceConcernSniffsTest.php | 88 ++++ tests/Unit/Sniff/WhitespaceSniffTest.php | 28 +- tests/Unit/Source/FileTest.php | 96 +++++ tests/Unit/Violation/AffectedRangesTest.php | 50 +++ 85 files changed, 4618 insertions(+), 581 deletions(-) create mode 100644 src/Fix/Fix.php create mode 100644 src/Fix/FixApplier.php create mode 100644 src/Fix/FixPlan.php create mode 100644 src/Fix/FixResult.php create mode 100644 src/Fix/Fixer/AttributeOrderFixer.php create mode 100644 src/Fix/Fixer/ExceptionNameFixer.php create mode 100644 src/Fix/Fixer/Fixer.php create mode 100644 src/Fix/Fixer/MixedIndentationFixer.php create mode 100644 src/Fix/Fixer/SimparaFixer.php create mode 100644 src/Fix/Fixer/TrailingWhitespaceFixer.php create mode 100644 src/Fix/Fixer/WhitespaceFixer.php create mode 100644 src/Fix/FixerException.php delete mode 100644 src/Report/Violation.php rename src/Runner/{SniffRunner.php => RunCoordinator.php} (52%) create mode 100644 src/Runner/RunMode.php create mode 100644 src/Runner/SourceScope.php create mode 100644 src/Runner/ViolationScopeFilter.php create mode 100644 src/Runner/XmlProcessingResult.php create mode 100644 src/Sniff/Fixable.php create mode 100644 src/Sniff/MixedIndentationSniff.php create mode 100644 src/Sniff/TrailingWhitespaceSniff.php create mode 100644 src/Source/File.php create mode 100644 src/Source/Line.php rename src/{Report => Violation}/Severity.php (81%) create mode 100644 src/Violation/SourceRange.php create mode 100644 src/Violation/Violation.php create mode 100644 tests/Integration/Fix/AttributeOrderFixerTest.php create mode 100644 tests/Integration/Fix/ExceptionNameFixerTest.php create mode 100644 tests/Integration/Fix/SimparaFixerTest.php create mode 100644 tests/Integration/Fix/WhitespaceConcernFixersTest.php create mode 100644 tests/Integration/Fix/WhitespaceFixerTest.php create mode 100644 tests/Integration/Runner/FixConvergenceTest.php create mode 100644 tests/Integration/Runner/RunCoordinatorFileFailureTest.php create mode 100644 tests/Integration/Runner/RunScopeTest.php create mode 100644 tests/Integration/Runner/SourceRangeScopeTest.php create mode 100644 tests/Integration/Runner/XmlFileProcessorPipelineTest.php create mode 100644 tests/Support/Fix/LineBreakFixer.php create mode 100644 tests/Support/Fix/ToggleElementFixer.php create mode 100644 tests/Unit/Fix/FixPlanTest.php create mode 100644 tests/Unit/Runner/SourceScopeTest.php create mode 100644 tests/Unit/Runner/ViolationScopeFilterTest.php create mode 100644 tests/Unit/Runner/XmlProcessingResultTest.php create mode 100644 tests/Unit/Sniff/AffectedRangesTest.php create mode 100644 tests/Unit/Sniff/WhitespaceConcernSniffsTest.php create mode 100644 tests/Unit/Source/FileTest.php create mode 100644 tests/Unit/Violation/AffectedRangesTest.php diff --git a/README.md b/README.md index 4be5799..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(...) @@ -70,9 +71,10 @@ through the working tree. Alternatively, a unified diff can be piped or file and directory paths passed. The inspection scope is limited to the given diff or the full contents of the given file paths. -XML references are expanded by default, but reported violations remain limited -to the given scope. With `--wide`, every file inferred from paths or a diff is -checked as a whole, and referenced `SYSTEM` XML files are recursively included. +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 | |------------|---------:|-------------:|-----------:| diff --git a/bin/docbook-cs b/bin/docbook-cs index 4b72e67..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); })(); -exit(Application::fromGlobals($argv ?? [])->run()); +try { + $application = Application::fromGlobals($argv ?? []); +} catch (\RuntimeException $e) { + fwrite(STDERR, $e->getMessage() . PHP_EOL); + exit(2); +} + +exit($application->run()); diff --git a/phpcs.xml b/phpcs.xml index 93f3b2f..3495fd9 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -2,6 +2,9 @@ DocbookCS Coding Standard + + tests/* + diff --git a/src/Application.php b/src/Application.php index bb36f76..59e7e3e 100644 --- a/src/Application.php +++ b/src/Application.php @@ -14,8 +14,9 @@ use DocbookCS\Report\Reporter\ConsoleReporter; use DocbookCS\Report\Reporter\JsonReporter; use DocbookCS\Report\Reporter\ReporterInterface; +use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunPlanner; -use DocbookCS\Runner\SniffRunner; final class Application { @@ -112,7 +113,11 @@ public function run(): int } try { - $runPlan = new RunPlanner($config, $options['wide'])->plan($options['paths'], $this->stdin); + $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); @@ -122,8 +127,7 @@ public function run(): int $progress = $this->createProgress($options); try { - $runner = new SniffRunner($progress); - $report = $runner->run($runPlan); + $report = new RunCoordinator($progress)->run($runPlan); } catch (\Throwable $e) { $this->writeError('Runtime error: ' . $e->getMessage() . PHP_EOL); @@ -148,12 +152,13 @@ public function run(): int * config: string, * report: string, * colors: bool, + * fix: bool, + * wide: bool, * quiet: bool, * paths: list, - * wide: bool, * perf: bool, * } - * @throws \InvalidArgumentException for unsupported options. + * @throws \InvalidArgumentException for unsupported or removed options. */ private function parseArgv(): array { @@ -163,9 +168,10 @@ private function parseArgv(): array 'config' => self::DEFAULT_CONFIG, 'report' => 'console', 'colors' => $this->detectColorSupport(), + 'fix' => false, + 'wide' => false, 'quiet' => false, 'paths' => [], - 'wide' => false, 'perf' => false, ]; @@ -236,6 +242,12 @@ private function parseArgv(): array continue; } + if ($arg === '--fix') { + $result['fix'] = true; + $i++; + continue; + } + if ($arg === '--wide') { $result['wide'] = true; $i++; @@ -359,6 +371,8 @@ private function printHelp(): void --report= Output format: console (default), checkstyle, json. --colors Force ANSI color output. --no-colors Disable ANSI color output. + --fix Automatically fix violations when fixers exist + (experimental). --wide Check whole selected files and recursively include referenced XML files. @@ -370,9 +384,14 @@ private function printHelp(): void 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 + 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 --report=checkstyle + git diff HEAD | docbook-cs --wide + git diff HEAD | docbook-cs --wide --fix --report=checkstyle HELP; diff --git a/src/Diff/DiffParser.php b/src/Diff/DiffParser.php index 0c62f3a..95e274f 100644 --- a/src/Diff/DiffParser.php +++ b/src/Diff/DiffParser.php @@ -101,7 +101,11 @@ public function parse(string $diff): Diff $fileChanges = []; foreach ($changedLinesByFile as $filePath => $lineNumbers) { - $fileChanges[] = new FileChange($filePath, $lineNumbers, $deletionAnchorsByFile[$filePath]); + $fileChanges[] = new FileChange( + $filePath, + $lineNumbers, + $deletionAnchorsByFile[$filePath], + ); } return new Diff($fileChanges); diff --git a/src/Diff/GitDiffProvider.php b/src/Diff/GitDiffProvider.php index 3e7c146..fe4e6b3 100644 --- a/src/Diff/GitDiffProvider.php +++ b/src/Diff/GitDiffProvider.php @@ -24,10 +24,13 @@ public function for(string $workingDirectory): string )); $baseReference = $this->resolveBaseReference($repositoryRoot); + + $error = sprintf('Unclear where HEAD branched from %s.', $baseReference); + $mergeBase = $this->runOrThrow( ['git', 'merge-base', 'HEAD', $baseReference], $repositoryRoot, - sprintf('Unclear where HEAD branched from %s.', $baseReference), + $error, ); return $this->runOrThrow( @@ -70,7 +73,9 @@ private function resolveBaseReference(string $repositoryRoot): string } } - throw new \RuntimeException('Could not determine the upstream default branch for the contribution diff.'); + throw new \RuntimeException( + 'Could not determine the upstream default branch for the contribution diff.', + ); } /** @@ -87,6 +92,8 @@ private function runOrThrow(array $command, string $workingDirectory, string $er $detail = trim($result->stderr); - throw new \RuntimeException($detail !== '' ? "$error $detail" : $error); + 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 index 22706e0..5e48116 100644 --- a/src/Path/DiffPathLoader.php +++ b/src/Path/DiffPathLoader.php @@ -9,7 +9,9 @@ final readonly class DiffPathLoader { - /** @param array $projectRoots */ + /** + * @param array $projectRoots + */ public function __construct( private Diff $diff, private string $workingDirectory, @@ -69,8 +71,8 @@ private function candidates(string $path): array } return array_map($this->normalize(...), $candidates) - |> array_unique(...) - |> array_values(...); + |> array_unique(...) + |> array_values(...); } private function isAbsolute(string $path): bool diff --git a/src/Path/EntityResolver.php b/src/Path/EntityResolver.php index 8dc3f29..aafb27b 100644 --- a/src/Path/EntityResolver.php +++ b/src/Path/EntityResolver.php @@ -121,12 +121,8 @@ private function scanDirectory(string $directory): array * @param array $paths * @return array */ - private function resolveFile( - string $filePath, - array &$visited, - array &$paths, - ?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 []; } @@ -153,14 +149,9 @@ private function resolveFile( * @param array $paths * @return array */ - private function extractEntities( - string $content, - string $filePath, - array &$visited, - array &$paths, - ): array { - return - $this->extractDtdEntities($content, $filePath, $visited, $paths) + private function extractEntities(string $content, string $filePath, array &$visited, array &$paths): array + { + return $this->extractDtdEntities($content, $filePath, $visited, $paths) + $this->extractXmlEntities($content); } @@ -169,12 +160,8 @@ private function extractEntities( * @param array $paths * @return array */ - private function extractDtdEntities( - string $content, - string $filePath, - array &$visited, - array &$paths, - ): array { + private function extractDtdEntities(string $content, string $filePath, array &$visited, array &$paths): array + { $result = []; if (!str_contains($content, ' */ @@ -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/ConsoleReporter.php b/src/Report/Reporter/ConsoleReporter.php index 3e2a1bb..b6967c6 100644 --- a/src/Report/Reporter/ConsoleReporter.php +++ b/src/Report/Reporter/ConsoleReporter.php @@ -6,7 +6,7 @@ use DocbookCS\RelativePath; use DocbookCS\Report\Report; -use DocbookCS\Report\Severity; +use DocbookCS\Violation\Severity; final class ConsoleReporter implements ReporterInterface { diff --git a/src/Report/Violation.php b/src/Report/Violation.php deleted file mode 100644 index 24df707..0000000 --- a/src/Report/Violation.php +++ /dev/null @@ -1,17 +0,0 @@ -||<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/', function (array $matches) use (&$changed, $markXmlExpansions): string { - if ( - str_starts_with($matches[0], '