Merge pull request #50 from veewee/immutable-traversal

Fix mutable nodeList reference during traversal
veewee · Jan 26, 2023 · b34b0d6 · b34b0d6
2 parents 6123139 + 2aa54e0
commit b34b0d6
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 3 deletions.
diff --git a/src/Xml/Dom/Traverser/Traverser.php b/src/Xml/Dom/Traverser/Traverser.php
@@ -6,6 +6,7 @@
 
 use DOMNode;
 use function VeeWee\Xml\Dom\Locator\Attribute\attributes_list;
+use function VeeWee\Xml\Dom\Locator\Node\children;
 
 final class Traverser
 {
@@ -30,7 +31,7 @@ public function traverse(DOMNode $node): DOMNode
  $this->traverse($attribute);
  }
 
- foreach ($node->childNodes as $child) {
+ foreach (children($node) as $child) {
  $this->traverse($child);
  }
 

diff --git a/src/Xml/ErrorHandling/detect_issues.php b/src/Xml/ErrorHandling/detect_issues.php
@@ -4,7 +4,6 @@
 
 namespace VeeWee\Xml\ErrorHandling;
 
-use LibXMLError;
 use Psl\Result;
 use Psl\Result\ResultInterface;
 
@@ -31,7 +30,6 @@ function detect_issues(callable $run): array
 
  $result = Result\wrap($run(...));
 
- /** @var list<LibXMLError> $errors */
  $errors = libxml_get_errors();
  libxml_clear_errors();
  libxml_use_internal_errors($previousErrorReporting);

diff --git a/tests/Xml/Dom/Traverser/TraverserTest.php b/tests/Xml/Dom/Traverser/TraverserTest.php
@@ -11,6 +11,7 @@
 use VeeWee\Xml\Dom\Traverser\Traverser;
 use VeeWee\Xml\Dom\Traverser\Visitor\AbstractVisitor;
 use function VeeWee\Xml\Dom\Builder\attribute;
+use function VeeWee\Xml\Dom\Configurator\comparable;
 use function VeeWee\Xml\Dom\Locator\document_element;
 use function VeeWee\Xml\Dom\Mapper\xml_string;
 use function VeeWee\Xml\Dom\Predicate\is_attribute;
@@ -130,4 +131,51 @@ public function onNodeLeave(DOMNode $node): Action
 
  static::assertXmlStringEqualsXmlString($doc->toXmlString(), '<hello enter="yes" leave="yes" />');
  }
+
+
+ public function test_it_can_recursively_remove_empty_nodes(): void
+ {
+ $doc = Document::fromXmlString(
+ <<<EOXML
+ <movies>
+ <movie>
+ <name>Terminator</name>
+ <genre>
+ <action />
+ <romance />
+ </genre>
+ <prices>
+ <oscar />
+ </prices>
+ </movie>
+ </movies>
+ EOXML
+ );
+
+ $transformedNode = $doc->traverse(new class extends AbstractVisitor {
+ public function onNodeLeave(DOMNode $node): Action
+ {
+ if (!is_element($node)) {
+ return new Action\Noop();
+ }
+
+ if (trim($node->textContent) !== '') {
+ return new Action\Noop();
+ }
+
+ return new Action\RemoveNode();
+ }
+ });
+
+ $actual = Document::fromXmlNode($transformedNode, comparable());
+ $expected = Document::fromXmlString(<<<EOXML
+ <movies>
+ <movie>
+ <name>Terminator</name>
+ </movie>
+ </movies>
+ EOXML, comparable());
+
+ static::assertSame($actual->toXmlString(), $expected->toXmlString());
+ }
 }