From 2b1561cae92c31f34544d797fb078e803ac0ad66 Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sun, 6 Sep 2026 12:08:08 +0800 Subject: [PATCH 1/2] feat: added fixer data Co-authored-by: Lars Moelleken --- src/Fix/Fixer/Fixer.php | 6 +++++- src/Sniff/AbstractSniff.php | 15 +++++++++++++-- src/Sniff/Fixable.php | 6 +++++- src/Sniff/SniffInterface.php | 3 ++- src/Violation/Violation.php | 3 +++ tests/Unit/Runner/SniffRunnerTest.php | 6 +++--- tests/Unit/Runner/XmlSniffRunnerTest.php | 6 +++--- tests/Unit/Violation/AffectedRangesTest.php | 16 ++++++++++++++++ 8 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/Fix/Fixer/Fixer.php b/src/Fix/Fixer/Fixer.php index a488c7f..8ddefdb 100644 --- a/src/Fix/Fixer/Fixer.php +++ b/src/Fix/Fixer/Fixer.php @@ -9,8 +9,12 @@ use DocbookCS\Fix\FixerException; use DocbookCS\Violation\Violation; +/** @template TFixerData = mixed */ interface Fixer { - /** @throws FixerException */ + /** + * @param Violation $violation + * @throws FixerException + */ public function process(Violation $violation): Fix|FixPlan; } diff --git a/src/Sniff/AbstractSniff.php b/src/Sniff/AbstractSniff.php index 8bc8997..b73d80a 100644 --- a/src/Sniff/AbstractSniff.php +++ b/src/Sniff/AbstractSniff.php @@ -10,6 +10,10 @@ use DocbookCS\Violation\SourceRange; use DocbookCS\Violation\Violation; +/** + * @template TFixerData = mixed + * @implements SniffInterface + */ abstract class AbstractSniff implements SniffInterface { protected Severity $severity = Severity::ERROR; @@ -72,16 +76,23 @@ protected function elementNameRanges(File $file, int $beginOffset, int $untilOff /** * @param non-empty-list $affectedRanges + * @param TFixerData $fixerData * + * @return Violation * @throws \InvalidArgumentException if the affected ranges are inconsistent */ - protected function createViolation(string $filePath, string $message, array $affectedRanges): Violation - { + protected function createViolation( + string $filePath, + string $message, + array $affectedRanges, + mixed $fixerData = null, + ): Violation { return new Violation( sniffCode: static::getCode(), filePath: $filePath, message: $message, affectedRanges: $affectedRanges, + fixerData: $fixerData, severity: $this->severity, ); } diff --git a/src/Sniff/Fixable.php b/src/Sniff/Fixable.php index 270bfab..d21ce0a 100644 --- a/src/Sniff/Fixable.php +++ b/src/Sniff/Fixable.php @@ -6,8 +6,12 @@ use DocbookCS\Fix\Fixer\Fixer; +/** + * @template TFixerData = mixed + * @extends SniffInterface + */ interface Fixable extends SniffInterface { - /** @return class-string */ + /** @return class-string> */ public static function getFixerClassName(): string; } diff --git a/src/Sniff/SniffInterface.php b/src/Sniff/SniffInterface.php index bb7743e..c29bbce 100644 --- a/src/Sniff/SniffInterface.php +++ b/src/Sniff/SniffInterface.php @@ -11,6 +11,7 @@ * A sniff receives a DOMDocument (already loaded) and its source file, * then returns zero or more findings for reports and optional fixes. */ +/** @template TFixerData = mixed */ interface SniffInterface { /** @@ -21,7 +22,7 @@ public static function getCode(): string; /** * Apply the sniff to the given document. * - * @return list + * @return list> */ public function process(\DOMDocument $document, File $file): array; diff --git a/src/Violation/Violation.php b/src/Violation/Violation.php index 4b1ef29..1c97921 100644 --- a/src/Violation/Violation.php +++ b/src/Violation/Violation.php @@ -4,10 +4,12 @@ namespace DocbookCS\Violation; +/** @template TFixerData = mixed */ final readonly class Violation { /** * @param non-empty-list $affectedRanges + * @param TFixerData $fixerData * @throws \InvalidArgumentException if the affected ranges are inconsistent */ public function __construct( @@ -15,6 +17,7 @@ public function __construct( public string $filePath, public string $message, public array $affectedRanges, + public mixed $fixerData = null, public Severity $severity = Severity::WARNING, ) { if ($affectedRanges === []) { diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 9554e7a..3bafe65 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -144,7 +144,7 @@ public function itCallsProgressMethods(): void #[Test] public function itAddsFileReportsForFilesWithViolations(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -181,7 +181,7 @@ public function setProperty(string $name, string $value): void #[Test] public function itStoresAbsolutePathsInFileReports(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -371,7 +371,7 @@ public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void #[Test] public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; diff --git a/tests/Unit/Runner/XmlSniffRunnerTest.php b/tests/Unit/Runner/XmlSniffRunnerTest.php index b31b2fc..ad2a5cb 100644 --- a/tests/Unit/Runner/XmlSniffRunnerTest.php +++ b/tests/Unit/Runner/XmlSniffRunnerTest.php @@ -312,7 +312,7 @@ public function itReportsNoViolationsInDiffModeWhenNoLinesWereAdded(): void #[Test] public function itDoesNotFixViolationsFromNonFixableSniffs(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.NonFixable'; @@ -347,7 +347,7 @@ public function setProperty(string $name, string $value): void #[Test] public function itThrowsWhenFixableSniffReportsViolationWithoutContentInFixMode(): void { - $sniff = new class implements Fixable { + $sniff = new /** @implements Fixable */ class implements Fixable { public static function getCode(): string { return 'Test.BrokenFixable'; @@ -392,7 +392,7 @@ public function setProperty(string $name, string $value): void /** @param list $lines */ private function sniff(array $lines): SniffInterface { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { /** @var list */ public array $lines = []; diff --git a/tests/Unit/Violation/AffectedRangesTest.php b/tests/Unit/Violation/AffectedRangesTest.php index d9c050d..e44b84c 100644 --- a/tests/Unit/Violation/AffectedRangesTest.php +++ b/tests/Unit/Violation/AffectedRangesTest.php @@ -57,6 +57,22 @@ public function itRejectsUnorderedOrOverlappingAffectedRanges(): void ]); } + #[Test] + public function itTransportsArbitraryFixerData(): void + { + $fixerData = ['replacement' => 'int', 'attributes' => ['role' => 'return']]; + + $violation = new Violation( + 'Test', + 'file.xml', + 'Message', + [new SourceRange(1, 0, 1)], + fixerData: $fixerData, + ); + + self::assertSame($fixerData, $violation->fixerData); + } + #[Test, DataProvider('invalidSourceRanges')] public function itRejectsInvalidSourceRanges(int $line, int $beginOffset, int $untilOffset, ?string $content): void { From 8f57472c839d50e6b4096efd16506ed42d130f2e Mon Sep 17 00:00:00 2001 From: NickSdot Date: Sun, 6 Sep 2026 12:35:23 +0800 Subject: [PATCH 2/2] refactor: unified indentation handling --- docbookcs.xml.dist | 2 + src/Fix/Fixer/IndentationFixer.php | 44 ++++ src/Fix/Fixer/MixedIndentationFixer.php | 38 --- src/IndentationAnalyzer.php | 231 ++++++++++++++++++ src/Sniff/IndentationSniff.php | 67 +++++ src/Sniff/MixedIndentationSniff.php | 59 ----- src/Violation/Violation.php | 2 +- tests/Unit/Fix/FixerInputValidationTest.php | 8 +- tests/Unit/Fix/IndentationFixerTest.php | 200 +++++++++++++++ .../Unit/Fix/WhitespaceConcernFixersTest.php | 71 +++--- tests/Unit/Sniff/IndentationSniffTest.php | 83 +++++++ .../Sniff/WhitespaceConcernSniffsTest.php | 32 +-- tests/Unit/Source/IndentationAnalyzerTest.php | 141 +++++++++++ tests/fixtures/indentation/issue_47.fixed.xml | 39 +++ tests/fixtures/indentation/issue_47.xml | 39 +++ tests/fixtures/indentation/preserved.xml | 41 ++++ 16 files changed, 941 insertions(+), 156 deletions(-) create mode 100644 src/Fix/Fixer/IndentationFixer.php delete mode 100644 src/Fix/Fixer/MixedIndentationFixer.php create mode 100644 src/IndentationAnalyzer.php create mode 100644 src/Sniff/IndentationSniff.php delete mode 100644 src/Sniff/MixedIndentationSniff.php create mode 100644 tests/Unit/Fix/IndentationFixerTest.php create mode 100644 tests/Unit/Sniff/IndentationSniffTest.php create mode 100644 tests/Unit/Source/IndentationAnalyzerTest.php create mode 100644 tests/fixtures/indentation/issue_47.fixed.xml create mode 100644 tests/fixtures/indentation/issue_47.xml create mode 100644 tests/fixtures/indentation/preserved.xml diff --git a/docbookcs.xml.dist b/docbookcs.xml.dist index 51ee17a..012c7f0 100644 --- a/docbookcs.xml.dist +++ b/docbookcs.xml.dist @@ -16,6 +16,8 @@ + + diff --git a/src/Fix/Fixer/IndentationFixer.php b/src/Fix/Fixer/IndentationFixer.php new file mode 100644 index 0000000..afb467e --- /dev/null +++ b/src/Fix/Fixer/IndentationFixer.php @@ -0,0 +1,44 @@ + */ +final class IndentationFixer implements Fixer +{ + private const string INDENTATION_PATTERN = '/^[ \t]*$/D'; + + /** @throws FixerException */ + public function process(Violation $violation): Fix + { + $affectedRange = $violation->rangeOne(); + + if ($affectedRange->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + $expectedDepth = $violation->fixerData; + + if ( + count($violation->affectedRanges) !== 1 + || !preg_match(self::INDENTATION_PATTERN, $affectedRange->content) + || !is_int($expectedDepth) + || $expectedDepth < 0 + ) { + throw FixerException::cannotFixInvalidContent($violation); + } + + $replacement = str_repeat(' ', $expectedDepth); + + if ($affectedRange->content === $replacement) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return Fix::fromViolationAndRange($violation, $affectedRange, $replacement); + } +} diff --git a/src/Fix/Fixer/MixedIndentationFixer.php b/src/Fix/Fixer/MixedIndentationFixer.php deleted file mode 100644 index 8241a46..0000000 --- a/src/Fix/Fixer/MixedIndentationFixer.php +++ /dev/null @@ -1,38 +0,0 @@ -rangeOne(); - - if ($affectedRange->content === null) { - throw FixerException::cannotFixMissingContent(); - } - - if ( - !preg_match(self::INDENTATION_PATTERN, $affectedRange->content) - || !str_contains($affectedRange->content, ' ') - || !str_contains($affectedRange->content, "\t") - ) { - throw FixerException::cannotFixInvalidContent($violation); - } - - return Fix::fromViolationAndRange( - $violation, - $affectedRange, - str_replace("\t", ' ', $affectedRange->content), - ); - } -} diff --git a/src/IndentationAnalyzer.php b/src/IndentationAnalyzer.php new file mode 100644 index 0000000..9934c4f --- /dev/null +++ b/src/IndentationAnalyzer.php @@ -0,0 +1,231 @@ +\/)?(?[a-z_:][a-z0-9_.:-]*)" + . "(?(?:\"[^\"]*\"|'[^']*'|[^'\">])*)>/is"; + private const string XML_SPACE_PATTERN = '/\sxml:space\s*=\s*(["\'])(preserve|default)\1/'; + + /** + * The complete set of elements using db.verbatim.attributes in DocBook 5.2+. + * Their whitespace must be preserved even without xml:space="preserve". + */ + private const array VERBATIM_ELEMENTS = [ + 'address', + 'classsynopsisinfo', + 'funcsynopsisinfo', + 'literallayout', + 'programlisting', + 'screen', + 'synopsis', + 'synopsisinfo', + ]; + + /** + * @return \Generator + * @throws \InvalidArgumentException if a generated source range is inconsistent + */ + public function analyze(File $file): \Generator + { + $maskedContent = $file->contentWithNonElementMarkupMasked(); + + $tagIndex = 0; + $tags = $this->findTags($maskedContent); + + /** @var list $elementStack */ + $elementStack = []; + + foreach ($file->lines() as $line) { + + $firstTag = $tags[$tagIndex] ?? null; + $mismatch = $this->mismatchForLine($line, $maskedContent, $firstTag, $elementStack); + + if ($mismatch !== null) { + yield $mismatch; + } + + while (isset($tags[$tagIndex]) && $tags[$tagIndex]['untilOffset'] <= $line->offsetAfterContent()) { + $this->applyTagToElementStack($tags[$tagIndex], $elementStack); + $tagIndex++; + } + } + } + + /** + * @param SourceTag|null $firstTag + * @param list $elementStack + * @return IndentationMismatch|null + * @throws \InvalidArgumentException if the generated source range is inconsistent + */ + private function mismatchForLine(Line $line, string $maskedContent, ?array $firstTag, array $elementStack): ?array + { + preg_match(self::INDENTATION_PATTERN, $line->content, $matches); + + $indentation = $matches[0] ?? ''; + $contentOffset = $line->beginOffset + strlen($indentation); + + if (!$this->shouldCheckLine($line, $contentOffset, $maskedContent, $firstTag)) { + return null; + } + + $startsWithClosingTag = $firstTag !== null + && $firstTag['beginOffset'] === $contentOffset + && $firstTag['closing']; + + $insidePreservedRegion = $elementStack !== [] + && $elementStack[array_key_last($elementStack)]['preservesWhitespace']; + + if ( + $insidePreservedRegion + && !$this->closesPreservedRegion($startsWithClosingTag, $firstTag, $elementStack) + ) { + return null; + } + + $expectedDepth = max(0, count($elementStack) - ($startsWithClosingTag ? 1 : 0)); + + if ($indentation === str_repeat(' ', $expectedDepth)) { + return null; + } + + return [ + 'range' => new SourceRange( + line: $line->number, + beginOffset: $line->beginOffset, + untilOffset: $contentOffset, + content: $indentation, + ), + 'expectedDepth' => $expectedDepth, + ]; + } + + /** @param SourceTag|null $firstTag */ + private function shouldCheckLine(Line $line, int $contentOffset, string $maskedContent, ?array $firstTag): bool + { + if ($contentOffset >= $line->offsetAfterContent()) { + return false; + } + + if ($maskedContent[$contentOffset] === ' ') { + return false; + } + + if ($firstTag === null) { + return true; + } + + return $contentOffset <= $firstTag['beginOffset'] || $contentOffset >= $firstTag['untilOffset']; + } + + /** + * @param SourceTag|null $tag + * @param list $elementStack + */ + private function closesPreservedRegion(bool $startsWithClosingTag, ?array $tag, array $elementStack): bool + { + if (!$startsWithClosingTag || $tag === null || $elementStack === []) { + return false; + } + + $elementIndex = array_key_last($elementStack); + $element = $elementStack[$elementIndex]; + + $parentPreservesWhitespace = $elementIndex > 0 + && $elementStack[$elementIndex - 1]['preservesWhitespace']; + + return $element['preservesWhitespace'] + && !$parentPreservesWhitespace + && $element['name'] === $tag['name']; + } + + /** + * @param SourceTag $tag + * @param list &$elementStack + */ + private function applyTagToElementStack(array $tag, array &$elementStack): void + { + if ($tag['closing']) { + array_pop($elementStack); + return; + } + + if ($tag['selfClosing']) { + return; + } + + $inherited = $elementStack !== [] + && $elementStack[array_key_last($elementStack)]['preservesWhitespace']; + + $elementStack[] = [ + 'name' => $tag['name'], + 'preservesWhitespace' => $this->preservesWhitespace($tag['name'], $tag['xmlSpace'], $inherited), + ]; + } + + private function preservesWhitespace(string $elementName, ?string $xmlSpace, bool $inherited): bool + { + $separator = strrpos($elementName, ':'); + + $localName = strtolower($separator === false + ? $elementName + : substr($elementName, $separator + 1)); + + if (in_array($localName, self::VERBATIM_ELEMENTS, true)) { + return true; + } + + return match ($xmlSpace) { + 'preserve' => true, + 'default' => false, + default => $inherited, + }; + } + + /** @return list */ + private function findTags(string $maskedContent): array + { + $tags = []; + + preg_match_all(self::TAG_PATTERN, $maskedContent, $matches, PREG_OFFSET_CAPTURE); + + foreach ($matches[0] as $index => [$tag, $beginOffset]) { + + preg_match(self::XML_SPACE_PATTERN, $matches['attributes'][$index][0], $xmlSpaceMatches); + + $tags[] = [ + 'beginOffset' => (int) $beginOffset, + 'untilOffset' => (int) $beginOffset + strlen($tag), + 'name' => $matches['name'][$index][0], + 'closing' => $matches['closing'][$index][0] !== '', + 'selfClosing' => str_ends_with(rtrim($tag), '/>'), + 'xmlSpace' => $xmlSpaceMatches[2] ?? null, + ]; + } + + return $tags; + } +} diff --git a/src/Sniff/IndentationSniff.php b/src/Sniff/IndentationSniff.php new file mode 100644 index 0000000..dc44730 --- /dev/null +++ b/src/Sniff/IndentationSniff.php @@ -0,0 +1,67 @@ + + * @implements Fixable + */ +final class IndentationSniff extends AbstractSniff implements Fixable +{ + private const string REPORTING_MESSAGE = 'Expected indentation of %d %s.'; + + /** @var array */ + private array $reportingMessages = []; + + public function __construct( + private readonly IndentationAnalyzer $analyzer = new IndentationAnalyzer(), + ) {} + + public static function getCode(): string + { + return 'DocbookCS.Indentation'; + } + + public static function getFixerClassName(): string + { + return IndentationFixer::class; + } + + /** + * @throws \InvalidArgumentException if a generated source range is inconsistent + * @throws \OutOfBoundsException if a generated source range lies outside the source + */ + public function process(\DOMDocument $document, File $file): array + { + $violations = []; + + foreach ($this->analyzer->analyze($file) as $mismatch) { + + $expectedDepth = $mismatch['expectedDepth']; + + $violations[] = $this->createViolation( + $file->path, + $this->reportingMessage($expectedDepth), + [$mismatch['range']], + $expectedDepth, + ); + } + + return $violations; + } + + private function reportingMessage(int $expectedDepth): string + { + return $this->reportingMessages[$expectedDepth] ??= sprintf( + self::REPORTING_MESSAGE, + $expectedDepth, + $expectedDepth === 1 ? 'space' : 'spaces', + ); + } +} diff --git a/src/Sniff/MixedIndentationSniff.php b/src/Sniff/MixedIndentationSniff.php deleted file mode 100644 index 9921da2..0000000 --- a/src/Sniff/MixedIndentationSniff.php +++ /dev/null @@ -1,59 +0,0 @@ -lines() as $line) { - if (!preg_match(self::INDENTATION_PATTERN, $line->content, $matches)) { - continue; - } - - $indentation = $matches[0]; - if (!str_contains($indentation, ' ') || !str_contains($indentation, "\t")) { - continue; - } - - $violations[] = $this->createViolation( - $file->path, - self::REPORTING_MESSAGE, - [ - SourceRange::fromFile( - $file, - $line->beginOffset, - $line->beginOffset + strlen($indentation), - ), - ], - ); - } - - return $violations; - } -} diff --git a/src/Violation/Violation.php b/src/Violation/Violation.php index 1c97921..5c3f44b 100644 --- a/src/Violation/Violation.php +++ b/src/Violation/Violation.php @@ -53,7 +53,7 @@ public static function fromFileReadFailure(string $filePath): self filePath: $filePath, message: 'Could not read file.', affectedRanges: [ - new SourceRange(0, 0, 0) + new SourceRange(0, 0, 0), ], severity: Severity::ERROR, ); diff --git a/tests/Unit/Fix/FixerInputValidationTest.php b/tests/Unit/Fix/FixerInputValidationTest.php index 7163752..2dd19fd 100644 --- a/tests/Unit/Fix/FixerInputValidationTest.php +++ b/tests/Unit/Fix/FixerInputValidationTest.php @@ -7,7 +7,7 @@ use DocbookCS\Fix\Fixer\AttributeOrderFixer; use DocbookCS\Fix\Fixer\ExceptionNameFixer; use DocbookCS\Fix\Fixer\Fixer; -use DocbookCS\Fix\Fixer\MixedIndentationFixer; +use DocbookCS\Fix\Fixer\IndentationFixer; use DocbookCS\Fix\Fixer\SimparaFixer; use DocbookCS\Fix\Fixer\TrailingWhitespaceFixer; use DocbookCS\Fix\FixerException; @@ -23,7 +23,7 @@ CoversClass(AttributeOrderFixer::class), CoversClass(ExceptionNameFixer::class), CoversClass(FixerException::class), - CoversClass(MixedIndentationFixer::class), + CoversClass(IndentationFixer::class), CoversClass(SimparaFixer::class), CoversClass(TrailingWhitespaceFixer::class), // @@ -50,7 +50,7 @@ public static function missingContent(): iterable new SourceRange(1, 0, 9), new SourceRange(1, 10, 19), ]]; - yield 'mixed indentation' => [new MixedIndentationFixer(), [new SourceRange(1, 0, 2)]]; + yield 'indentation' => [new IndentationFixer(), [new SourceRange(1, 0, 2)]]; yield 'simpara' => [new SimparaFixer(), [ new SourceRange(1, 0, 4), new SourceRange(1, 5, 9), @@ -77,7 +77,7 @@ public static function invalidContent(): iterable new SourceRange(1, 0, 5, 'class'), new SourceRange(1, 6, 11, 'class'), ]]; - yield 'mixed indentation' => [new MixedIndentationFixer(), [new SourceRange(1, 0, 2, ' ')]]; + yield 'indentation' => [new IndentationFixer(), [new SourceRange(1, 0, 4, 'text')]]; yield 'simpara' => [new SimparaFixer(), [ new SourceRange(1, 0, 4, 'span'), new SourceRange(1, 5, 9, 'span'), diff --git a/tests/Unit/Fix/IndentationFixerTest.php b/tests/Unit/Fix/IndentationFixerTest.php new file mode 100644 index 0000000..8497522 --- /dev/null +++ b/tests/Unit/Fix/IndentationFixerTest.php @@ -0,0 +1,200 @@ +fixture('issue_47.xml')); + $fileReport = new FileReport($source->path); + + $fixedFile = $this->processor()->process( + $source, + $fileReport, + RunScope::fromFileAndFileChange($source, null), + ); + + self::assertNotNull($fixedFile); + self::assertSame($this->fixture('issue_47.fixed.xml'), $fixedFile->content); + self::assertSame(9, $fileReport->getFoundViolationCount()); + self::assertSame(9, $fileReport->getAppliedFixesCount()); + self::assertSame(1, $fileReport->fixingPasses); + self::assertFalse($fileReport->hasFinalViolations()); + } + + #[Test] + public function itReplacesIndentationWithTheProvidedExpectedDepth(): void + { + $violation = new Violation( + IndentationSniff::getCode(), + 'file.xml', + 'Expected indentation.', + [new SourceRange(2, 7, 8, "\t")], + fixerData: 3, + ); + + $fix = new IndentationFixer()->process($violation); + + self::assertEquals(new Fix( + filePath: 'file.xml', + beginOffset: 7, + untilOffset: 8, + replacement: ' ', + sniffCode: IndentationSniff::getCode(), + expectedContent: "\t", + ), $fix); + } + + #[Test] + public function itUsesUnchangedStructureWhenFixingAChangedLine(): void + { + $content = "\n
\n\n
\n"; + $source = new File('file.xml', $content); + $fileReport = new FileReport($source->path); + + $fixedFile = $this->processor()->process( + $source, + $fileReport, + RunScope::fromFileAndFileChange( + $source, + new FileChange($source->path, [3]), + ), + ); + + self::assertNotNull($fixedFile); + self::assertSame( + "\n
\n \n
\n", + $fixedFile->content, + ); + self::assertSame(1, $fileReport->getAppliedFixesCount()); + self::assertFalse($fileReport->hasFinalViolations()); + } + + #[Test, DataProvider('invalidFixerData')] + public function itRejectsInvalidFixerData(mixed $fixerData): void + { + $this->expectException(FixerException::class); + + $violation = new Violation( + IndentationSniff::getCode(), + 'file.xml', + 'Expected indentation.', + [new SourceRange(1, 0, 1, "\t")], + fixerData: $fixerData, + ); + + // Bypass the PHPStan generic contract to exercise the runtime boundary. + new \ReflectionMethod(IndentationFixer::class, 'process')->invoke(new IndentationFixer(), $violation); + } + + /** @return iterable */ + public static function invalidFixerData(): iterable + { + yield 'not an integer' => ['1']; + yield 'negative integer' => [-1]; + } + + #[Test] + public function itRejectsMultipleAffectedRanges(): void + { + $this->expectException(FixerException::class); + + new IndentationFixer()->process(new Violation( + IndentationSniff::getCode(), + 'file.xml', + 'Expected indentation.', + [ + new SourceRange(1, 0, 1, "\t"), + new SourceRange(2, 2, 3, "\t"), + ], + fixerData: 1, + )); + } + + #[Test] + public function itRejectsAnAlreadyCorrectReplacement(): void + { + $this->expectException(FixerException::class); + + new IndentationFixer()->process(new Violation( + IndentationSniff::getCode(), + 'file.xml', + 'Expected indentation.', + [new SourceRange(1, 0, 1, ' ')], + fixerData: 1, + )); + } + + private function fixture(string $name): string + { + $content = file_get_contents(__DIR__ . '/../../fixtures/indentation/' . $name); + self::assertIsString($content); + + return $content; + } + + private function processor(): XmlFileProcessor + { + return new XmlFileProcessor(new XmlSniffRunner( + RunMode::Fix, + [new IndentationSniff()], + )); + } +} diff --git a/tests/Unit/Fix/WhitespaceConcernFixersTest.php b/tests/Unit/Fix/WhitespaceConcernFixersTest.php index 5067538..ac8a8f5 100644 --- a/tests/Unit/Fix/WhitespaceConcernFixersTest.php +++ b/tests/Unit/Fix/WhitespaceConcernFixersTest.php @@ -6,16 +6,26 @@ use DocbookCS\Fix\Fix; use DocbookCS\Fix\FixApplier; -use DocbookCS\Fix\FixPlan; -use DocbookCS\Fix\Fixer\MixedIndentationFixer; +use DocbookCS\Fix\Fixer\IndentationFixer; use DocbookCS\Fix\Fixer\TrailingWhitespaceFixer; +use DocbookCS\Fix\FixPlan; use DocbookCS\Fix\FixResult; -use DocbookCS\Sniff\MixedIndentationSniff; +use DocbookCS\IndentationAnalyzer; +use DocbookCS\Report\FileReport; +use DocbookCS\Runner\EntityPreprocessor; +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\IndentationSniff; use DocbookCS\Sniff\TrailingWhitespaceSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; use DocbookCS\Violation\SourceRange; use DocbookCS\Violation\Violation; +use DocbookCS\Xml\XmlParser; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; @@ -25,16 +35,26 @@ CoversClass(Fix::class), CoversClass(FixApplier::class), CoversClass(FixResult::class), - CoversClass(MixedIndentationFixer::class), - CoversClass(MixedIndentationSniff::class), + CoversClass(IndentationFixer::class), + CoversClass(IndentationSniff::class), CoversClass(TrailingWhitespaceFixer::class), CoversClass(TrailingWhitespaceSniff::class), // + UsesClass(EntityPreprocessor::class), UsesClass(File::class), + UsesClass(FileReport::class), UsesClass(FixPlan::class), + UsesClass(IndentationAnalyzer::class), UsesClass(Line::class), + UsesClass(RunMode::class), + UsesClass(RunScope::class), UsesClass(SourceRange::class), + UsesClass(ViolationScopeFilter::class), UsesClass(Violation::class), + UsesClass(XmlFileProcessor::class), + UsesClass(XmlFixRunner::class), + UsesClass(XmlParser::class), + UsesClass(XmlSniffRunner::class), ] final class WhitespaceConcernFixersTest extends TestCase { @@ -42,34 +62,23 @@ final class WhitespaceConcernFixersTest extends TestCase public function itFixesIndependentWhitespaceConcernsTogether(): void { $content = " \n \t \n"; - $document = new \DOMDocument(); - $document->loadXML($content); $source = new File('file.xml', $content); + $fileReport = new FileReport($source->path); - $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 ($trailingSniffer::getFixerClassName())(); - foreach ($trailingViolations as $violation) { - $fixes[] = $trailingFixer->process($violation); - } - - $indentationFixer = new ($indentationSniffer::getFixerClassName())(); - foreach ($indentationViolations as $violation) { - $fixes[] = $indentationFixer->process($violation); - } - - $result = new FixApplier()->apply($source, $fixes); + $fixedFile = new XmlFileProcessor(new XmlSniffRunner( + RunMode::Fix, + [new TrailingWhitespaceSniff(), new IndentationSniff()], + ))->process( + $source, + $fileReport, + RunScope::fromFileAndFileChange($source, null), + ); - self::assertSame("\n \n", $result->file->content); - self::assertSame(3, $result->applied); - self::assertSame(0, $result->skipped); + self::assertNotNull($fixedFile); + self::assertSame("\n \n", $fixedFile->content); + self::assertSame(3, $fileReport->getFoundViolationCount()); + self::assertSame(3, $fileReport->getAppliedFixesCount()); + self::assertSame(1, $fileReport->fixingPasses); + self::assertFalse($fileReport->hasFinalViolations()); } } diff --git a/tests/Unit/Sniff/IndentationSniffTest.php b/tests/Unit/Sniff/IndentationSniffTest.php new file mode 100644 index 0000000..50fa605 --- /dev/null +++ b/tests/Unit/Sniff/IndentationSniffTest.php @@ -0,0 +1,83 @@ +process($this->fixture('issue_47.xml')); + + self::assertCount(9, $violations); + self::assertContains("\t ", array_map( + static fn(Violation $violation): ?string => $violation->rangeOne()->content, + $violations, + )); + self::assertSame('DocbookCS.Indentation', $violations[0]->sniffCode); + self::assertMatchesRegularExpression('/^Expected indentation of \d+ spaces?\.$/', $violations[0]->message); + self::assertIsInt($violations[0]->fixerData); + } + + #[Test] + public function itAcceptsCanonicalOneSpaceIndentation(): void + { + self::assertSame([], $this->process($this->fixture('issue_47.fixed.xml'))); + } + + #[Test] + public function itLeavesDocBookAndXmlPreservedWhitespaceAlone(): void + { + self::assertSame([], $this->process($this->fixture('preserved.xml'))); + } + + #[Test] + public function itReportsTabsEvenWhenTheyAreNotMixedWithSpaces(): void + { + $violations = $this->process("\n\t\n"); + + self::assertCount(1, $violations); + self::assertSame("\t", $violations[0]->rangeOne()->content); + self::assertSame('Expected indentation of 1 space.', $violations[0]->message); + self::assertSame(1, $violations[0]->fixerData); + } + + /** @return list> */ + private function process(string $content): array + { + $document = new \DOMDocument(); + $document->loadXML($content); + + return new IndentationSniff()->process($document, new File('file.xml', $content)); + } + + private function fixture(string $name): string + { + $content = file_get_contents(__DIR__ . '/../../fixtures/indentation/' . $name); + self::assertIsString($content); + + return $content; + } +} diff --git a/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php index bf87642..ea0da2e 100644 --- a/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php +++ b/tests/Unit/Sniff/WhitespaceConcernSniffsTest.php @@ -4,7 +4,8 @@ namespace DocbookCS\Tests\Unit\Sniff; -use DocbookCS\Sniff\MixedIndentationSniff; +use DocbookCS\IndentationAnalyzer; +use DocbookCS\Sniff\IndentationSniff; use DocbookCS\Sniff\TrailingWhitespaceSniff; use DocbookCS\Source\File; use DocbookCS\Source\Line; @@ -18,10 +19,11 @@ #[ CoversClass(File::class), CoversClass(Line::class), - CoversClass(MixedIndentationSniff::class), + CoversClass(IndentationSniff::class), CoversClass(TrailingWhitespaceSniff::class), CoversClass(Violation::class), // + UsesClass(IndentationAnalyzer::class), UsesClass(SourceRange::class), ] final class WhitespaceConcernSniffsTest extends TestCase @@ -45,18 +47,18 @@ public function itReportsOnlyTrailingWhitespaceAsAffected(): void } #[Test] - public function itReportsOnlyMixedLeadingIndentationAsAffected(): void + public function itReportsOnlyLeadingIndentationAsAffected(): void { $content = "\n \t\n"; $lineOffset = strlen("\n"); - $violations = new MixedIndentationSniff()->process( + $violations = new IndentationSniff()->process( $this->createDocument($content), new File('file.xml', $content), ); self::assertCount(1, $violations); - self::assertSame('DocbookCS.MixedIndentation', $violations[0]->sniffCode); - self::assertSame('Mixed tabs and spaces in indentation.', $violations[0]->message); + self::assertSame('DocbookCS.Indentation', $violations[0]->sniffCode); + self::assertSame('Expected indentation of 1 space.', $violations[0]->message); self::assertSame(" \t", $violations[0]->rangeOne()->content); self::assertSame($lineOffset, $violations[0]->rangeOne()->beginOffset); self::assertSame($lineOffset + 2, $violations[0]->rangeOne()->untilOffset); @@ -70,7 +72,7 @@ public function itReportsDisjointConcernsOnTheSameLine(): void $document = $this->createDocument($content); $source = new File('file.xml', $content); - $indentation = new MixedIndentationSniff()->process($document, $source)[0]; + $indentation = new IndentationSniff()->process($document, $source)[0]; $trailing = new TrailingWhitespaceSniff()->process($document, $source)[0]; self::assertSame(2, $indentation->rangeOne()->line); @@ -81,22 +83,6 @@ 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(); diff --git a/tests/Unit/Source/IndentationAnalyzerTest.php b/tests/Unit/Source/IndentationAnalyzerTest.php new file mode 100644 index 0000000..439bb46 --- /dev/null +++ b/tests/Unit/Source/IndentationAnalyzerTest.php @@ -0,0 +1,141 @@ +mismatches($this->fixture('issue_47.xml'))); + } + + /** @param list $expected */ + #[Test, DataProvider('structuralCases')] + public function itTracksStructuralContext(string $content, array $expected): void + { + self::assertSame($expected, $this->mismatches($content)); + } + + /** @return iterable}> */ + public static function structuralCases(): iterable + { + yield 'xml:space default resets inherited preservation' => [ + <<<'XML' + +
+ +
+ +XML, + [[3, '', 2]], + ]; + + yield 'preserved content is ignored but its boundaries are checked' => [ + <<<'XML' + + +unstructured preserved content + + +XML, + [ + [2, ' ', 1], + [4, ' ', 1], + ], + ]; + + yield 'self-closing elements do not increase the depth' => [ + <<<'XML' + + + + +XML, + [[3, ' ', 1]], + ]; + + yield 'namespace prefixes do not prevent verbatim preservation' => [ + <<<'XML' + + +unstructured preserved content + + +XML, + [], + ]; + + yield 'all tags on a line affect following lines' => [ + <<<'XML' + + + +XML, + [ + [2, ' ', 2], + [3, '', 1], + ], + ]; + + yield 'whitespace-only lines are ignored' => [ + "\n \t \n", + [], + ]; + } + + /** @return list */ + private function mismatches(string $content): array + { + $mismatches = []; + + foreach (new IndentationAnalyzer()->analyze(new File('file.xml', $content)) as $mismatch) { + self::assertNotNull($mismatch['range']->content); + + $mismatches[] = [ + $mismatch['range']->line, + $mismatch['range']->content, + $mismatch['expectedDepth'], + ]; + } + + return $mismatches; + } + + private function fixture(string $name): string + { + $content = file_get_contents(__DIR__ . '/../../fixtures/indentation/' . $name); + self::assertIsString($content); + + return $content; + } +} diff --git a/tests/fixtures/indentation/issue_47.fixed.xml b/tests/fixtures/indentation/issue_47.fixed.xml new file mode 100644 index 0000000..099b31f --- /dev/null +++ b/tests/fixtures/indentation/issue_47.fixed.xml @@ -0,0 +1,39 @@ + + + + + + ImagickPixel <function>getColorCount</function> + + +]]> + + + The output for this will be similar to: + + + + + + + + + + + + compression_alg + + Compression algorithm. Both + compression_alg and compression_level + must be set in order to enable data compression. + + + + + + diff --git a/tests/fixtures/indentation/issue_47.xml b/tests/fixtures/indentation/issue_47.xml new file mode 100644 index 0000000..4c4bdb4 --- /dev/null +++ b/tests/fixtures/indentation/issue_47.xml @@ -0,0 +1,39 @@ + + + + + + ImagickPixel <function>getColorCount</function> + + +]]> + + +The output for this will be similar to: + + + + + + + + + + + + compression_alg + + Compression algorithm. Both + compression_alg and compression_level + must be set in order to enable data compression. + + + + + + diff --git a/tests/fixtures/indentation/preserved.xml b/tests/fixtures/indentation/preserved.xml new file mode 100644 index 0000000..abcb0a0 --- /dev/null +++ b/tests/fixtures/indentation/preserved.xml @@ -0,0 +1,41 @@ + + +]> + + +
+ address whitespace +
+ + class synopsis whitespace + + + function synopsis whitespace + + + literal layout whitespace + + + program listing whitespace + + screen whitespace +]]> + + synopsis whitespace + + + synopsis information whitespace + +
+ +unstructured preserved whitespace + +
+ + modifier whitespace + + +