Rewrite XMLConverter and HTMLConverter for PHP8.4+

thephpleague · Jan 26, 2025 · 68fd8ed · 68fd8ed
1 parent f95ca7b
commit 68fd8ed
Show file tree

Hide file tree

Showing 11 changed files with 129 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,8 @@ All Notable changes to `Csv` will be documented in this file
 - Fix testing to improve Debian integration [#549](https://github.com/thephpleague/csv/pull/549) by [David Prévot and tenzap](https://github.com/tenzap)
 - `Bom::tryFromSequence` and `Bom::fromSequence` supports the `Reader` and `Writer` classes.
 - `XMLConverter::$formatter` visibility it should not be public.
+- `XMLConverter` internal rewritten to take advantage of PHP8.4 new dom classes
+- `HTMLConverter` internal rewritten to take advantage of PHP8.4 new dom classes
 
 ### Removed
 

diff --git a/docs/9.0/converter/html.md b/docs/9.0/converter/html.md
@@ -11,7 +11,9 @@ The `HTMLConverter` converts a CSV records collection into an HTML Table using P
 
 Prior to converting your records collection into an HTML table, you may wish to configure optional information to improve your table rendering.
 
-<p class="message-warning">Because we are using the <a href="/9.0/converter/xml/">XMLConverter</a> internally, if an error occurs while validating the submitted values, a <code>DOMException</code> exception will be thrown.</p>
+<p class="message-warning">Before version <code>9.22</code> the class was using the <a href="/9.0/converter/xml/">XMLConverter</a> internally.
+This is no longer the case to take advantage of PHP8.4 new functionalities. If an error occurs while validating the submitted values,
+a <code>DOMException</code> exception will be thrown.</p>
 
 ### HTMLConverter::table
 

diff --git a/phpstan.neon b/phpstan.neon
@@ -10,6 +10,12 @@ parameters:
         - '#implements deprecated interface League\\Csv\\ByteSequence#'
         - '#Attribute class Deprecated does not exist.#'
         - '#Parameter \#4 \$params of function stream_filter_(pre|ap)pend expects array, mixed given#'
+        - '#League\\Csv\\HTMLConverter:#'
+        - '#League\\Csv\\XMLConverter:#'
+        - '#Dom\\HTMLElement#'
+        - '#Dom\\XMLDocument#'
+        - '#Dom\\HTMLDocument#'
+        - '#Dom\\Element#'
     level: max
     paths:
         - src

diff --git a/src/AbstractCsvTest.php b/src/AbstractCsvTest.php
@@ -26,6 +26,7 @@
 use function ob_get_clean;
 use function ob_start;
 use function strtolower;
+use function Symfony\Component\VarDumper\Dumper\esc;
 use function tempnam;
 use function tmpfile;
 use function unlink;
@@ -46,7 +47,7 @@ protected function setUp(): void
     {
         $tmp = new SplTempFileObject();
         foreach ($this->expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: "\\");
         }
 
         $this->csv = Reader::createFromFileObject($tmp);

diff --git a/src/CallbackStreamFilterTest.php b/src/CallbackStreamFilterTest.php
@@ -114,7 +114,7 @@ public function it_can_be_added_to_a_stream(): void
         StreamFilter::appendOnReadTo($stream, 'swap.carrier.return');
         StreamFilter::prependOnReadTo($stream, 'toUpper');
         $data = [];
-        while (($record = fgetcsv($stream, 1000, ',')) !== false) {
+        while (($record = fgetcsv($stream, 1000, ',', escape: "\\")) !== false) {
             $data[] = $record;
         }
         fclose($stream);

diff --git a/src/ColumnConsistencyTest.php b/src/ColumnConsistencyTest.php
@@ -32,7 +32,7 @@ protected function tearDown(): void
     {
         $csv = new SplFileObject(__DIR__.'/../test_files/foo.csv', 'w');
         $csv->setCsvControl(escape: '\\');
-        $csv->fputcsv(fields: ['john', 'doe', '[email protected]']);
+        $csv->fputcsv(fields: ['john', 'doe', '[email protected]'], escape: '\\');
         unset($this->csv);
     }
 

diff --git a/src/HTMLConverter.php b/src/HTMLConverter.php
@@ -14,7 +14,9 @@
 namespace League\Csv;
 
 use Closure;
-use Deprecated;
+use Dom\HTMLDocument;
+use Dom\HTMLElement;
+use Dom\XMLDocument;
 use DOMDocument;
 use DOMElement;
 use DOMException;
@@ -30,9 +32,15 @@ class HTMLConverter
     protected string $class_name = 'table-csv-data';
     /** table id attribute value. */
     protected string $id_value = '';
-    protected XMLConverter $xml_converter;
     /** @var ?Closure(array, array-key): array */
     protected ?Closure $formatter = null;
+    protected string $offset_attr = '';
+    protected string $column_attr = '';
+
+    private static function supportsModerDom(): bool
+    {
+        return extension_loaded('dom') && class_exists(HTMLDocument::class);
+    }
 
     public static function create(): self
     {
@@ -48,11 +56,6 @@ public static function create(): self
      */
     public function __construct()
     {
-        $this->xml_converter = XMLConverter::create()
-            ->rootElement('table')
-            ->recordElement('tr')
-            ->fieldElement('td')
-        ;
     }
 
     /**
@@ -67,66 +70,72 @@ public function convert(iterable $records, array $header_record = [], array $foo
             $records = MapIterator::fromIterable($records, $this->formatter);
         }
 
-        $doc = new DOMDocument('1.0');
-        if ([] === $header_record && [] === $footer_record) {
-            $table = $this->xml_converter->import($records, $doc);
-            $this->addHTMLAttributes($table);
-            $doc->appendChild($table);
-
-            /** @var string $content */
-            $content = $doc->saveHTML();
-
-            return $content;
+        $document = self::supportsModerDom() ? HTMLDocument::createEmpty() : new DOMDocument('1.0');
+        $table = $document->createElement('table');
+        if ('' !== $this->class_name) {
+            $table->setAttribute('class', $this->class_name);
         }
 
-        $table = $doc->createElement('table');
+        if ('' !== $this->id_value) {
+            $table->setAttribute('id', $this->id_value);
+        }
 
-        $this->addHTMLAttributes($table);
         $this->appendHeaderSection('thead', $header_record, $table);
         $this->appendHeaderSection('tfoot', $footer_record, $table);
 
-        $table->appendChild($this->xml_converter->rootElement('tbody')->import($records, $doc));
+        $tbody = $table;
+        if ($table->hasChildNodes()) {
+            $tbody = $document->createElement('tbody');
+            $table->appendChild($tbody);
+        }
 
-        $doc->appendChild($table);
+        foreach ($records as $offset => $record) {
+            $tr = $document->createElement('tr');
+            if ('' !== $this->offset_attr) {
+                $tr->setAttribute($this->offset_attr, (string) $offset);
+            }
+
+            foreach ($record as $field_name => $field_value) {
+                $td = $document->createElement('td');
+                if ('' !== $this->column_attr) {
+                    $td->setAttribute($this->column_attr, (string) $field_name);
+                }
+                $td->appendChild($document->createTextNode($field_value));
+                $tr->appendChild($td);
+            }
+
+            $tbody->appendChild($tr);
+        }
+
+        $document->appendChild($table);
 
-        return (string) $doc->saveHTML();
+        return (string) $document->saveHTML($table);
     }
 
     /**
      * Creates a DOMElement representing an HTML table heading section.
      *
      * @throws DOMException
      */
-    protected function appendHeaderSection(string $node_name, array $record, DOMElement $table): void
+    protected function appendHeaderSection(string $node_name, array $record, DOMElement|HTMLElement $table): void
     {
         if ([] === $record) {
             return;
         }
 
-        /** @var DOMDocument $ownerDocument */
-        $ownerDocument = $table->ownerDocument;
-        $node = $this->xml_converter
-            ->rootElement($node_name)
-            ->recordElement('tr')
-            ->fieldElement('th')
-            ->import([$record], $ownerDocument)
-        ;
-
-        /** @var DOMElement $element */
-        foreach ($node->getElementsByTagName('th') as $element) {
-            $element->setAttribute('scope', 'col');
+        /** @var DOMDocument|HTMLDocument $document */
+        $document = $table->ownerDocument;
+        $header = $document->createElement($node_name);
+        $tr = $document->createElement('tr');
+        foreach ($record as $field_value) {
+            $th = $document->createElement('th');
+            $th->setAttribute('scope', 'col');
+            $th->appendChild($document->createTextNode($field_value));
+            $tr->appendChild($th);
         }
 
-        $table->appendChild($node);
-    }
-
-    /**
-     * Adds class and id attributes to an HTML tag.
-     */
-    protected function addHTMLAttributes(DOMElement $node): void
-    {
-        $node->setAttribute('class', $this->class_name);
-        $node->setAttribute('id', $this->id_value);
+        $header->appendChild($tr);
+        $table->appendChild($header);
     }
 
     /**
@@ -150,8 +159,16 @@ public function table(string $class_name, string $id_value = ''): self
      */
     public function tr(string $record_offset_attribute_name): self
     {
+        if ($record_offset_attribute_name === $this->offset_attr) {
+            return $this;
+        }
+
+        if (!self::filterAttributeNme($record_offset_attribute_name)) {
+            throw new DOMException('The submitted attribute name `'.$record_offset_attribute_name.'` is not valid.');
+        }
+
         $clone = clone $this;
-        $clone->xml_converter = $this->xml_converter->recordElement('tr', $record_offset_attribute_name);
+        $clone->offset_attr = $record_offset_attribute_name;
 
         return $clone;
     }
@@ -161,12 +178,33 @@ public function tr(string $record_offset_attribute_name): self
      */
     public function td(string $fieldname_attribute_name): self
     {
+        if ($fieldname_attribute_name === $this->column_attr) {
+            return $this;
+        }
+
+        if (!self::filterAttributeNme($fieldname_attribute_name)) {
+            throw new DOMException('The submitted attribute name `'.$fieldname_attribute_name.'` is not valid.');
+        }
+
         $clone = clone $this;
-        $clone->xml_converter = $this->xml_converter->fieldElement('td', $fieldname_attribute_name);
+        $clone->column_attr = $fieldname_attribute_name;
 
         return $clone;
     }
 
+    private static function filterAttributeNme(string $attribute_name): bool
+    {
+        try {
+            $document = self::supportsModerDom() ? XmlDocument::createEmpty() : new DOMDocument('1.0');
+            $div = $document->createElement('div');
+            $div->setAttribute($attribute_name, 'foo');
+
+            return true;
+        } catch (DOMException) {
+            return false;
+        }
+    }
+
     /**
      * Set a callback to format each item before json encode.
      *

diff --git a/src/ReaderTest.php b/src/ReaderTest.php
@@ -41,7 +41,7 @@ protected function setUp(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($this->expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $this->csv = Reader::createFromFileObject($tmp);
@@ -160,7 +160,7 @@ public function testCall(): void
         $file = new SplTempFileObject();
         $file->setCsvControl(escape: '\\');
         foreach ($raw as $row) {
-            $file->fputcsv($row);
+            $file->fputcsv($row, escape: '\\');
         }
         $csv = Reader::createFromFileObject($file);
         $csv->setHeaderOffset(0);
@@ -368,7 +368,7 @@ public function testJsonSerialize(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $reader = Reader::createFromFileObject($tmp)->setHeaderOffset(0);

diff --git a/src/ResultSetTest.php b/src/ResultSetTest.php
@@ -39,7 +39,7 @@ protected function setUp(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($this->expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $this->csv = Reader::createFromFileObject($tmp);
@@ -198,7 +198,7 @@ public function testFetchAssocWithRowIndex(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($arr as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $csv = Reader::createFromFileObject($tmp);
@@ -229,7 +229,7 @@ public function testFetchColumnInconsistentColumnCSV(): void
         $file = new SplTempFileObject();
         $file->setCsvControl(escape: '\\');
         foreach ($raw as $row) {
-            $file->fputcsv($row);
+            $file->fputcsv($row, escape: '\\');
         }
         $csv = Reader::createFromFileObject($file);
         $res = $this->stmt->process($csv)->fetchColumnByOffset(2);
@@ -247,7 +247,7 @@ public function testFetchColumnEmptyCol(): void
         $file = new SplTempFileObject();
         $file->setCsvControl(escape: '\\');
         foreach ($raw as $row) {
-            $file->fputcsv($row);
+            $file->fputcsv($row, escape: '\\');
         }
         $csv = Reader::createFromFileObject($file);
         $res = $this->stmt->process($csv)->fetchColumnByOffset(2);
@@ -340,7 +340,7 @@ public function testJsonSerialize(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $reader = Reader::createFromFileObject($tmp)->setHeaderOffset(0);

diff --git a/src/StatementTest.php b/src/StatementTest.php
@@ -41,7 +41,7 @@ protected function setUp(): void
         $tmp = new SplTempFileObject();
         $tmp->setCsvControl(escape: '\\');
         foreach ($this->expected as $row) {
-            $tmp->fputcsv($row);
+            $tmp->fputcsv($row, escape: '\\');
         }
 
         $this->csv = Reader::createFromFileObject($tmp);