fix: ignore expanded entity nodes in source-backed sniffs

Author: NickSdot

phpunit.xml.dist

@@ -18,6 +18,9 @@
 		<testsuite name="unit">
 			<directory>tests/Unit</directory>
 		</testsuite>
+		<testsuite name="integration">
+			<directory>tests/Integration</directory>
+		</testsuite>
 	</testsuites>
 
 	<source restrictNotices="true" restrictWarnings="true">

src/Runner/EntityExpansionMarker.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DocbookCS\Runner;
+
+final class EntityExpansionMarker
+{
+    private const string START = 'docbook-cs:entity-expansion:start';
+    private const string END = 'docbook-cs:entity-expansion:end';
+
+    public static function wrap(string $content): string
+    {
+        return '<!--' . self::START . '-->'
+            . $content
+            . '<!--' . self::END . '-->';
+    }
+
+    public static function contains(\DOMNode $node): bool
+    {
+        for ($current = $node; $current->parentNode !== null; $current = $current->parentNode) {
+            if (self::isBetweenMarkers($current)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private static function isBetweenMarkers(\DOMNode $node): bool
+    {
+        $nestedMarkers = 0;
+
+        for ($sibling = $node->previousSibling; $sibling !== null; $sibling = $sibling->previousSibling) {
+            if (self::isEnd($sibling)) {
+                $nestedMarkers++;
+                continue;
+            }
+
+            if (!self::isStart($sibling)) {
+                continue;
+            }
+
+            if ($nestedMarkers === 0) {
+                return true;
+            }
+
+            $nestedMarkers--;
+        }
+
+        return false;
+    }
+
+    private static function isStart(\DOMNode $node): bool
+    {
+        return $node instanceof \DOMComment && $node->textContent === self::START;
+    }
+
+    private static function isEnd(\DOMNode $node): bool
+    {
+        return $node instanceof \DOMComment && $node->textContent === self::END;
+    }
+}

src/Runner/EntityPreprocessor.php

@@ -25,18 +25,28 @@ public function process(string $xml): string
         return $this->expandEntities($xml);
     }
 
-    private function expandEntities(string $content): string
+    public function processForParsing(string $xml): string
+    {
+        $xml = $this->stripDoctype($xml);
+
+        return $this->expandEntities($xml, markXmlExpansions: true);
+    }
+
+    private function expandEntities(string $content, bool $markXmlExpansions = false): string
     {
         $maxDepth = 20;
 
         for ($i = 0; $i < $maxDepth; $i++) {
             $changed = false;
 
             $content = preg_replace_callback(
-                '/<!--[\s\S]*?-->|' . self::ENTITY_PATTERN . '/',
-                function (array $matches) use (&$changed): string {
-                    // If this is a comment, return as is
-                    if (str_starts_with($matches[0], '<!--')) {
+                '/<!--[\s\S]*?-->|<!\[CDATA\[[\s\S]*?\]\]>|<\?[\s\S]*?\?>|' . self::ENTITY_PATTERN . '/',
+                function (array $matches) use (&$changed, $markXmlExpansions): string {
+                    if (
+                        str_starts_with($matches[0], '<!--')
+                        || str_starts_with($matches[0], '<![CDATA[')
+                        || str_starts_with($matches[0], '<?')
+                    ) {
                         return $matches[0];
                     }
 
@@ -51,9 +61,11 @@ function (array $matches) use (&$changed): string {
 
                     $changed = true;
 
-                    $value = $this->entities[$name];
+                    $value = $this->stripXmlDeclaration($this->entities[$name]);
 
-                    return $this->stripXmlDeclaration($value);
+                    return $markXmlExpansions && $this->containsXmlElement($value)
+                        ? EntityExpansionMarker::wrap($value)
+                        : $value;
                 },
                 $content,
             ) ?: $content;
@@ -66,6 +78,11 @@ function (array $matches) use (&$changed): string {
         return $content;
     }
 
+    private function containsXmlElement(string $content): bool
+    {
+        return preg_match('/<\s*[a-zA-Z_][\w:.-]*(?:\s|\/?>)/', $content) === 1;
+    }
+
     private function stripDoctype(string $xmlContent): string
     {
         $start = stripos($xmlContent, '<!DOCTYPE');

src/Runner/XmlFileProcessor.php

@@ -69,7 +69,7 @@ private function processContent(
         FileReport $fileReport,
         ?array $changedLines = null,
     ): FileReport {
-        $content = $this->preprocessor->process($content);
+        $content = $this->preprocessor->processForParsing($content);
 
         $document = $this->parseXml($content, $filePath, $fileReport);
         if ($document === null) {

src/Sniff/AbstractSniff.php

@@ -6,6 +6,7 @@
 
 use DocbookCS\Report\Severity;
 use DocbookCS\Report\Violation;
+use DocbookCS\Runner\EntityExpansionMarker;
 
 abstract class AbstractSniff implements SniffInterface
 {
@@ -22,6 +23,11 @@ protected function getProperty(string $name, string $default = ''): string
         return $this->properties[$name] ?? $default;
     }
 
+    protected function isSourceBacked(\DOMNode $node): bool
+    {
+        return !EntityExpansionMarker::contains($node);
+    }
+
     /** @throws \LogicException if an invalid severity level is configured */
     protected function createViolation(
         string $filePath,

src/Sniff/ExceptionNameSniff.php

@@ -38,6 +38,10 @@ public function process(\DOMDocument $document, string $content, string $filePat
 
         /** @var \DOMElement $node */
         foreach ($classnames as $node) {
+            if (!$this->isSourceBacked($node)) {
+                continue;
+            }
+
             $text = trim($node->textContent);
 
             if ($text === '') {

src/Sniff/SimparaSniff.php

@@ -111,6 +111,10 @@ public function process(\DOMDocument $document, string $content, string $filePat
 
         /** @var \DOMElement $para */
         foreach ($paras as $para) {
+            if (!$this->isSourceBacked($para)) {
+                continue;
+            }
+
             $parent = $para->parentNode;
             if (
                 $parent instanceof \DOMElement

tests/Integration/Runner/EntityExpansionMarkerTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DocbookCS\Tests\Integration\Runner;
+
+use DocbookCS\Runner\EntityExpansionMarker;
+use DocbookCS\Runner\EntityPreprocessor;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+#[
+    CoversClass(EntityExpansionMarker::class),
+    CoversClass(EntityPreprocessor::class),
+]
+final class EntityExpansionMarkerTest extends TestCase
+{
+    #[Test]
+    public function itMarksXmlExpansionsForParsingOnly(): void
+    {
+        $preprocessor = new EntityPreprocessor([
+            'expanded' => '<para>Expanded</para>',
+        ]);
+        $source = '<root>&expanded;</root>';
+        $document = $this->parse($preprocessor->processForParsing($source));
+        $para = $document->getElementsByTagName('para')->item(0);
+        $root = $document->documentElement;
+
+        self::assertSame('<root><para>Expanded</para></root>', $preprocessor->process($source));
+        self::assertInstanceOf(\DOMElement::class, $para);
+        self::assertInstanceOf(\DOMElement::class, $root);
+        self::assertTrue(EntityExpansionMarker::contains($para));
+        self::assertFalse(EntityExpansionMarker::contains($root));
+    }
+
+    #[Test]
+    public function itRecognizesNestedExpansionMarkers(): void
+    {
+        $preprocessor = new EntityPreprocessor([
+            'outer' => '<wrapper>&inner;<after/></wrapper>',
+            'inner' => '<para/>',
+        ]);
+        $document = $this->parse($preprocessor->processForParsing('<root>&outer;</root>'));
+
+        foreach (['wrapper', 'para', 'after'] as $elementName) {
+            $element = $document->getElementsByTagName($elementName)->item(0);
+            self::assertInstanceOf(\DOMElement::class, $element);
+            self::assertTrue(EntityExpansionMarker::contains($element));
+        }
+    }
+
+    #[Test]
+    public function itDoesNotExpandLiteralEntityText(): void
+    {
+        $preprocessor = new EntityPreprocessor(['value' => 'expanded']);
+        $source = '<root><!-- &value; --><![CDATA[&value;]]><?test &value;?>&value;</root>';
+
+        self::assertSame(
+            '<root><!-- &value; --><![CDATA[&value;]]><?test &value;?>expanded</root>',
+            $preprocessor->process($source),
+        );
+    }
+
+    private function parse(string $xml): \DOMDocument
+    {
+        $document = new \DOMDocument();
+        $document->loadXML($xml);
+
+        return $document;
+    }
+}

tests/Integration/Sniff/EntityExpandedSniffTest.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DocbookCS\Tests\Integration\Sniff;
+
+use DocbookCS\Report\Violation;
+use DocbookCS\Runner\EntityExpansionMarker;
+use DocbookCS\Runner\EntityPreprocessor;
+use DocbookCS\Sniff\AbstractSniff;
+use DocbookCS\Sniff\ExceptionNameSniff;
+use DocbookCS\Sniff\SimparaSniff;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+#[
+    CoversClass(AbstractSniff::class),
+    CoversClass(EntityExpansionMarker::class),
+    CoversClass(EntityPreprocessor::class),
+    CoversClass(ExceptionNameSniff::class),
+    CoversClass(SimparaSniff::class),
+    CoversClass(Violation::class),
+]
+final class EntityExpandedSniffTest extends TestCase
+{
+    #[Test]
+    public function simparaIgnoresExpandedElements(): void
+    {
+        $source = '<root><para>Source</para>&expanded;</root>';
+        $document = $this->processedDocument($source, '<para>Expanded</para>');
+
+        $violations = new SimparaSniff()->process($document, $source, 'file.xml');
+
+        self::assertCount(1, $violations);
+        self::assertSame(1, $violations[0]->line);
+    }
+
+    #[Test]
+    public function exceptionNameIgnoresExpandedElements(): void
+    {
+        $source = '<root><classname>RuntimeException</classname>&expanded;</root>';
+        $document = $this->processedDocument(
+            $source,
+            '<classname>ExpandedException</classname>',
+        );
+
+        $violations = new ExceptionNameSniff()->process($document, $source, 'file.xml');
+
+        self::assertCount(1, $violations);
+        self::assertSame(1, $violations[0]->line);
+    }
+
+    private function processedDocument(string $source, string $expanded): \DOMDocument
+    {
+        $content = new EntityPreprocessor(['expanded' => $expanded])->processForParsing($source);
+        $document = new \DOMDocument();
+        $document->loadXML($content);
+
+        return $document;
+    }
+}

tests/Unit/Sniff/ExceptionNameSniffTest.php

@@ -5,14 +5,18 @@
 namespace DocbookCS\Tests\Unit\Sniff;
 
 use DocbookCS\Report\Violation;
+use DocbookCS\Runner\EntityExpansionMarker;
 use DocbookCS\Sniff\ExceptionNameSniff;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
 use PHPUnit\Framework\TestCase;
 
 #[
     CoversClass(ExceptionNameSniff::class),
     CoversClass(Violation::class),
+    //
+    UsesClass(EntityExpansionMarker::class),
 ]
 final class ExceptionNameSniffTest extends TestCase
 {