From b6758984505631722238e65db0a9214f57821b56 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 13:47:45 +0700 Subject: [PATCH 01/24] test: drop redundancy to avoid coverage gymnastics --- src/Path/EntityResolver.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Path/EntityResolver.php b/src/Path/EntityResolver.php index aafb27b..a4fd274 100644 --- a/src/Path/EntityResolver.php +++ b/src/Path/EntityResolver.php @@ -123,16 +123,16 @@ private function scanDirectory(string $directory): array */ private function resolveFile(string $filePath, array &$visited, array &$paths, ?string $originEntity = null): array { - if (isset($visited[$filePath]) || !is_readable($filePath)) { + if (isset($visited[$filePath])) { return []; } $visited[$filePath] = true; - $content = file_get_contents($filePath); + $content = @file_get_contents($filePath); if ($content === false) { - return []; // @codeCoverageIgnore + return []; } $entities = $this->extractEntities($content, $filePath, $visited, $paths); From 2a99cadc2af8938964f469540604f560842850d8 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:16:52 +0700 Subject: [PATCH 02/24] test: suppressed native warning --- src/Process/NativeProcessRunner.php | 2 +- tests/Unit/Process/NativeProcessRunnerTest.php | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php index 8e8907c..8c12354 100644 --- a/src/Process/NativeProcessRunner.php +++ b/src/Process/NativeProcessRunner.php @@ -8,7 +8,7 @@ final class NativeProcessRunner implements ProcessRunnerInterface { public function run(array $command, string $workingDirectory, array $environment = []): ProcessResult { - $process = proc_open( + $process = @proc_open( $command, [ ['pipe', 'r'], diff --git a/tests/Unit/Process/NativeProcessRunnerTest.php b/tests/Unit/Process/NativeProcessRunnerTest.php index 4b5eb06..81c954a 100644 --- a/tests/Unit/Process/NativeProcessRunnerTest.php +++ b/tests/Unit/Process/NativeProcessRunnerTest.php @@ -5,6 +5,7 @@ namespace DocbookCS\Tests\Unit\Process; use DocbookCS\Process\NativeProcessRunner; +use DocbookCS\Process\ProcessException; use DocbookCS\Process\ProcessResult; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -12,6 +13,7 @@ #[ CoversClass(NativeProcessRunner::class), + CoversClass(ProcessException::class), CoversClass(ProcessResult::class), ] final class NativeProcessRunnerTest extends TestCase @@ -46,4 +48,16 @@ public function itPassesEnvironmentVariablesToTheProcess(): void self::assertSame(0, $result->exitCode); self::assertSame('value', $result->stdout); } + + #[Test] + public function itRejectsAnInvalidWorkingDirectory(): void + { + $this->expectException(ProcessException::class); + $this->expectExceptionMessageIs('Could not start process.'); + + new NativeProcessRunner()->run( + [PHP_BINARY, '-r', ''], + sys_get_temp_dir() . '/missing-' . bin2hex(random_bytes(6)), + ); + } } From 5e1d15d80ab92685e69db676cb4bb2a0c05db6d3 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:20:34 +0700 Subject: [PATCH 03/24] test: added coverage for UpstreamResolver --- src/Diff/UpstreamResolver.php | 3 +- tests/Unit/Diff/UpstreamResolverTest.php | 116 +++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/Diff/UpstreamResolverTest.php diff --git a/src/Diff/UpstreamResolver.php b/src/Diff/UpstreamResolver.php index 4fc45ae..6705b7c 100644 --- a/src/Diff/UpstreamResolver.php +++ b/src/Diff/UpstreamResolver.php @@ -48,8 +48,9 @@ public function resolve(string $repoRoot, string $repoName): ?string } try { + // filesystem or OS edge cases if (!flock($lock, LOCK_EX)) { - return null; + return null; // @codeCoverageIgnore } return $this->refreshAndResolve($repoRoot, $repoName); diff --git a/tests/Unit/Diff/UpstreamResolverTest.php b/tests/Unit/Diff/UpstreamResolverTest.php new file mode 100644 index 0000000..94bf9d4 --- /dev/null +++ b/tests/Unit/Diff/UpstreamResolverTest.php @@ -0,0 +1,116 @@ +workspace = sys_get_temp_dir() . '/docbook-cs-upstream-' . bin2hex(random_bytes(6)); + mkdir($this->workspace); + } + + protected function tearDown(): void + { + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($this->workspace, \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->workspace); + } + + #[Test] + public function itFallsBackWhenTheCacheDirectoryCannotBeCreated(): void + { + $cachePath = $this->workspace . '/cache'; + file_put_contents($cachePath, 'not a directory'); + + self::assertNull($this->resolver($this->successfulRunner(), $cachePath)->resolve('/repo', 'doc-en')); + } + + #[Test] + public function itFallsBackWhenTheCacheLockCannotBeOpened(): void + { + $cachePath = $this->workspace . '/cache'; + mkdir($cachePath); + mkdir($cachePath . '/doc-en.lock'); + + self::assertNull($this->resolver($this->successfulRunner(), $cachePath)->resolve('/repo', 'doc-en')); + } + + #[Test] + public function itFallsBackWhenTheCacheRepositoryCannotBeInitialised(): void + { + $runner = new class implements ProcessRunnerInterface { + public function run(array $command, string $workingDirectory, array $environment = []): ProcessResult + { + return new ProcessResult(1, '', 'Could not initialise cache.'); + } + }; + + self::assertNull($this->resolver($runner)->resolve('/repo', 'doc-en')); + } + + #[Test] + public function itFallsBackWhenGitCannotBeStarted(): void + { + $runner = new class implements ProcessRunnerInterface { + public function run(array $command, string $workingDirectory, array $environment = []): ProcessResult + { + throw ProcessException::couldNotStart(); + } + }; + + self::assertNull($this->resolver($runner)->resolve('/repo', 'doc-en')); + } + + private function resolver(ProcessRunnerInterface $runner, ?string $cachePath = null): UpstreamResolver + { + return new UpstreamResolver( + new GitClient($runner), + $cachePath ?? $this->workspace . '/cache', + ); + } + + private function successfulRunner(): ProcessRunnerInterface + { + return new class implements ProcessRunnerInterface { + public function run(array $command, string $workingDirectory, array $environment = []): ProcessResult + { + return new ProcessResult(0, '', ''); + } + }; + } +} From d321870988813f83fd4a6c1f519742a669b9c388 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:43:34 +0700 Subject: [PATCH 04/24] test: added early return --- src/Progress/ConsoleProgress.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Progress/ConsoleProgress.php b/src/Progress/ConsoleProgress.php index 1157038..5b6c32b 100644 --- a/src/Progress/ConsoleProgress.php +++ b/src/Progress/ConsoleProgress.php @@ -67,13 +67,14 @@ public function finish(): void private function drawBar(int $current, string $filePath, int $violations = 0): void { - $percent = $this->totalFiles > 0 - ? (int)floor(($current / $this->totalFiles) * 100) - : 0; // @codeCoverageIgnore + if ($this->totalFiles === 0) { + // prevented by public methods + return; // @codeCoverageIgnore + } - $filled = $this->totalFiles > 0 - ? (int)floor(($current / $this->totalFiles) * self::BAR_WIDTH) - : 0; // @codeCoverageIgnore + $ratio = $current / $this->totalFiles; + $percent = (int)floor($ratio * 100); + $filled = (int)floor($ratio * self::BAR_WIDTH); $empty = self::BAR_WIDTH - $filled; From fb91944b9ecc6ce1b2b28552e78bd5ccc65df9a9 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:45:10 +0700 Subject: [PATCH 05/24] test: increased coverage --- src/Report/Reporter/ConsoleReporter.php | 4 ++-- tests/Unit/Report/Reporter/ConsoleReporterTest.php | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Report/Reporter/ConsoleReporter.php b/src/Report/Reporter/ConsoleReporter.php index 39c2f6b..eb0b8b4 100644 --- a/src/Report/Reporter/ConsoleReporter.php +++ b/src/Report/Reporter/ConsoleReporter.php @@ -152,11 +152,11 @@ private function collectPerformanceRows(Report $report): array private function formatSeverity(Severity $severity): string { - return match ($severity) { // @codeCoverageIgnore + return match ($severity) { Severity::ERROR => $this->red(str_pad(Severity::ERROR->name, 7)), Severity::WARNING => $this->yellow(str_pad(Severity::WARNING->name, 7)), default => $this->dim(str_pad(strtoupper($severity->name), 7)), - }; // @codeCoverageIgnore + }; } private function formatPerformanceCell(?float $time, float $totalTime): string diff --git a/tests/Unit/Report/Reporter/ConsoleReporterTest.php b/tests/Unit/Report/Reporter/ConsoleReporterTest.php index b6d6ddd..453f4f1 100644 --- a/tests/Unit/Report/Reporter/ConsoleReporterTest.php +++ b/tests/Unit/Report/Reporter/ConsoleReporterTest.php @@ -329,6 +329,18 @@ public function itShowsErrorSeverityLabel(): void self::assertStringContainsString('ERROR', $output); } + #[Test] + public function itShowsInfoSeverityLabel(): void + { + $fileReport = new FileReport('file.xml'); + $fileReport->addFoundViolations([$this->createViolation(severity: Severity::INFO)]); + + $report = new Report(); + $report->addFileReport($fileReport); + + self::assertStringContainsString('INFO', $this->reporter->generate($report)); + } + #[Test] public function itShowsMultipleViolationsForOneFile(): void { From dd84480fb45a05d751d0cd71c93618fb115bcdfc Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:51:33 +0700 Subject: [PATCH 06/24] test: increased edge case coverage --- src/Runner/RunScopeResolver.php | 7 ------ tests/Unit/Runner/RunScopeResolverTest.php | 25 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/Runner/RunScopeResolver.php b/src/Runner/RunScopeResolver.php index 89efb53..dd57aca 100644 --- a/src/Runner/RunScopeResolver.php +++ b/src/Runner/RunScopeResolver.php @@ -105,17 +105,10 @@ private function absolutePaths(array $paths): array 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) { diff --git a/tests/Unit/Runner/RunScopeResolverTest.php b/tests/Unit/Runner/RunScopeResolverTest.php index 75d97c2..c914f00 100644 --- a/tests/Unit/Runner/RunScopeResolverTest.php +++ b/tests/Unit/Runner/RunScopeResolverTest.php @@ -161,6 +161,31 @@ public function wideScopeHandlesCyclicEntityReferences(): void ); } + #[Test] + public function wideScopeSkipsExpansionForUnreadableFiles(): void + { + if (DIRECTORY_SEPARATOR === '\\') { + self::markTestSkipped('Windows does not support Unix file permissions.'); + } + + chmod($this->sourceFile, 0000); + clearstatcache(true, $this->sourceFile); + + if (is_readable($this->sourceFile)) { + chmod($this->sourceFile, 0600); + self::markTestSkipped('The current user can read files without permissions.'); + } + + try { + self::assertSame( + [$this->sourceFile => null], + $this->resolver(wide: true)->resolvePaths([$this->sourceFile]), + ); + } finally { + chmod($this->sourceFile, 0600); + } + } + #[Test] public function wideScopeNormalisesParentDirectorySegmentsInEntityPaths(): void { From adf1d61251b62aa5f4a643408137334ac6b9c897 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 14:55:42 +0700 Subject: [PATCH 07/24] test: added cwd fallback --- src/RelativePath.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/RelativePath.php b/src/RelativePath.php index 1f0689b..3a7dac5 100644 --- a/src/RelativePath.php +++ b/src/RelativePath.php @@ -8,11 +8,7 @@ final class RelativePath { public static function fromWorkingDirectory(string $filePath): string { - $workingDirectory = getcwd(); - if ($workingDirectory === false) { - return $filePath; // @codeCoverageIgnore - } - + $workingDirectory = getcwd() ?: '.'; $prefix = rtrim(str_replace('\\', '/', $workingDirectory), '/') . '/'; $normalisedPath = str_replace('\\', '/', $filePath); From 2febc56708c2067aaf443bffb1b8f59862530678 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:12:05 +0700 Subject: [PATCH 08/24] test: added fixer mapping coverage --- tests/Unit/Fix/AttributeOrderFixerTest.php | 5 +++-- tests/Unit/Fix/ExceptionNameFixerTest.php | 5 +++-- tests/Unit/Fix/SimparaFixerTest.php | 5 +++-- tests/Unit/Fix/WhitespaceConcernFixersTest.php | 11 +++++++---- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/Unit/Fix/AttributeOrderFixerTest.php b/tests/Unit/Fix/AttributeOrderFixerTest.php index d4a38da..9c65253 100644 --- a/tests/Unit/Fix/AttributeOrderFixerTest.php +++ b/tests/Unit/Fix/AttributeOrderFixerTest.php @@ -41,7 +41,8 @@ public function itMovesXmlIdBeforeXmlns(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new AttributeOrderSniff()->process($document, $source); + $sniffer = new AttributeOrderSniff(); + $violations = $sniffer->process($document, $source); $beginOffset = (int)strpos($content, 'rangeOne()->untilOffset); self::assertSame(1, $violations[0]->rangeOne()->line); - $fix = new AttributeOrderFixer()->process($violations[0]); + $fix = new ($sniffer::getFixerClassName())()->process($violations[0]); $result = new FixApplier()->apply($source, [ $fix ]); diff --git a/tests/Unit/Fix/ExceptionNameFixerTest.php b/tests/Unit/Fix/ExceptionNameFixerTest.php index d384631..832fd32 100644 --- a/tests/Unit/Fix/ExceptionNameFixerTest.php +++ b/tests/Unit/Fix/ExceptionNameFixerTest.php @@ -43,7 +43,8 @@ public function itReplacesSimpleClassnameTags(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new ExceptionNameSniff()->process($document, $source); + $sniffer = new ExceptionNameSniff(); + $violations = $sniffer->process($document, $source); $beginOffset = (int) strpos($content, ''); $untilOffset = (int) strpos($content, ''); @@ -56,7 +57,7 @@ public function itReplacesSimpleClassnameTags(): void new SourceRange(1, $untilOffset + 2, $untilOffset + 11, 'classname'), ], $violations[0]->affectedRanges); - $fix = new ExceptionNameFixer()->process($violations[0]); + $fix = new ($sniffer::getFixerClassName())()->process($violations[0]); $result = new FixApplier()->apply($source, [$fix]); diff --git a/tests/Unit/Fix/SimparaFixerTest.php b/tests/Unit/Fix/SimparaFixerTest.php index 9a75cc1..25f4266 100644 --- a/tests/Unit/Fix/SimparaFixerTest.php +++ b/tests/Unit/Fix/SimparaFixerTest.php @@ -43,13 +43,14 @@ public function itReplacesSimpleParaTags(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $violations = new SimparaSniff()->process($document, $source); + $sniffer = new SimparaSniff(); + $violations = $sniffer->process($document, $source); self::assertCount(1, $violations); self::assertSame('para', $violations[0]->rangeOne()->content); self::assertSame('para', $violations[0]->rangeTwo()->content); - $fix = new SimparaFixer()->process($violations[0]); + $fix = new ($sniffer::getFixerClassName())()->process($violations[0]); $result = new FixApplier()->apply($source, [$fix]); diff --git a/tests/Unit/Fix/WhitespaceConcernFixersTest.php b/tests/Unit/Fix/WhitespaceConcernFixersTest.php index cdfa081..5067538 100644 --- a/tests/Unit/Fix/WhitespaceConcernFixersTest.php +++ b/tests/Unit/Fix/WhitespaceConcernFixersTest.php @@ -46,19 +46,22 @@ public function itFixesIndependentWhitespaceConcernsTogether(): void $document->loadXML($content); $source = new File('file.xml', $content); - $trailingViolations = new TrailingWhitespaceSniff()->process($document, $source); - $indentationViolations = new MixedIndentationSniff()->process($document, $source); + $trailingSniffer = new TrailingWhitespaceSniff(); + $indentationSniffer = new MixedIndentationSniff(); + + $trailingViolations = $trailingSniffer->process($document, $source); + $indentationViolations = $indentationSniffer->process($document, $source); self::assertCount(2, $trailingViolations); self::assertCount(1, $indentationViolations); $fixes = []; - $trailingFixer = new TrailingWhitespaceFixer(); + $trailingFixer = new ($trailingSniffer::getFixerClassName())(); foreach ($trailingViolations as $violation) { $fixes[] = $trailingFixer->process($violation); } - $indentationFixer = new MixedIndentationFixer(); + $indentationFixer = new ($indentationSniffer::getFixerClassName())(); foreach ($indentationViolations as $violation) { $fixes[] = $indentationFixer->process($violation); } From aa177ddc21eef1bda3882b25ed7310c505ca685b Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:16:11 +0700 Subject: [PATCH 09/24] test: removed obsolete todos ref https://github.com/php/docbook-cs/pull/33#discussion_r3633675044 --- tests/Unit/ApplicationTest.php | 36 +++++++++---------- tests/Unit/Runner/RunReportingTest.php | 2 +- tests/Unit/Runner/SniffRunnerTest.php | 28 +++++++-------- .../Runner/XmlFileProcessorPipelineTest.php | 4 +-- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/tests/Unit/ApplicationTest.php b/tests/Unit/ApplicationTest.php index 197c115..c3de72f 100644 --- a/tests/Unit/ApplicationTest.php +++ b/tests/Unit/ApplicationTest.php @@ -120,7 +120,7 @@ private function readStream(mixed $stream): string return stream_get_contents($stream) ?: ''; } - #[Test] // TODO: should be feature + #[Test] public function itPrintsHelpAndExitsWithZero(): void { $app = new Application(['docbook-cs', '--help'], $this->stdout, $this->stderr); @@ -132,7 +132,7 @@ public function itPrintsHelpAndExitsWithZero(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itPrintsVersionAndExitsWithZero(): void { $app = new Application(['docbook-cs', '--version'], $this->stdout, $this->stderr); @@ -144,7 +144,7 @@ public function itPrintsVersionAndExitsWithZero(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itReturnsErrorWhenConfigCannotBeLoaded(): void { $app = new Application( @@ -159,7 +159,7 @@ public function itReturnsErrorWhenConfigCannotBeLoaded(): void self::assertStringContainsString('Error:', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itHandlesSeparateConfigArgument(): void { $app = new Application( @@ -174,7 +174,7 @@ public function itHandlesSeparateConfigArgument(): void self::assertStringContainsString('Error:', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itAcceptsPathsWithoutCrashing(): void { $app = new Application( @@ -188,7 +188,7 @@ public function itAcceptsPathsWithoutCrashing(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] // TODO: should be feature + #[Test] public function itSupportsQuietFlag(): void { $app = new Application(['docbook-cs', '--quiet'], $this->stdout, $this->stderr); @@ -198,7 +198,7 @@ public function itSupportsQuietFlag(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] // TODO: should be feature + #[Test] public function itSupportsReportFormats(): void { foreach (['console', 'json', 'checkstyle'] as $format) { @@ -214,7 +214,7 @@ public function itSupportsReportFormats(): void } } - #[Test] // TODO: should be feature + #[Test] public function itSupportsColorFlags(): void { foreach (['--colors', '--no-colors'] as $flag) { @@ -230,7 +230,7 @@ public function itSupportsColorFlags(): void } } - #[Test] // TODO: should be feature + #[Test] public function helpShortCircuitsExecution(): void { $app = new Application( @@ -246,7 +246,7 @@ public function helpShortCircuitsExecution(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function versionShortCircuitsExecution(): void { $app = new Application( @@ -262,7 +262,7 @@ public function versionShortCircuitsExecution(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itResolvesRelativeOverridePathsAgainstCwd(): void { $app = new Application( @@ -278,7 +278,7 @@ public function itResolvesRelativeOverridePathsAgainstCwd(): void self::assertNotSame(2, $exitCode); } - #[Test] // TODO: should be feature + #[Test] public function itCatchesRuntimeErrorFromRunner(): void { $app = new Application( @@ -293,7 +293,7 @@ public function itCatchesRuntimeErrorFromRunner(): void self::assertStringContainsString('Runtime error:', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itSupportsSeparateReportArgument(): void { $app = new Application( @@ -307,7 +307,7 @@ public function itSupportsSeparateReportArgument(): void self::assertContains($exitCode, [0, 1, 2]); } - #[Test] // TODO: should be feature + #[Test] public function itPassesThroughAbsoluteOverridePaths(): void { $app = new Application( @@ -321,7 +321,7 @@ public function itPassesThroughAbsoluteOverridePaths(): void self::assertNotSame(2, $exitCode); } - #[Test] // TODO: should be feature + #[Test] public function itSuppressesProgressWhenQuietFlagIsSet(): void { $app = new Application( @@ -336,7 +336,7 @@ public function itSuppressesProgressWhenQuietFlagIsSet(): void self::assertSame('', $this->readStream($this->stderr)); } - #[Test] // TODO: should be feature + #[Test] public function itSuppressesProgressForStructuredReportFormats(): void { foreach (['json', 'checkstyle'] as $format) { @@ -359,7 +359,7 @@ public function itSuppressesProgressForStructuredReportFormats(): void } } - #[Test] // TODO: should be feature + #[Test] public function itShowsPerformanceWhenPerfFlagIsEnabled(): void { $app = new Application( @@ -382,7 +382,7 @@ public function itShowsPerformanceWhenPerfFlagIsEnabled(): void self::assertStringContainsString('PERFORMANCE', $output); } - #[Test] // TODO: should be feature + #[Test] public function itDoesNotShowPerformanceByDefault(): void { $app = new Application( diff --git a/tests/Unit/Runner/RunReportingTest.php b/tests/Unit/Runner/RunReportingTest.php index 2510b8b..5bb7e7c 100644 --- a/tests/Unit/Runner/RunReportingTest.php +++ b/tests/Unit/Runner/RunReportingTest.php @@ -19,7 +19,7 @@ #[CoversNothing] final class RunReportingTest extends TestCase { - #[Test] // TODO: should be integration + #[Test] public function itReportsFixingOutcomeAndPerformance(): void { $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-reporting-'); diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index eca2c75..5a59c3d 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -92,7 +92,7 @@ private function createConfig(array $sniffs = []): ConfigData ); } - #[Test] // TODO: should be integration + #[Test] public function itProcessesFilesWithoutViolations(): void { $config = $this->createConfig(); @@ -105,7 +105,7 @@ public function itProcessesFilesWithoutViolations(): void self::assertCount(2, $report->fileReports); } - #[Test] // TODO: should be integration + #[Test] public function itUsesOverridePathsWhenProvided(): void { $config = $this->createConfig(); @@ -118,7 +118,7 @@ public function itUsesOverridePathsWhenProvided(): void self::assertSame(1, $report->getScannedFilesCount()); } - #[Test] // TODO: should be integration + #[Test] public function itCallsProgressMethods(): void { $progress = $this->createMock(ProgressInterface::class); @@ -139,7 +139,7 @@ public function itCallsProgressMethods(): void $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } - #[Test] // TODO: should be integration + #[Test] public function itAddsFileReportsForFilesWithViolations(): void { $sniff = new class implements SniffInterface { @@ -176,7 +176,7 @@ public function setProperty(string $name, string $value): void self::assertTrue($report->hasFinalViolations()); } - #[Test] // TODO: should be integration + #[Test] public function itStoresAbsolutePathsInFileReports(): void { $sniff = new class implements SniffInterface { @@ -216,7 +216,7 @@ public function setProperty(string $name, string $value): void } } - #[Test] // TODO: should be integration + #[Test] public function itPassesPropertiesToSniffs(): void { $sniffClass = new class implements SniffInterface { @@ -246,7 +246,7 @@ public function process(\DOMDocument $document, File $file): array self::assertSame('someValue', $sniffClass::$captured); } - #[Test] // TODO: should be integration + #[Test] public function itThrowsWhenSniffClassDoesNotExist(): void { $config = $this->createConfig(sniffs: [new SniffEntry('NonExistent\\FakeSniff')]); @@ -259,7 +259,7 @@ public function itThrowsWhenSniffClassDoesNotExist(): void $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } - #[Test] // TODO: should be integration + #[Test] public function itThrowsWhenClassDoesNotImplementSniffInterface(): void { $config = $this->createConfig(sniffs: [new SniffEntry(\stdClass::class)]); @@ -272,7 +272,7 @@ public function itThrowsWhenClassDoesNotImplementSniffInterface(): void $runner->runWithMetrics(new RunPlanner($config)->planPaths($config->getIncludePaths())); } - #[Test] // TODO: should be integration + #[Test] public function itFiltersFilesToOnlyThoseInTheDiff(): void { $config = $this->createConfig(); @@ -284,7 +284,7 @@ public function itFiltersFilesToOnlyThoseInTheDiff(): void self::assertSame(1, $report->getScannedFilesCount()); } - #[Test] // TODO: should be integration + #[Test] public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void { $config = $this->createConfig(); @@ -296,7 +296,7 @@ public function itScansNoFilesWhenDiffContainsNoMatchingPaths(): void self::assertSame(0, $report->getScannedFilesCount()); } - #[Test] // TODO: should be integration + #[Test] public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void { $config = $this->createConfig(); @@ -310,7 +310,7 @@ public function itMatchesWhenDiffPathEqualsDiscoveredPath(): void self::assertSame(1, $report->getScannedFilesCount()); } - #[Test] // TODO: should be integration + #[Test] public function itScansAllFilesWhenNoDiffIsGiven(): void { $config = $this->createConfig(); @@ -321,7 +321,7 @@ public function itScansAllFilesWhenNoDiffIsGiven(): void self::assertSame(2, $report->getScannedFilesCount()); } - #[Test] // TODO: should be integration + #[Test] public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void { $directory = sys_get_temp_dir() . '/docbook-cs-scan-' . bin2hex(random_bytes(6)); @@ -366,7 +366,7 @@ public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void } } - #[Test] // TODO: should be integration + #[Test] public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { $sniff = new class implements SniffInterface { diff --git a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php index a58be5a..5d5ca83 100644 --- a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php +++ b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php @@ -62,7 +62,7 @@ public function itAppliesFixesToTheOriginalSourceWhenEntitiesExpandBeforeTheViol ); } - #[Test] // TODO: should be integration + #[Test] public function itHandlesEntitiesWithoutParseErrors(): void { $xml = $this->xml( @@ -88,7 +88,7 @@ public function itHandlesEntitiesWithoutParseErrors(): void self::assertFalse($fileReport->hasFinalViolations()); } - #[Test] // TODO: should be integration + #[Test] public function itUsesCustomPreprocessor(): void { $xml = $this->xml('&custom.entity;'); From 882d5049eb57bc18d8e87f76e1f81f1ad76976ae Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:23:57 +0700 Subject: [PATCH 10/24] test: simplified simpara source matching --- src/Sniff/SimparaSniff.php | 17 ++--------------- tests/Unit/Sniff/SimparaSniffTest.php | 11 +++++++++++ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Sniff/SimparaSniff.php b/src/Sniff/SimparaSniff.php index 0721d8f..75f5fd5 100644 --- a/src/Sniff/SimparaSniff.php +++ b/src/Sniff/SimparaSniff.php @@ -144,7 +144,7 @@ public function process(\DOMDocument $document, File $file): array throw new \LogicException('Could not map simpara violation to source content.'); } - if ($match['selfClosing']) { + if (null === $closingOffset = $match['closingOffset']) { continue; } @@ -160,11 +160,6 @@ public function process(\DOMDocument $document, File $file): array continue; } - $closingOffset = $match['closingOffset']; - if ($closingOffset === null) { - throw new \LogicException('Could not map simpara violation to source content.'); - } - $affectedRanges = $this->elementNameRanges( $file, $match['beginOffset'], @@ -219,13 +214,7 @@ private function getAllowedElements(): array |> array_values(...); } - /** - * @return list - */ + /** @return list */ private function sourceMatches(File $file): array { preg_match_all( @@ -245,7 +234,6 @@ private function sourceMatches(File $file): array if (str_ends_with(rtrim($tag), '/>')) { $sourceMatches[] = [ 'beginOffset' => $offset, - 'selfClosing' => true, 'closingOffset' => null, ]; continue; @@ -262,7 +250,6 @@ private function sourceMatches(File $file): array $sourceMatches[] = [ 'beginOffset' => $opening, - 'selfClosing' => false, 'closingOffset' => $offset, ]; } diff --git a/tests/Unit/Sniff/SimparaSniffTest.php b/tests/Unit/Sniff/SimparaSniffTest.php index 7133099..69b4d96 100644 --- a/tests/Unit/Sniff/SimparaSniffTest.php +++ b/tests/Unit/Sniff/SimparaSniffTest.php @@ -214,4 +214,15 @@ public function itDoesNotFlagParaInFormalparaRegardlessOfCase(): void self::assertSame([], new SimparaSniff()->process($doc, new File('file.xml', $content))); } + + #[Test] + public function itRejectsSourceThatDoesNotMatchTheParsedDocument(): void + { + $document = $this->createDocument('Text'); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessageIs('Could not map simpara violation to source content.'); + + new SimparaSniff()->process($document, new File('file.xml', '')); + } } From 6ac4382684f140062b4a4db645312864eedee994 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:35:01 +0700 Subject: [PATCH 11/24] test: covered declaration masking --- tests/Support/Sniff/ExposedAbstractSniff.php | 5 +++++ tests/Unit/Sniff/AbstractSniffTest.php | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/tests/Support/Sniff/ExposedAbstractSniff.php b/tests/Support/Sniff/ExposedAbstractSniff.php index 3704de1..8f8fc07 100644 --- a/tests/Support/Sniff/ExposedAbstractSniff.php +++ b/tests/Support/Sniff/ExposedAbstractSniff.php @@ -29,4 +29,9 @@ public function exposeSeverity(): Severity { return $this->severity; } + + public function exposeMaskNonElementMarkup(string $source): string + { + return $this->maskNonElementMarkup($source); + } } diff --git a/tests/Unit/Sniff/AbstractSniffTest.php b/tests/Unit/Sniff/AbstractSniffTest.php index b9684fd..68b4cc4 100644 --- a/tests/Unit/Sniff/AbstractSniffTest.php +++ b/tests/Unit/Sniff/AbstractSniffTest.php @@ -53,4 +53,13 @@ public function itRejectsInvalidSeverityProperties(): void new ExposedAbstractSniff()->setProperty('severity', 'invalid'); } + + #[Test] + public function itMasksAnUnterminatedDeclarationThroughTheEndOfTheSource(): void + { + $source = 'exposeMaskNonElementMarkup($source); + + self::assertSame(str_repeat(' ', strlen($source)), $masked); + } } From 6d5bbea2b5ce00d573866a32fcb57acde9b9c1f1 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:39:53 +0700 Subject: [PATCH 12/24] test: covered valid indentation styles --- tests/Unit/Sniff/WhitespaceConcernSniffsTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php index 6536700..bf87642 100644 --- a/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php +++ b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php @@ -81,6 +81,22 @@ public function itReportsDisjointConcernsOnTheSameLine(): void ); } + #[Test] + public function itAllowsIndentationUsingOnlySpacesOrOnlyTabs(): void + { + foreach ([" ", "\t\t"] as $line) { + $content = "\n{$line}\n"; + + self::assertSame( + [], + new MixedIndentationSniff()->process( + $this->createDocument($content), + new File('file.xml', $content), + ), + ); + } + } + private function createDocument(string $xml): \DOMDocument { $document = new \DOMDocument(); From 710b2f669521923726c5dcb34137bf839a3a411d Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:41:27 +0700 Subject: [PATCH 13/24] test: covered exception name mismatch --- tests/Unit/Sniff/ExceptionNameSniffTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Unit/Sniff/ExceptionNameSniffTest.php b/tests/Unit/Sniff/ExceptionNameSniffTest.php index 7b097fc..2a7f349 100644 --- a/tests/Unit/Sniff/ExceptionNameSniffTest.php +++ b/tests/Unit/Sniff/ExceptionNameSniffTest.php @@ -221,4 +221,18 @@ public function itIgnoresClassnamesInsideCommentsWhenMappingSource(): void self::assertSame('classname', $violations[0]->rangeOne()->content); self::assertSame('classname', $violations[0]->rangeTwo()->content); } + + #[Test] + public function itRejectsSourceThatDoesNotMatchTheParsedDocument(): void + { + $document = $this->createDocument('RuntimeException'); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessageIs('Could not map classname violation to source content.'); + + new ExceptionNameSniff()->process( + $document, + new File('file.xml', 'OtherException'), + ); + } } From d4c1c487bbd9687ad6a29a6cc1fe14408ac593ea Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:41:57 +0700 Subject: [PATCH 14/24] test: covered report edge cases --- tests/Unit/Report/ReportTest.php | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/Unit/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php index 520d9d7..4c5013b 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -78,6 +78,17 @@ public function itRejectsAddingFoundViolationsTwice(): void $fileReport->addFoundViolations([]); } + #[Test] + public function itRejectsAddingASecondFileFailure(): void + { + $fileReport = new FileReport('file.xml'); + $fileReport->addFailedViolation($this->createViolation()); + + $this->expectException(ReportException::class); + + $fileReport->addFailedViolation($this->createViolation()); + } + #[Test] public function itRejectsAddingFinalViolationsBeforeFoundViolations(): void { @@ -223,6 +234,28 @@ public function itReturnsZeroTotalWarningsWhenEmpty(): void self::assertSame(0, $report->getTotalWarningLevelViolationCount()); } + #[Test] + public function itReturnsTotalInfoViolationsAcrossAllFiles(): void + { + $file1 = new FileReport('a.xml'); + $file1->addFoundViolations([ + $this->createViolation(severity: Severity::INFO), + $this->createViolation(severity: Severity::ERROR), + ]); + + $file2 = new FileReport('b.xml'); + $file2->addFoundViolations([ + $this->createViolation(severity: Severity::INFO), + $this->createViolation(severity: Severity::INFO), + ]); + + $report = new Report(); + $report->addFileReport($file1); + $report->addFileReport($file2); + + self::assertSame(3, $report->getTotalInfoLevelViolationCount()); + } + #[Test] public function itHasFinalViolationsWhenFinalViolationsExist(): void { @@ -268,6 +301,7 @@ public function itReturnsAllViolationsFromAllFiles(): void $report = new Report(); $report->addFileReport($file1); $report->addFileReport($file2); + $report->addFileReport(new FileReport('clean.xml')); $all = $report->getAllViolations(); @@ -401,7 +435,9 @@ public function itAggregatesFixerTimes(): void #[Test] public function itMeasuresSniffingAndReturnsTheOperationResult(): void { + $report = new Report(); $fileReport = new FileReport('file.xml', collectPerformance: true); + $report->addFileReport($fileReport); $result = $fileReport->measureSniffing(static function (): string { usleep(1_000); @@ -411,6 +447,7 @@ public function itMeasuresSniffingAndReturnsTheOperationResult(): void self::assertSame('result', $result); self::assertGreaterThan(0.0, $fileReport->totalSniffingTime); + self::assertSame($fileReport->totalSniffingTime, $report->getTotalSniffingTime()); } #[Test] From aa3efc18144cae95841335c7df7b08b0c7cddb39 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:43:44 +0700 Subject: [PATCH 15/24] test: covered input all paths --- tests/Unit/ApplicationInputTest.php | 86 +++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/Unit/ApplicationInputTest.php b/tests/Unit/ApplicationInputTest.php index 1420f2e..f667276 100644 --- a/tests/Unit/ApplicationInputTest.php +++ b/tests/Unit/ApplicationInputTest.php @@ -13,23 +13,38 @@ use DocbookCS\Diff\DiffParser; use DocbookCS\Diff\GitDiffProvider; use DocbookCS\Diff\UpstreamResolver; +use DocbookCS\Fix\Fix; +use DocbookCS\Fix\FixApplier; +use DocbookCS\Fix\FixPlan; +use DocbookCS\Fix\FixResult; +use DocbookCS\Fix\Fixer\ExceptionNameFixer; use DocbookCS\Git\GitClient; use DocbookCS\Git\GitException; use DocbookCS\Path\DiffPathLoader; use DocbookCS\Path\EntityResolver; +use DocbookCS\Path\PathLoader; use DocbookCS\Path\PathMatcher; use DocbookCS\Progress\NullProgress; +use DocbookCS\Report\FileReport; use DocbookCS\Report\Report; use DocbookCS\Report\Reporter\ConsoleReporter; +use DocbookCS\Runner\EntityExpansionMarker; use DocbookCS\Runner\EntityPreprocessor; use DocbookCS\Runner\RunCoordinator; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunPlan; use DocbookCS\Runner\RunPlanner; +use DocbookCS\Runner\RunScope; use DocbookCS\Runner\RunScopeResolver; +use DocbookCS\Runner\ViolationScopeFilter; use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\XmlSniffRunner; use DocbookCS\Sniff\AbstractSniff; +use DocbookCS\Sniff\ExceptionNameSniff; +use DocbookCS\Source\File; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -46,22 +61,37 @@ UsesClass(DiffChangeset::class), UsesClass(DiffParser::class), UsesClass(DiffPathLoader::class), + UsesClass(EntityExpansionMarker::class), UsesClass(EntityPreprocessor::class), UsesClass(EntityResolver::class), + UsesClass(ExceptionNameFixer::class), + UsesClass(ExceptionNameSniff::class), + UsesClass(File::class), + UsesClass(FileReport::class), + UsesClass(Fix::class), + UsesClass(FixApplier::class), + UsesClass(FixPlan::class), + UsesClass(FixResult::class), UsesClass(GitClient::class), UsesClass(GitDiffProvider::class), UsesClass(GitException::class), UsesClass(NullProgress::class), + UsesClass(PathLoader::class), UsesClass(PathMatcher::class), UsesClass(Report::class), UsesClass(RunCoordinator::class), UsesClass(RunMode::class), UsesClass(RunPlan::class), UsesClass(RunPlanner::class), + UsesClass(RunScope::class), UsesClass(RunScopeResolver::class), UsesClass(SniffEntry::class), + UsesClass(SourceRange::class), UsesClass(UpstreamResolver::class), + UsesClass(Violation::class), + UsesClass(ViolationScopeFilter::class), UsesClass(XmlFileProcessor::class), + UsesClass(XmlFixRunner::class), UsesClass(XmlSniffRunner::class), ] final class ApplicationInputTest extends TestCase @@ -119,6 +149,36 @@ public function itDetectsAPipedDiffWithoutAFlag(): void self::assertSame('', $this->readStream($this->stderr)); } + #[Test] + public function itDetectsPipedDiffThroughCliEntryPoint(): void + { + $process = proc_open( + [PHP_BINARY, __DIR__ . '/../../bin/docbook-cs', '--config=' . self::VALID_CONFIG, self::SCAN_FILE], + [ + ['pipe', 'r'], + ['pipe', 'w'], + ['pipe', 'w'], + ], + $pipes, + getcwd() ?: '.', + ); + self::assertIsResource($process); + self::assertCount(3, $pipes); + self::assertIsResource($pipes[0]); + self::assertIsResource($pipes[1]); + self::assertIsResource($pipes[2]); + + fclose($pipes[0]); + $stdout = stream_get_contents($pipes[1]); + $stderr = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + + self::assertSame(2, proc_close($process), $stdout ?: ''); + self::assertIsString($stderr); + self::assertStringContainsString('Paths cannot be combined with diff input', $stderr); + } + #[Test] public function itRejectsPathsCombinedWithAPipedDiff(): void { @@ -160,6 +220,32 @@ public function itAcceptsTheWideOption(): void self::assertSame('', $this->readStream($this->stderr)); } + #[Test] + public function itAppliesFixesInFixMode(): void + { + $temporaryPath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($temporaryPath); + $filePath = $temporaryPath . '.xml'; + rename($temporaryPath, $filePath); + file_put_contents($filePath, 'RuntimeException'); + + try { + $app = new Application( + ['docbook-cs', '--config=' . self::VALID_CONFIG, '--fix', $filePath], + $this->stdout, + $this->stderr, + ); + + self::assertSame(0, $app->run()); + self::assertSame( + 'RuntimeException', + file_get_contents($filePath), + ); + } finally { + @unlink($filePath); + } + } + /** @param resource $stream */ private function readStream(mixed $stream): string { From 9ccabf9486ec4d17e9557311927a24d808afa5e9 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:44:52 +0700 Subject: [PATCH 16/24] test: covered diff edge cases --- tests/Unit/Diff/DiffChangesetTest.php | 37 +++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/Unit/Diff/DiffChangesetTest.php diff --git a/tests/Unit/Diff/DiffChangesetTest.php b/tests/Unit/Diff/DiffChangesetTest.php new file mode 100644 index 0000000..23fea73 --- /dev/null +++ b/tests/Unit/Diff/DiffChangesetTest.php @@ -0,0 +1,37 @@ +changeFor('/project/reference/file.xml')); + } + + #[Test] + public function itReturnsNullForUnchangedPaths(): void + { + $changeset = new DiffChangeset([new FileChange('reference/file.xml', [2])]); + + self::assertNull($changeset->changeFor('/project/reference/other.xml')); + } +} From dc9867d0528ef4030335669059edd494ec1a22d7 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:47:27 +0700 Subject: [PATCH 17/24] test: covered scope edge cases --- tests/Unit/Runner/SourceScopeTest.php | 48 ++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Runner/SourceScopeTest.php b/tests/Unit/Runner/SourceScopeTest.php index 0c73508..3d74c0a 100644 --- a/tests/Unit/Runner/SourceScopeTest.php +++ b/tests/Unit/Runner/SourceScopeTest.php @@ -36,6 +36,7 @@ public function itIncludesOnlyViolationsIntersectingChangedLines(): void self::assertFalse($scope->isWholeFile()); self::assertSame([2], $scope->lineNumbers($file)); + self::assertTrue($scope->includes($this->violation(5, 5, 2))); self::assertTrue($scope->includes($this->violation(4, 7, 2))); self::assertFalse($scope->includes($this->violation(8, 13, 3))); } @@ -48,6 +49,10 @@ public function wholeFileScopeIncludesEveryLine(): void self::assertTrue($scope->isWholeFile()); self::assertSame([1, 2, 3], $scope->lineNumbers($file)); + self::assertTrue($scope->includes($this->violation(8, 13, 3))); + self::assertSame($scope, $scope->after([ + new Fix('file.xml', 0, 0, "zero\n", 'Test'), + ])); } #[Test] @@ -57,6 +62,31 @@ public function anEmptyChangedLineSetSelectsNoLines(): void $scope = RunScope::fromFileAndFileChange($file, new FileChange('file.xml', [])); self::assertSame([], $scope->lineNumbers($file)); + self::assertSame($scope, $scope->after([ + new Fix('file.xml', 0, 0, "zero\n", 'Test'), + ])); + } + + #[Test] + public function itIgnoresDeletionAnchorsOutsideTheSource(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = RunScope::fromFileAndFileChange( + $file, + new FileChange('file.xml', [], deletionAnchors: [10]), + ); + + self::assertSame([], $scope->lineNumbers($file)); + } + + #[Test] + public function itCombinesAdjacentChangedLines(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = RunScope::fromFileAndFileChange($file, new FileChange('file.xml', [2, 3])); + + self::assertSame([2, 3], $scope->lineNumbers($file)); + self::assertTrue($scope->includes($this->violation(7, 9, 2))); } #[Test] @@ -87,6 +117,19 @@ public function itIncludesContentInsertedIntoAnEmptySelectedLine(): void self::assertTrue($scope->includes($this->violation(5, 10, 2))); } + #[Test] + public function itKeepsScopeAlignedWhenAReplacementSpansItsBeginning(): void + { + $file = new File('file.xml', "one\ntwo\nthree"); + $scope = RunScope::fromFileAndFileChange($file, new FileChange('file.xml', [2]))->after([ + new Fix('file.xml', 2, 6, 'X', 'Test'), + ]); + + self::assertSame([1], $scope->lineNumbers($file->withContent("onXo\nthree"))); + self::assertTrue($scope->includes($this->violation(2, 5, 1))); + self::assertFalse($scope->includes($this->violation(5, 10, 2))); + } + #[Test] public function itSortsFixesBeforeMappingScopeOffsets(): void { @@ -123,7 +166,10 @@ public function itAnchorsADeletionAtTheEndOfTheFile(): void new FileChange('file.xml', [], deletionAnchors: [2]), ); - self::assertTrue($scope->includes($this->violation(0, strlen($file->content) + 1, 1))); + $untilOffset = strlen($file->content); + + self::assertTrue($scope->includes($this->violation(0, $untilOffset + 1, 1))); + self::assertTrue($scope->includes($this->violation($untilOffset, $untilOffset, 2))); } private function violation(int $beginOffset, int $untilOffset, int $line): Violation From aadfd5fd69a1452acac95cfd2427c395bd6110ab Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:48:37 +0700 Subject: [PATCH 18/24] test: covered noop fix edge cases --- tests/Unit/Fix/FixPlanTest.php | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/Unit/Fix/FixPlanTest.php b/tests/Unit/Fix/FixPlanTest.php index 490938c..56c27a3 100644 --- a/tests/Unit/Fix/FixPlanTest.php +++ b/tests/Unit/Fix/FixPlanTest.php @@ -24,6 +24,73 @@ ] final class FixPlanTest extends TestCase { + #[Test] + public function itLeavesTheSourceUntouchedWithoutFixes(): void + { + $source = new File('file.xml', ''); + + $result = new FixApplier()->apply($source, []); + + self::assertSame($source, $result->file); + self::assertSame(0, $result->applied); + self::assertSame(0, $result->skipped); + } + + #[Test] + public function itSkipsFixesThatAlreadyHaveTheDesiredContent(): void + { + $source = new File('file.xml', ''); + $fix = new Fix('file.xml', 1, 5, 'root', 'Sniff', 'root'); + + $result = new FixApplier()->apply($source, [$fix]); + + self::assertSame($source->content, $result->file->content); + self::assertSame(0, $result->applied); + self::assertSame(1, $result->skipped); + } + + #[Test] + public function itSkipsCompetingInsertionsAtTheSameOffset(): void + { + $source = new File('file.xml', 'abc'); + $first = new Fix('file.xml', 1, 1, 'X', 'FirstSniff'); + $second = new Fix('file.xml', 1, 1, 'X', 'SecondSniff'); + + $result = new FixApplier()->apply($source, [$first, $second]); + + self::assertSame('aXbc', $result->file->content); + self::assertSame(1, $result->applied); + self::assertSame(1, $result->skipped); + } + + #[Test] + public function itAllowsAnInsertionAtTheStartOfAReplacement(): void + { + $source = new File('file.xml', 'abc'); + $insertion = new Fix('file.xml', 1, 1, 'X', 'InsertionSniff'); + $replacement = new Fix('file.xml', 1, 2, 'B', 'ReplacementSniff', 'b'); + + $result = new FixApplier()->apply($source, [$insertion, $replacement]); + + self::assertSame('aXBc', $result->file->content); + self::assertSame(2, $result->applied); + self::assertSame(0, $result->skipped); + } + + #[Test] + public function itSkipsAnInsertionInsideAReplacement(): void + { + $source = new File('file.xml', 'abc'); + $replacement = new Fix('file.xml', 0, 3, 'ABC', 'ReplacementSniff', 'abc'); + $insertion = new Fix('file.xml', 1, 1, 'X', 'InsertionSniff'); + + $result = new FixApplier()->apply($source, [$replacement, $insertion]); + + self::assertSame('ABC', $result->file->content); + self::assertSame(1, $result->applied); + self::assertSame(1, $result->skipped); + } + #[Test] public function itAppliesEveryFixInAPlanAtomically(): void { From 4517f3ddee6a9cfd9dc6839b07fa2811615b0e9d Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:49:17 +0700 Subject: [PATCH 19/24] test: covered fixer input edge cases --- tests/Unit/Fix/FixerInputValidationTest.php | 130 ++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 tests/Unit/Fix/FixerInputValidationTest.php diff --git a/tests/Unit/Fix/FixerInputValidationTest.php b/tests/Unit/Fix/FixerInputValidationTest.php new file mode 100644 index 0000000..7163752 --- /dev/null +++ b/tests/Unit/Fix/FixerInputValidationTest.php @@ -0,0 +1,130 @@ + $ranges */ + #[Test, DataProvider('missingContent')] + public function itRejectsAffectedRangesWithoutSourceContent(Fixer $fixer, array $ranges): void + { + $this->expectException(FixerException::class); + $this->expectExceptionMessageIs('Fixers require affected source ranges with source content.'); + + $fixer->process($this->violation($ranges)); + } + + /** @return iterable}> */ + public static function missingContent(): iterable + { + yield 'attribute order' => [new AttributeOrderFixer(), [new SourceRange(1, 0, 4)]]; + yield 'exception name' => [new ExceptionNameFixer(), [ + new SourceRange(1, 0, 9), + new SourceRange(1, 10, 19), + ]]; + yield 'mixed indentation' => [new MixedIndentationFixer(), [new SourceRange(1, 0, 2)]]; + yield 'simpara' => [new SimparaFixer(), [ + new SourceRange(1, 0, 4), + new SourceRange(1, 5, 9), + ]]; + yield 'trailing whitespace' => [new TrailingWhitespaceFixer(), [new SourceRange(1, 0, 2)]]; + } + + /** @param non-empty-list $ranges */ + #[Test, DataProvider('invalidContent')] + public function itRejectsInvalidSourceContent(Fixer $fixer, array $ranges): void + { + $this->expectException(FixerException::class); + $m = 'Cannot fix violation T.Sniff in f.xml on line 1 because its source content is not valid fixer input.'; + $this->expectExceptionMessageIs($m); + + $fixer->process($this->violation($ranges)); + } + + /** @return iterable}> */ + public static function invalidContent(): iterable + { + yield 'attribute order' => [new AttributeOrderFixer(), [new SourceRange(1, 0, 6, '')]]; + yield 'exception name' => [new ExceptionNameFixer(), [ + new SourceRange(1, 0, 5, 'class'), + new SourceRange(1, 6, 11, 'class'), + ]]; + yield 'mixed indentation' => [new MixedIndentationFixer(), [new SourceRange(1, 0, 2, ' ')]]; + yield 'simpara' => [new SimparaFixer(), [ + new SourceRange(1, 0, 4, 'span'), + new SourceRange(1, 5, 9, 'span'), + ]]; + yield 'trailing whitespace' => [new TrailingWhitespaceFixer(), [new SourceRange(1, 0, 4, 'text')]]; + } + + #[Test, DataProvider('pairedElementFixers')] + public function itRejectsIncompletePairedElementRanges(Fixer $fixer): void + { + $this->expectException(FixerException::class); + + $fixer->process($this->violation([new SourceRange(1, 0, 4, 'para')])); + } + + /** @return iterable */ + public static function pairedElementFixers(): iterable + { + yield 'exception name' => [new ExceptionNameFixer()]; + yield 'simpara' => [new SimparaFixer()]; + } + + #[Test] + public function itRejectsMalformedAttributeOrderInput(): void + { + $this->expectException(FixerException::class); + + new AttributeOrderFixer()->process($this->violation([ + new SourceRange(1, 0, 9, 'not a tag'), + ])); + } + + #[Test] + public function itRejectsAttributeOrderInputThatIsAlreadyOrdered(): void + { + $content = ''; + + $this->expectException(FixerException::class); + + new AttributeOrderFixer()->process($this->violation([ + new SourceRange(1, 0, strlen($content), $content), + ])); + } + + /** @param non-empty-list $ranges */ + private function violation(array $ranges): Violation + { + return new Violation('T.Sniff', 'f.xml', 'violation.', $ranges); + } +} From ac4839c0bb3af2b76680e00a22c96ded44f385d2 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:49:48 +0700 Subject: [PATCH 20/24] test: academics --- tests/Unit/Runner/FixConvergenceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Runner/FixConvergenceTest.php b/tests/Unit/Runner/FixConvergenceTest.php index 71f5989..a6466b8 100644 --- a/tests/Unit/Runner/FixConvergenceTest.php +++ b/tests/Unit/Runner/FixConvergenceTest.php @@ -39,6 +39,7 @@ use PHPUnit\Framework\TestCase; #[ + CoversClass(FixerException::class), CoversClass(XmlFileProcessor::class), CoversClass(XmlFixRunner::class), CoversClass(XmlSniffRunner::class), @@ -53,7 +54,6 @@ UsesClass(FileReport::class), UsesClass(Fix::class), UsesClass(FixApplier::class), - UsesClass(FixerException::class), UsesClass(FixPlan::class), UsesClass(FixResult::class), UsesClass(Line::class), From 7fde630b108eedf9dc7ad049f81addaa37d669a8 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:50:31 +0700 Subject: [PATCH 21/24] test: academics --- .../Runner/RunCoordinatorFileFailureTest.php | 106 +++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Runner/RunCoordinatorFileFailureTest.php b/tests/Unit/Runner/RunCoordinatorFileFailureTest.php index 83c236d..34e0877 100644 --- a/tests/Unit/Runner/RunCoordinatorFileFailureTest.php +++ b/tests/Unit/Runner/RunCoordinatorFileFailureTest.php @@ -5,16 +5,90 @@ namespace DocbookCS\Tests\Unit\Runner; use DocbookCS\Config\ConfigData; +use DocbookCS\Config\SniffEntry; +use DocbookCS\Diff\DiffBaseResolver; use DocbookCS\Diff\DiffChangeset; use DocbookCS\Diff\FileChange; +use DocbookCS\Diff\GitDiffProvider; +use DocbookCS\Diff\UpstreamResolver; +use DocbookCS\Fix\Fix; +use DocbookCS\Fix\FixApplier; +use DocbookCS\Fix\FixPlan; +use DocbookCS\Fix\FixResult; +use DocbookCS\Fix\Fixer\SimparaFixer; +use DocbookCS\Fix\FixerException; +use DocbookCS\Git\GitClient; +use DocbookCS\Path\DiffPathLoader; +use DocbookCS\Path\EntityResolver; +use DocbookCS\Path\PathLoader; +use DocbookCS\Path\PathMatcher; +use DocbookCS\Progress\NullProgress; use DocbookCS\Progress\ProgressInterface; +use DocbookCS\Report\FileReport; +use DocbookCS\Report\Report; +use DocbookCS\Runner\EntityExpansionMarker; +use DocbookCS\Runner\EntityPreprocessor; use DocbookCS\Runner\RunCoordinator; +use DocbookCS\Runner\RunMode; +use DocbookCS\Runner\RunPlan; use DocbookCS\Runner\RunPlanner; -use PHPUnit\Framework\Attributes\CoversNothing; +use DocbookCS\Runner\RunScope; +use DocbookCS\Runner\RunScopeResolver; +use DocbookCS\Runner\ViolationScopeFilter; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; +use DocbookCS\Runner\XmlSniffRunner; +use DocbookCS\Sniff\AbstractSniff; +use DocbookCS\Sniff\SimparaSniff; +use DocbookCS\Source\File; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -#[CoversNothing] +#[ + CoversClass(FixerException::class), + CoversClass(RunCoordinator::class), + // + UsesClass(AbstractSniff::class), + UsesClass(ConfigData::class), + UsesClass(DiffBaseResolver::class), + UsesClass(DiffChangeset::class), + UsesClass(DiffPathLoader::class), + UsesClass(EntityExpansionMarker::class), + UsesClass(EntityPreprocessor::class), + UsesClass(EntityResolver::class), + UsesClass(File::class), + UsesClass(FileChange::class), + UsesClass(FileReport::class), + UsesClass(Fix::class), + UsesClass(FixApplier::class), + UsesClass(FixPlan::class), + UsesClass(FixResult::class), + UsesClass(GitClient::class), + UsesClass(GitDiffProvider::class), + UsesClass(NullProgress::class), + UsesClass(PathLoader::class), + UsesClass(PathMatcher::class), + UsesClass(Report::class), + UsesClass(RunMode::class), + UsesClass(RunPlan::class), + UsesClass(RunPlanner::class), + UsesClass(RunScope::class), + UsesClass(RunScopeResolver::class), + UsesClass(SimparaFixer::class), + UsesClass(SimparaSniff::class), + UsesClass(SniffEntry::class), + UsesClass(SourceRange::class), + UsesClass(UpstreamResolver::class), + UsesClass(Violation::class), + UsesClass(ViolationScopeFilter::class), + UsesClass(XmlFileProcessor::class), + UsesClass(XmlFixRunner::class), + UsesClass(XmlSniffRunner::class), +] final class RunCoordinatorFileFailureTest extends TestCase { #[Test] @@ -97,4 +171,32 @@ static function () use ($xmlFilePath): void { self::assertTrue($report->hasFinalViolations()); self::assertSame('DocbookCS.Internal', $report->getAllViolations()[0]->sniffCode); } + + #[Test] + public function itFailsClearlyWhenFixedContentCannotBePersisted(): void + { + if (PHP_OS_FAMILY === 'Windows') { + self::markTestSkipped('Unix file permissions are required.'); + } + + $filePath = tempnam(sys_get_temp_dir(), 'docbook-cs-'); + self::assertIsString($filePath); + file_put_contents($filePath, 'Text'); + chmod($filePath, 0444); + + try { + $this->expectException(FixerException::class); + $this->expectExceptionMessageIs('Could not write fixed file: ' . $filePath); + + new RunCoordinator()->runWithMetrics(new RunPlan( + mode: RunMode::Fix, + sniffs: [new SniffEntry(SimparaSniff::class)], + targets: [$filePath => null], + entities: [], + )); + } finally { + chmod($filePath, 0644); + @unlink($filePath); + } + } } From cd66b7e9cd9ed9bec92b9aa8c620cf7519ba5347 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:51:21 +0700 Subject: [PATCH 22/24] test: covered processing failure paths --- .../Runner/XmlFileProcessorPipelineTest.php | 95 ++++++++++++++++++- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php index 5d5ca83..ca85199 100644 --- a/tests/Unit/Runner/XmlFileProcessorPipelineTest.php +++ b/tests/Unit/Runner/XmlFileProcessorPipelineTest.php @@ -4,21 +4,110 @@ namespace DocbookCS\Tests\Unit\Runner; +use DocbookCS\Fix\Fix; +use DocbookCS\Fix\FixApplier; +use DocbookCS\Fix\FixPlan; +use DocbookCS\Fix\FixResult; +use DocbookCS\Fix\Fixer\AttributeOrderFixer; +use DocbookCS\Fix\Fixer\SimparaFixer; use DocbookCS\Report\FileReport; use DocbookCS\Runner\EntityPreprocessor; -use DocbookCS\Runner\XmlFileProcessor; use DocbookCS\Runner\RunMode; use DocbookCS\Runner\RunScope; +use DocbookCS\Runner\ViolationScopeFilter; +use DocbookCS\Runner\XmlFileProcessor; +use DocbookCS\Runner\XmlFixRunner; use DocbookCS\Runner\XmlSniffRunner; +use DocbookCS\Sniff\AbstractSniff; use DocbookCS\Sniff\AttributeOrderSniff; +use DocbookCS\Sniff\Fixable; use DocbookCS\Source\File; -use PHPUnit\Framework\Attributes\CoversNothing; +use DocbookCS\Violation\SourceRange; +use DocbookCS\Violation\Violation; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -#[CoversNothing] +#[ + CoversClass(XmlFileProcessor::class), + // + UsesClass(AbstractSniff::class), + UsesClass(AttributeOrderFixer::class), + UsesClass(AttributeOrderSniff::class), + UsesClass(EntityPreprocessor::class), + UsesClass(File::class), + UsesClass(FileReport::class), + UsesClass(Fix::class), + UsesClass(FixApplier::class), + UsesClass(FixPlan::class), + UsesClass(FixResult::class), + UsesClass(RunMode::class), + UsesClass(RunScope::class), + UsesClass(SimparaFixer::class), + UsesClass(SourceRange::class), + UsesClass(Violation::class), + UsesClass(ViolationScopeFilter::class), + UsesClass(XmlFixRunner::class), + UsesClass(XmlSniffRunner::class), +] final class XmlFileProcessorPipelineTest extends TestCase { + #[Test] + public function itReportsXmlParseErrors(): void + { + $file = new File('input.xml', ''); + $fileReport = new FileReport($file->path); + + $fixedFile = new XmlFileProcessor( + new XmlSniffRunner(RunMode::Sniff, []), + )->process($file, $fileReport, RunScope::fromFileAndFileChange($file, null)); + + self::assertNull($fixedFile); + self::assertSame(1, $fileReport->getFinalViolationCount()); + self::assertSame('DocbookCS.Internal', $fileReport->finalViolations[0]->sniffCode); + } + + #[Test] + public function itStopsWhenAStaleFixCannotBeApplied(): void + { + $file = new File('input.xml', ''); + $fileReport = new FileReport($file->path); + $sniff = new class extends AbstractSniff implements Fixable { + public static function getCode(): string + { + return 'Test.StaleFix'; + } + + public static function getFixerClassName(): string + { + return SimparaFixer::class; + } + + public function process(\DOMDocument $document, File $file): array + { + return [$this->createViolation( + $file->path, + 'Stale fix.', + [ + new SourceRange(1, 0, 4, 'para'), + new SourceRange(1, 4, 8, 'para'), + ], + )]; + } + }; + + $fixedFile = new XmlFileProcessor( + new XmlSniffRunner(RunMode::Fix, [$sniff]), + )->process($file, $fileReport, RunScope::fromFileAndFileChange($file, null)); + + self::assertNull($fixedFile); + self::assertSame(1, $fileReport->getFoundViolationCount()); + self::assertSame(1, $fileReport->getFinalViolationCount()); + self::assertSame(1, $fileReport->fixingPasses); + self::assertFalse($fileReport->changed); + } + #[Test] public function itKeepsTheActualSourcePathInViolations(): void { From 6d1922931d12f09a2a49483a4650c357b5e98c62 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sat, 25 Jul 2026 15:51:46 +0700 Subject: [PATCH 23/24] test: removed @api; now covered by tests --- src/Report/Report.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Report/Report.php b/src/Report/Report.php index f5b03cf..e993874 100644 --- a/src/Report/Report.php +++ b/src/Report/Report.php @@ -187,7 +187,7 @@ public function getTotalWarningLevelViolationCount(): int )); } - /** @api not implemented */ + // not implemented public function getTotalInfoLevelViolationCount(): int { return array_sum(array_map( From c413dc7340e45a54f9d6b611602c9adba3487c8e Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sun, 26 Jul 2026 17:35:49 +0700 Subject: [PATCH 24/24] review: removed yoda style --- src/Sniff/SimparaSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Sniff/SimparaSniff.php b/src/Sniff/SimparaSniff.php index 75f5fd5..e090ff2 100644 --- a/src/Sniff/SimparaSniff.php +++ b/src/Sniff/SimparaSniff.php @@ -144,7 +144,8 @@ public function process(\DOMDocument $document, File $file): array throw new \LogicException('Could not map simpara violation to source content.'); } - if (null === $closingOffset = $match['closingOffset']) { + $closingOffset = $match['closingOffset']; + if (null === $closingOffset) { continue; }