Skip to content

Improve DomNode methods return type #1761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 13, 2025
Merged
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
22 changes: 13 additions & 9 deletions dom/dom_c.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,15 @@ class DOMNode
/**
* Adds a new child before a reference node
* @link https://php.net/manual/en/domnode.insertbefore.php
* @param DOMNode $node <p>
* @template TNode of DOMNode
* @param TNode $node <p>
* The new node.
* </p>
* @param null|DOMNode $child [optional] <p>
* The reference node. If not supplied, newnode is
* appended to the children.
* </p>
* @return DOMNode The inserted node.
* @return TNode|false The inserted node.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the phpdoc and https://www.php.net/manual/en/domnode.insertbefore.php
it returns the node inserted, which is the one coming from the param.

AND false was missing

*/
public function insertBefore(
DOMNode $node,
Expand All @@ -179,35 +180,38 @@ public function insertBefore(
/**
* Replaces a child
* @link https://php.net/manual/en/domnode.replacechild.php
* @template TNode of DOMNode
* @param DOMNode $node <p>
* The new node. It must be a member of the target document, i.e.
* created by one of the DOMDocument->createXXX() methods or imported in
* the document by .
* </p>
* @param DOMNode $child <p>
* @param TNode $child <p>
* The old node.
* </p>
* @return DOMNode|false The old node or false if an error occur.
* @return TNode|false The old node or false if an error occur.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the phpdoc and https://www.php.net/manual/en/domnode.replacechild.php
the old node is returned which is the second param if I understand.

*/
public function replaceChild(DOMNode $node, DOMNode $child) {}

/**
* Removes child from list of children
* @link https://php.net/manual/en/domnode.removechild.php
* @param DOMNode $child <p>
* @template TNode of DOMNode
* @param TNode $child <p>
* The removed child.
* </p>
* @return DOMNode If the child could be removed the functions returns the old child.
* @return TNode|false If the child could be removed the functions returns the old child.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This returns the child removed (which is the one in param)
AND false was missing

https://www.php.net/manual/en/domnode.removechild.php

*/
public function removeChild(DOMNode $child) {}

/**
* Adds new child at the end of the children
* @link https://php.net/manual/en/domnode.appendchild.php
* @param DOMNode $node <p>
* @template TNode of DOMNode
* @param TNode $node <p>
* The appended child.
* </p>
* @return DOMNode The node added.
* @return TNode|false The node added.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This returns the node added (which is coming form the param)
AND false was missing

https://www.php.net/manual/en/domnode.appendchild.php

*/
public function appendChild(DOMNode $node) {}

Expand All @@ -226,7 +230,7 @@ public function hasChildNodes(): bool {}
* Indicates whether to copy all descendant nodes. This parameter is
* defaulted to false.
* </p>
* @return static The cloned node.
* @return static|false The cloned node.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

*/
public function cloneNode(
#[PhpStormStubsElementAvailable(from: '5.3', to: '5.6')] $deep,
Expand Down