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
42 changes: 37 additions & 5 deletions src/XmlNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,13 @@ public function removeTagName(string $tagName): bool
}
}


public function toArray(Closure $func = null): array
public function toFullArray(): array
{
$sxml = simplexml_import_dom($this->DOMNode());
return [$sxml->getName() => $this->_toArray($sxml)];
return [$sxml->getName() => $this->_toFullArray($sxml)];
}
Comment on lines +263 to 267
Copy link

Choose a reason for hiding this comment

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

Missing error handling for invalid XML in toFullArray category Error Handling

Tell me more
What is the issue?

The toFullArray() method doesn't handle the case where simplexml_import_dom() returns false, which can happen with invalid XML nodes.

Why this matters

If the XML node is invalid, the method will trigger a PHP error when trying to call getName() on a false value, causing unexpected crashes.

Suggested change ∙ Feature Preview

Add null check and throw an exception for invalid XML:

public function toFullArray(): array
{
    $sxml = simplexml_import_dom($this->DOMNode());
    if ($sxml === false) {
        throw new XmlUtilException('Failed to convert DOM node to SimpleXML');
    }
    return [$sxml->getName() => $this->_toFullArray($sxml)];
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


protected function _toArray(SimpleXMLElement $node): array|string
protected function _toFullArray(SimpleXMLElement $node): array|string
{
$hasAttributes = (count($node->attributes()) > 0);
$hasChildren = (count($node->children()) > 0);
Expand All @@ -289,7 +288,7 @@ protected function _toArray(SimpleXMLElement $node): array|string
if ($hasChildren) {
foreach ($node->children() as $child) {
$childName = $child->getName();
$childData = $this->_toArray($child);
$childData = $this->_toFullArray($child);

if (isset($output[$childName])) {
if (!is_array($output[$childName]) || !isset($output[$childName][0])) {
Expand All @@ -309,6 +308,39 @@ protected function _toArray(SimpleXMLElement $node): array|string
return $output;
}

public function toArray(Closure $func = null): array
{
return $this->_toArray($this->DOMNode(), $func);
}

protected function _toArray(SimpleXMLElement|DOMNode|array $arr, Closure|null $func = null): array
{
if ($arr instanceof SimpleXMLElement) {
return $this->_toArray((array) $arr, $func);
}

if ($arr instanceof DOMNode) {
return $this->_toArray((array) simplexml_import_dom($arr), $func);
}
Comment on lines +316 to +324
Copy link

Choose a reason for hiding this comment

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

Inefficient Recursive Type Conversions category Performance

Tell me more
What is the issue?

Multiple type casting and conversions in recursive calls create unnecessary object instantiations and type conversions.

Why this matters

Each recursive iteration potentially creates new object instances and performs type conversions, leading to memory churn and slower execution, especially for deeply nested XML structures.

Suggested change ∙ Feature Preview

Normalize the input type once at the entry point and work with a consistent type throughout the recursion:

protected function _toArray($input, ?Closure $func = null): array
{
    $arr = $this->normalizeToArray($input);
    $result = [];
    foreach ($arr as $key => $value) {
        $result[$key] = is_array($value) ? $this->_toArray($value, $func) : 
            (!empty($func) ? $func($value) : $value);
    }
    return $result;
}

private function normalizeToArray($input)
{
    if ($input instanceof SimpleXMLElement) return (array)$input;
    if ($input instanceof DOMNode) return (array)simplexml_import_dom($input);
    return $input;
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


$newArr = array();
if (!empty($arr)) {
foreach ($arr as $key => $value) {
$newArr[$key] =
(
is_array($value)
|| ($value instanceof DOMNode)
|| ($value instanceof SimpleXMLElement)
? $this->_toArray($value, $func)
: (!empty($func) ? $func($value) : $value)
);
}
}

return $newArr;
}


/**
* @param string|null $prefix
* @param string $uri
Expand Down
25 changes: 21 additions & 4 deletions tests/XmlUtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,19 +310,36 @@ public function testRemoveNode(): void

}

public function testXml2Array(): void
public function testXml2Array1(): void
{
$file = new File(__DIR__ . '/buggy.xml');
$xml = new XmlDocument($file, preserveWhiteSpace: false);

$array = $xml->toArray();
$this->assertEquals([ "node" => [ "subnode" => "value"]], $array);
}

public function testXml2Array2(): void
{
$xml = new XmlDocument('<root><node param="pval">value</node></root>');

$array = $xml->toArray();
$this->assertEquals([ "node" => "value"], $array);
}

public function testXml2FullArray(): void
{
$file = new File(__DIR__ . '/buggy.xml');
$xml = new XmlDocument($file, preserveWhiteSpace: false);

$array = $xml->toFullArray();
$this->assertEquals(['root' => [ "node" => [ "subnode" => "value"]]], $array);
}

public function testXmlToArrayWithAttributeAndNoText(): void
{
$xml = new XmlDocument('<root><node param="pval"/></root>');
$array = $xml->toArray();
$array = $xml->toFullArray();

$expected = [
'root' => [
Expand All @@ -338,7 +355,7 @@ public function testXmlToArrayWithAttributeAndNoText(): void
public function testXmlToArrayWithAttributeAndText(): void
{
$xml = new XmlDocument('<root><node param="pval">value</node></root>');
$array = $xml->toArray();
$array = $xml->toFullArray();

$expected = [
'root' => [
Expand All @@ -356,7 +373,7 @@ public function testXmlToArrayWithMixedContent(): void
{
$xmlString = '<root><node param="pval">value<other>value</other><node2 a="2"></node2></node></root>';
$xml = new XmlDocument($xmlString);
$array = $xml->toArray();
$array = $xml->toFullArray();

$expected = [
'root' => [
Expand Down