From fefc2a841e45c01ffe40b31c529a46efc4c119d1 Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Thu, 11 Sep 2025 07:30:49 +0200 Subject: [PATCH 1/3] Extract methods to reduce code duplication and handle error in DOMDocument::saveXML() for #1092 --- ChangeLog-11.0.md | 7 ++++++ src/Report/Clover.php | 20 ++++++--------- src/Report/Cobertura.php | 23 ++++++----------- src/Report/Crap4j.php | 20 ++++++--------- src/Report/PHP.php | 16 +++++------- src/Report/Xml/Facade.php | 37 +++------------------------ src/Util/Filesystem.php | 20 +++++++++++++++ src/Util/Xml.php | 53 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 111 insertions(+), 85 deletions(-) create mode 100644 src/Util/Xml.php diff --git a/ChangeLog-11.0.md b/ChangeLog-11.0.md index b32148616..ff7d4538e 100644 --- a/ChangeLog-11.0.md +++ b/ChangeLog-11.0.md @@ -2,6 +2,12 @@ All notable changes are documented in this file using the [Keep a CHANGELOG](http://keepachangelog.com/) principles. +## [11.0.11] - 2025-MM-DD + +### Fixed + +* [#1092](https://github.com/sebastianbergmann/php-code-coverage/issues/1092): Error in `DOMDocument::saveXML()` is not handled + ## [11.0.11] - 2025-08-27 ### Changed @@ -87,6 +93,7 @@ All notable changes are documented in this file using the [Keep a CHANGELOG](htt * This component now requires PHP-Parser 5 * This component is no longer supported on PHP 8.1 +[11.0.11]: https://github.com/sebastianbergmann/php-code-coverage/compare/11.0.11...11.0 [11.0.11]: https://github.com/sebastianbergmann/php-code-coverage/compare/11.0.10...11.0.11 [11.0.10]: https://github.com/sebastianbergmann/php-code-coverage/compare/11.0.9...11.0.10 [11.0.9]: https://github.com/sebastianbergmann/php-code-coverage/compare/11.0.8...11.0.9 diff --git a/src/Report/Clover.php b/src/Report/Clover.php index 4a0bd409c..14d3a15b5 100644 --- a/src/Report/Clover.php +++ b/src/Report/Clover.php @@ -10,31 +10,31 @@ namespace SebastianBergmann\CodeCoverage\Report; use function count; -use function dirname; -use function file_put_contents; use function is_string; use function ksort; use function max; use function range; -use function str_contains; use function time; use DOMDocument; use SebastianBergmann\CodeCoverage\CodeCoverage; use SebastianBergmann\CodeCoverage\Driver\WriteOperationFailedException; use SebastianBergmann\CodeCoverage\Node\File; use SebastianBergmann\CodeCoverage\Util\Filesystem; +use SebastianBergmann\CodeCoverage\Util\Xml; final class Clover { /** + * @param null|non-empty-string $target + * @param null|non-empty-string $name + * * @throws WriteOperationFailedException */ public function process(CodeCoverage $coverage, ?string $target = null, ?string $name = null): string { $time = (string) time(); - $xmlDocument = new DOMDocument('1.0', 'UTF-8'); - $xmlDocument->formatOutput = true; + $xmlDocument = new DOMDocument('1.0', 'UTF-8'); $xmlCoverage = $xmlDocument->createElement('coverage'); $xmlCoverage->setAttribute('generated', $time); @@ -214,16 +214,10 @@ public function process(CodeCoverage $coverage, ?string $target = null, ?string $xmlMetrics->setAttribute('coveredelements', (string) ($report->numberOfTestedMethods() + $report->numberOfExecutedLines() + $report->numberOfExecutedBranches())); $xmlProject->appendChild($xmlMetrics); - $buffer = $xmlDocument->saveXML(); + $buffer = Xml::asString($xmlDocument); if ($target !== null) { - if (!str_contains($target, '://')) { - Filesystem::createDirectory(dirname($target)); - } - - if (@file_put_contents($target, $buffer) === false) { - throw new WriteOperationFailedException($target); - } + Filesystem::write($target, $buffer); } return $buffer; diff --git a/src/Report/Cobertura.php b/src/Report/Cobertura.php index 215fc0c54..136070bda 100644 --- a/src/Report/Cobertura.php +++ b/src/Report/Cobertura.php @@ -12,11 +12,8 @@ use const DIRECTORY_SEPARATOR; use function basename; use function count; -use function dirname; -use function file_put_contents; use function preg_match; use function range; -use function str_contains; use function str_replace; use function time; use DOMImplementation; @@ -24,10 +21,13 @@ use SebastianBergmann\CodeCoverage\Driver\WriteOperationFailedException; use SebastianBergmann\CodeCoverage\Node\File; use SebastianBergmann\CodeCoverage\Util\Filesystem; +use SebastianBergmann\CodeCoverage\Util\Xml; final class Cobertura { /** + * @param null|non-empty-string $target + * * @throws WriteOperationFailedException */ public function process(CodeCoverage $coverage, ?string $target = null): string @@ -44,10 +44,9 @@ public function process(CodeCoverage $coverage, ?string $target = null): string 'http://cobertura.sourceforge.net/xml/coverage-04.dtd', ); - $document = $implementation->createDocument('', '', $documentType); - $document->xmlVersion = '1.0'; - $document->encoding = 'UTF-8'; - $document->formatOutput = true; + $document = $implementation->createDocument('', '', $documentType); + $document->xmlVersion = '1.0'; + $document->encoding = 'UTF-8'; $coverageElement = $document->createElement('coverage'); @@ -289,16 +288,10 @@ public function process(CodeCoverage $coverage, ?string $target = null): string $coverageElement->setAttribute('complexity', (string) $complexity); - $buffer = $document->saveXML(); + $buffer = Xml::asString($document); if ($target !== null) { - if (!str_contains($target, '://')) { - Filesystem::createDirectory(dirname($target)); - } - - if (@file_put_contents($target, $buffer) === false) { - throw new WriteOperationFailedException($target); - } + Filesystem::write($target, $buffer); } return $buffer; diff --git a/src/Report/Crap4j.php b/src/Report/Crap4j.php index cb1bde605..f7381c1eb 100644 --- a/src/Report/Crap4j.php +++ b/src/Report/Crap4j.php @@ -10,17 +10,15 @@ namespace SebastianBergmann\CodeCoverage\Report; use function date; -use function dirname; -use function file_put_contents; use function htmlspecialchars; use function is_string; use function round; -use function str_contains; use DOMDocument; use SebastianBergmann\CodeCoverage\CodeCoverage; use SebastianBergmann\CodeCoverage\Driver\WriteOperationFailedException; use SebastianBergmann\CodeCoverage\Node\File; use SebastianBergmann\CodeCoverage\Util\Filesystem; +use SebastianBergmann\CodeCoverage\Util\Xml; final class Crap4j { @@ -32,12 +30,14 @@ public function __construct(int $threshold = 30) } /** + * @param null|non-empty-string $target + * @param null|non-empty-string $name + * * @throws WriteOperationFailedException */ public function process(CodeCoverage $coverage, ?string $target = null, ?string $name = null): string { - $document = new DOMDocument('1.0', 'UTF-8'); - $document->formatOutput = true; + $document = new DOMDocument('1.0', 'UTF-8'); $root = $document->createElement('crap_result'); $document->appendChild($root); @@ -119,16 +119,10 @@ public function process(CodeCoverage $coverage, ?string $target = null, ?string $root->appendChild($stats); $root->appendChild($methodsNode); - $buffer = $document->saveXML(); + $buffer = Xml::asString($document); if ($target !== null) { - if (!str_contains($target, '://')) { - Filesystem::createDirectory(dirname($target)); - } - - if (@file_put_contents($target, $buffer) === false) { - throw new WriteOperationFailedException($target); - } + Filesystem::write($target, $buffer); } return $buffer; diff --git a/src/Report/PHP.php b/src/Report/PHP.php index f4f874a30..e02bd25cb 100644 --- a/src/Report/PHP.php +++ b/src/Report/PHP.php @@ -10,16 +10,18 @@ namespace SebastianBergmann\CodeCoverage\Report; use const PHP_EOL; -use function dirname; -use function file_put_contents; use function serialize; -use function str_contains; use SebastianBergmann\CodeCoverage\CodeCoverage; use SebastianBergmann\CodeCoverage\Driver\WriteOperationFailedException; use SebastianBergmann\CodeCoverage\Util\Filesystem; final class PHP { + /** + * @param null|non-empty-string $target + * + * @throws WriteOperationFailedException + */ public function process(CodeCoverage $coverage, ?string $target = null): string { $coverage->clearCache(); @@ -28,13 +30,7 @@ public function process(CodeCoverage $coverage, ?string $target = null): string return \unserialize(<<<'END_OF_COVERAGE_SERIALIZATION'" . PHP_EOL . serialize($coverage) . PHP_EOL . 'END_OF_COVERAGE_SERIALIZATION' . PHP_EOL . ');'; if ($target !== null) { - if (!str_contains($target, '://')) { - Filesystem::createDirectory(dirname($target)); - } - - if (@file_put_contents($target, $buffer) === false) { - throw new WriteOperationFailedException($target); - } + Filesystem::write($target, $buffer); } return $buffer; diff --git a/src/Report/Xml/Facade.php b/src/Report/Xml/Facade.php index 3264718cf..5c78dcebc 100644 --- a/src/Report/Xml/Facade.php +++ b/src/Report/Xml/Facade.php @@ -10,18 +10,13 @@ namespace SebastianBergmann\CodeCoverage\Report\Xml; use const DIRECTORY_SEPARATOR; -use const PHP_EOL; use function count; use function dirname; use function file_get_contents; -use function file_put_contents; use function is_array; use function is_dir; use function is_file; use function is_writable; -use function libxml_clear_errors; -use function libxml_get_errors; -use function libxml_use_internal_errors; use function sprintf; use function strlen; use function substr; @@ -33,7 +28,9 @@ use SebastianBergmann\CodeCoverage\Node\AbstractNode; use SebastianBergmann\CodeCoverage\Node\Directory as DirectoryNode; use SebastianBergmann\CodeCoverage\Node\File as FileNode; +use SebastianBergmann\CodeCoverage\Util\Filesystem; use SebastianBergmann\CodeCoverage\Util\Filesystem as DirectoryUtil; +use SebastianBergmann\CodeCoverage\Util\Xml; use SebastianBergmann\CodeCoverage\Version; use SebastianBergmann\CodeCoverage\XmlException; use SebastianBergmann\Environment\Runtime; @@ -269,36 +266,8 @@ private function saveDocument(DOMDocument $document, string $name): void { $filename = sprintf('%s/%s.xml', $this->targetDirectory(), $name); - $document->formatOutput = true; - $document->preserveWhiteSpace = false; $this->initTargetDirectory(dirname($filename)); - file_put_contents($filename, $this->documentAsString($document)); - } - - /** - * @throws XmlException - * - * @see https://bugs.php.net/bug.php?id=79191 - */ - private function documentAsString(DOMDocument $document): string - { - $xmlErrorHandling = libxml_use_internal_errors(true); - $xml = $document->saveXML(); - - if ($xml === false) { - $message = 'Unable to generate the XML'; - - foreach (libxml_get_errors() as $error) { - $message .= PHP_EOL . $error->message; - } - - throw new XmlException($message); - } - - libxml_clear_errors(); - libxml_use_internal_errors($xmlErrorHandling); - - return $xml; + Filesystem::write($filename, Xml::asString($document)); } } diff --git a/src/Util/Filesystem.php b/src/Util/Filesystem.php index 0e99b1593..1c4aa3802 100644 --- a/src/Util/Filesystem.php +++ b/src/Util/Filesystem.php @@ -9,9 +9,13 @@ */ namespace SebastianBergmann\CodeCoverage\Util; +use function dirname; +use function file_put_contents; use function is_dir; use function mkdir; use function sprintf; +use function str_contains; +use SebastianBergmann\CodeCoverage\Driver\WriteOperationFailedException; /** * @internal This class is not covered by the backward compatibility promise for phpunit/php-code-coverage @@ -34,4 +38,20 @@ public static function createDirectory(string $directory): void ); } } + + /** + * @param non-empty-string $target + * + * @throws WriteOperationFailedException + */ + public static function write(string $target, string $buffer): void + { + if (!str_contains($target, '://')) { + self::createDirectory(dirname($target)); + } + + if (@file_put_contents($target, $buffer) === false) { + throw new WriteOperationFailedException($target); + } + } } diff --git a/src/Util/Xml.php b/src/Util/Xml.php new file mode 100644 index 000000000..de958a4b2 --- /dev/null +++ b/src/Util/Xml.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace SebastianBergmann\CodeCoverage\Util; + +use const PHP_EOL; +use function libxml_clear_errors; +use function libxml_get_errors; +use function libxml_use_internal_errors; +use DOMDocument; +use SebastianBergmann\CodeCoverage\XmlException; + +/** + * @internal This class is not covered by the backward compatibility promise for phpunit/php-code-coverage + */ +final readonly class Xml +{ + /** + * @throws XmlException + * + * @see https://bugs.php.net/bug.php?id=79191 + */ + public static function asString(DOMDocument $document): string + { + $xmlErrorHandling = libxml_use_internal_errors(true); + + $document->formatOutput = true; + $document->preserveWhiteSpace = false; + + $buffer = $document->saveXML(); + + if ($buffer === false) { + $message = 'Unable to generate the XML'; + + foreach (libxml_get_errors() as $error) { + $message .= PHP_EOL . $error->message; + } + + throw new XmlException($message); + } + + libxml_clear_errors(); + libxml_use_internal_errors($xmlErrorHandling); + + return $buffer; + } +} From da3b6149d7370499e24d5e1d3fc517515cb31223 Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Thu, 11 Sep 2025 07:57:08 +0200 Subject: [PATCH 2/3] Use Xml::asString() and Filesystem::write() --- src/Report/OpenClover.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/Report/OpenClover.php b/src/Report/OpenClover.php index 042132b33..65d409b1c 100644 --- a/src/Report/OpenClover.php +++ b/src/Report/OpenClover.php @@ -12,13 +12,10 @@ use function assert; use function basename; use function count; -use function dirname; -use function file_put_contents; use function is_string; use function ksort; use function max; use function range; -use function str_contains; use function str_replace; use function time; use DOMDocument; @@ -26,6 +23,7 @@ use SebastianBergmann\CodeCoverage\CodeCoverage; use SebastianBergmann\CodeCoverage\Node\File; use SebastianBergmann\CodeCoverage\Util\Filesystem; +use SebastianBergmann\CodeCoverage\Util\Xml; use SebastianBergmann\CodeCoverage\Version; use SebastianBergmann\CodeCoverage\WriteOperationFailedException; @@ -240,16 +238,10 @@ public function process(CodeCoverage $coverage, ?string $target = null, ?string $xmlMetrics->setAttribute('coveredmethods', (string) $report->numberOfTestedMethods()); $xmlProject->insertBefore($xmlMetrics, $xmlProject->firstChild); - $buffer = $xmlDocument->saveXML(); + $buffer = Xml::asString($xmlDocument); if ($target !== null) { - if (!str_contains($target, '://')) { - Filesystem::createDirectory(dirname($target)); - } - - if (@file_put_contents($target, $buffer) === false) { - throw new WriteOperationFailedException($target); - } + Filesystem::write($target, $buffer); } return $buffer; From cd268c6055667e5efbf7df18e0553657858551a4 Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Thu, 11 Sep 2025 08:09:46 +0200 Subject: [PATCH 3/3] Narrow types --- tests/tests/Target/MapBuilderTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/tests/Target/MapBuilderTest.php b/tests/tests/Target/MapBuilderTest.php index 0c03682b4..49c1c0ed6 100644 --- a/tests/tests/Target/MapBuilderTest.php +++ b/tests/tests/Target/MapBuilderTest.php @@ -418,6 +418,10 @@ public static function provider(): array ]; } + /** + * @param TargetMap $expected, + * @param non-empty-list $files + */ #[DataProvider('provider')] public function testBuildsMap(array $expected, array $files): void {