refactor: extracted common XML parsing logic

Author: NickSdot

src/Config/ConfigParser.php

@@ -4,10 +4,16 @@
 
 namespace DocbookCS\Config;
 
+use DocbookCS\Xml\XmlParser;
+
 final class ConfigParser
 {
     private const string NAMESPACE_URI = 'https://php.github.io/docbook-cs/config';
 
+    public function __construct(private readonly XmlParser $xmlParser = new XmlParser())
+    {
+    }
+
     /**
      * @throws ConfigParserException if the file cannot be read or contains invalid XML.
      * @throws \InvalidArgumentException if a SniffEntry is constructed with an invalid class name.
@@ -19,21 +25,13 @@ public function parseFile(string $filePath): ConfigData
         }
 
         $basePath = dirname(realpath($filePath) ?: '');
+        $xml = $this->xmlParser->parseElement(file_get_contents($filePath) ?: '');
 
-        $previousUseErrors = libxml_use_internal_errors(true);
-        $xml = simplexml_load_string(file_get_contents($filePath) ?: '');
-        $errors = libxml_get_errors();
-        libxml_clear_errors();
-        libxml_use_internal_errors($previousUseErrors);
-
-        if ($xml === false) {
-            $message = $errors !== []
-                ? $errors[0]->message
-                : 'Unknown parse error'; // @codeCoverageIgnore
-            throw ConfigParserException::invalidXml($filePath, trim($message));
+        if ($xml instanceof \LibXMLError) {
+            throw ConfigParserException::invalidXml($filePath, trim($xml->message));
         }
 
-        return $this->parse($xml, $basePath);
+        return $this->createConfigData($xml, $basePath);
     }
 
     /**
@@ -42,27 +40,20 @@ public function parseFile(string $filePath): ConfigData
      */
     public function parseString(string $xmlContent, string $basePath): ConfigData
     {
-        $previousUseErrors = libxml_use_internal_errors(true);
-        $xml = simplexml_load_string($xmlContent);
-        $errors = libxml_get_errors();
-        libxml_clear_errors();
-        libxml_use_internal_errors($previousUseErrors);
-
-        if ($xml === false) {
-            $message = $errors !== []
-                ? $errors[0]->message
-                : 'Unknown parse error'; // @codeCoverageIgnore
-            throw ConfigParserException::invalidXml('(string)', trim($message));
+        $xml = $this->xmlParser->parseElement($xmlContent);
+
+        if ($xml instanceof \LibXMLError) {
+            throw ConfigParserException::invalidXml('(string)', trim($xml->message));
         }
 
-        return $this->parse($xml, $basePath);
+        return $this->createConfigData($xml, $basePath);
     }
 
     /**
      * @throws ConfigParserException if the XML is invalid or required elements/attributes are missing.
      * @throws \InvalidArgumentException if a SniffEntry is constructed with an invalid class name.
      */
-    private function parse(\SimpleXMLElement $root, string $basePath): ConfigData
+    private function createConfigData(\SimpleXMLElement $root, string $basePath): ConfigData
     {
         // Register the namespace for xpath queries.
         $root->registerXPathNamespace('d', self::NAMESPACE_URI);

src/Runner/XmlSniffRunner.php

@@ -10,6 +10,7 @@
 use DocbookCS\Sniff\SniffInterface;
 use DocbookCS\Source\File;
 use DocbookCS\Violation\Violation;
+use DocbookCS\Xml\XmlParser;
 
 final readonly class XmlSniffRunner
 {
@@ -18,7 +19,8 @@ public function __construct(
         private RunMode $mode,
         private array $sniffs,
         private EntityPreprocessor $preprocessor = new EntityPreprocessor(),
-        private ViolationScopeFilter $violationFilter = new ViolationScopeFilter()
+        private XmlParser $xmlParser = new XmlParser(),
+        private ViolationScopeFilter $violationFilter = new ViolationScopeFilter(),
     ) {
     }
 
@@ -94,27 +96,6 @@ private function parseXml(File $file): \DOMDocument|\LibXMLError
     {
         $content = $this->preprocessor->processForParsing($file->content);
 
-        $previousUseErrors = libxml_use_internal_errors(true);
-        libxml_clear_errors();
-
-        try {
-            $document = new \DOMDocument();
-            $document->preserveWhiteSpace = true;
-
-            // LIBXML_NONET prevents network access.
-            // No LIBXML_DTDLOAD needed since we stripped the DOCTYPE.
-            $loaded = $document->loadXML($content, LIBXML_NONET);
-
-            if ($loaded) {
-                return $document;
-            }
-
-            $error = libxml_get_errors()[0];
-        } finally {
-            libxml_clear_errors();
-            libxml_use_internal_errors($previousUseErrors);
-        }
-
-        return $error;
+        return $this->xmlParser->parseDocument($content);
     }
 }

src/Xml/XmlParser.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DocbookCS\Xml;
+
+final class XmlParser
+{
+    public function parseDocument(string $content): \DOMDocument|\LibXMLError
+    {
+        $document = new \DOMDocument();
+        $document->preserveWhiteSpace = true;
+
+        [$loaded, $error] = $this->parseWithErrors(
+            static fn(): bool => $document->loadXML($content, LIBXML_NONET),
+        );
+
+        if ($loaded) {
+            return $document;
+        }
+
+        return $this->parseError($error);
+    }
+
+    public function parseElement(string $content): \SimpleXMLElement|\LibXMLError
+    {
+        [$element, $error] = $this->parseWithErrors(
+            static fn(): \SimpleXMLElement|false => simplexml_load_string($content),
+        );
+
+        if ($element !== false) {
+            return $element;
+        }
+
+        return $this->parseError($error);
+    }
+
+    /**
+     * @template TResult
+     * @param callable(): TResult $parser
+     * @return array{TResult, ?\LibXMLError}
+     */
+    private function parseWithErrors(callable $parser): array
+    {
+        $previousUseErrors = libxml_use_internal_errors(true);
+        libxml_clear_errors();
+
+        try {
+            $result = $parser();
+            $error = libxml_get_errors()[0] ?? null;
+        } finally {
+            libxml_clear_errors();
+            libxml_use_internal_errors($previousUseErrors);
+        }
+
+        return [$result, $error];
+    }
+
+    private function parseError(?\LibXMLError $error): \LibXMLError
+    {
+        $error ??= new \LibXMLError();
+        $error->message ??= 'Unknown parse error';
+        $error->line ??= 0;
+
+        return $error;
+    }
+}

tests/Support/XmlHelper.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DocbookCS\Tests\Support;
+
+trait XmlHelper
+{
+    private function xml(string $body): string
+    {
+        return <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+$body
+XML;
+    }
+}

tests/Unit/ApplicationInputTest.php

@@ -45,6 +45,7 @@
 use DocbookCS\Source\File;
 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;
@@ -92,6 +93,7 @@
     UsesClass(ViolationScopeFilter::class),
     UsesClass(XmlFileProcessor::class),
     UsesClass(XmlFixRunner::class),
+    UsesClass(XmlParser::class),
     UsesClass(XmlSniffRunner::class),
 ]
 final class ApplicationInputTest extends TestCase

tests/Unit/ApplicationTest.php

@@ -41,6 +41,7 @@
 use DocbookCS\Runner\XmlSniffRunner;
 use DocbookCS\Sniff\ExceptionNameSniff;
 use DocbookCS\Source\File;
+use DocbookCS\Xml\XmlParser;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\Attributes\UsesClass;
@@ -85,6 +86,7 @@
     UsesClass(UpstreamResolver::class),
     UsesClass(ViolationScopeFilter::class),
     UsesClass(XmlFileProcessor::class),
+    UsesClass(XmlParser::class),
 ]
 final class ApplicationTest extends TestCase
 {

tests/Unit/Config/ConfigParserTest.php

@@ -8,15 +8,19 @@
 use DocbookCS\Config\ConfigParser;
 use DocbookCS\Config\ConfigParserException;
 use DocbookCS\Config\SniffEntry;
+use DocbookCS\Xml\XmlParser;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
 use PHPUnit\Framework\TestCase;
 
 #[
     CoversClass(ConfigData::class),
     CoversClass(ConfigParser::class),
     CoversClass(ConfigParserException::class),
     CoversClass(SniffEntry::class),
+    //
+    UsesClass(XmlParser::class),
 ]
 final class ConfigParserTest extends TestCase
 {

tests/Unit/Runner/FixConvergenceTest.php

@@ -33,6 +33,7 @@
 use DocbookCS\Tests\Support\Fix\ToggleElementFixer;
 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;
@@ -65,6 +66,7 @@
     UsesClass(SourceRange::class),
     UsesClass(Violation::class),
     UsesClass(ViolationScopeFilter::class),
+    UsesClass(XmlParser::class),
 ]
 final class FixConvergenceTest extends TestCase
 {

tests/Unit/Runner/RunCoordinatorFileFailureTest.php

@@ -43,6 +43,7 @@
 use DocbookCS\Source\File;
 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;
@@ -87,6 +88,7 @@
     UsesClass(ViolationScopeFilter::class),
     UsesClass(XmlFileProcessor::class),
     UsesClass(XmlFixRunner::class),
+    UsesClass(XmlParser::class),
     UsesClass(XmlSniffRunner::class),
 ]
 final class RunCoordinatorFileFailureTest extends TestCase

tests/Unit/Runner/RunScopeTest.php

@@ -42,6 +42,7 @@
 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;
@@ -86,6 +87,7 @@
     UsesClass(ViolationScopeFilter::class),
     UsesClass(XmlFileProcessor::class),
     UsesClass(XmlFixRunner::class),
+    UsesClass(XmlParser::class),
     UsesClass(XmlSniffRunner::class),
 ]
 final class RunScopeTest extends TestCase