Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
<testsuite name="unit">
<directory>tests/Unit</directory>
</testsuite>
<testsuite name="integration">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's debatable if they are unit tests. Could you merge them into tests/Unit instead?

@NickSdot NickSdot Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, can follow your preference! Before I move them back: the end result in later branches is having Unit, Feature, and Integration. That is because there will be tests that run actual git commands for e2e testing. Given that, still all to Unit?

Ref: https://github.com/NickSdot/php__docbook-cs/tree/stack-09-performance/tests

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, yes. As it would also be more easy to distinguish between tests and non-tests (fixtures & Support).

<directory>tests/Integration</directory>
</testsuite>
</testsuites>

<source restrictNotices="true" restrictWarnings="true">
Expand Down
61 changes: 61 additions & 0 deletions src/Runner/EntityExpansionMarker.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?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 sprintf('<!--%s-->%s<!--%s-->', 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;
}
}
31 changes: 24 additions & 7 deletions src/Runner/EntityPreprocessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand All @@ -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;
Expand All @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion src/Runner/XmlFileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions src/Sniff/AbstractSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use DocbookCS\Report\Severity;
use DocbookCS\Report\Violation;
use DocbookCS\Runner\EntityExpansionMarker;

abstract class AbstractSniff implements SniffInterface
{
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/Sniff/ExceptionNameSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '') {
Expand Down
4 changes: 4 additions & 0 deletions src/Sniff/SimparaSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions tests/Integration/Runner/EntityExpansionMarkerTest.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
62 changes: 62 additions & 0 deletions tests/Integration/Sniff/EntityExpandedSniffTest.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
4 changes: 4 additions & 0 deletions tests/Unit/Sniff/ExceptionNameSniffTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 4 additions & 0 deletions tests/Unit/Sniff/SimparaSniffTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
namespace DocbookCS\Tests\Unit\Sniff;

use DocbookCS\Report\Violation;
use DocbookCS\Runner\EntityExpansionMarker;
use DocbookCS\Sniff\SimparaSniff;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\Attributes\UsesClass;
use PHPUnit\Framework\TestCase;

#[
CoversClass(SimparaSniff::class),
CoversClass(Violation::class),
//
UsesClass(EntityExpansionMarker::class),
]
final class SimparaSniffTest extends TestCase
{
Expand Down