Skip to content

Revert #1741 - \DOMElement::$attributes is never null #1745

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented May 27, 2025

See phpstan/phpstan#13076

This reverts #1741
From the PHP docs: https://www.php.net/manual/en/class.domnode.php#domnode.props.attributes

A DOMNamedNodeMap containing the attributes of this node (if it is a DOMElement) or null otherwise.

So, on \DOMElement it's correct to override and mark it as not nullable.

@VincentLanglet
Copy link
Contributor

I agree with this PR @isfedorov, sorry my mistake

@isfedorov
Copy link
Contributor

I'm sorry but I'm not sure I've got it.

A DOMNamedNodeMap containing the attributes of this node (if it is a DOMElement) or null otherwise.

So as I understand this, this property can contain null if the object isn't a DOMElement. But even if the object is a DOMElement reflection still shows that the type of property is nullable.
https://3v4l.org/HPtTl
Could you please clarify?

@VincentLanglet
Copy link
Contributor

I'm sorry but I'm not sure I've got it.

A DOMNamedNodeMap containing the attributes of this node (if it is a DOMElement) or null otherwise.

So as I understand this, this property can contain null if the object isn't a DOMElement. But even if the object is a DOMElement reflection still shows that the type of property is nullable. 3v4l.org/HPtTl Could you please clarify?

The logic behind is kinda

get_type($dom->attributes) = $dom instanceof DOMELement ? DOMNamedNodeMap : null;

Indeed seems like the reflection is telling the property might be nullable, but it's untrue for a DOMELement.

  • The property is initialized
  • The property is read only so cannot be re-set to null

see https://onlinephp.io/c/7b879

@isfedorov
Copy link
Contributor

The property is read only so cannot be re-set to null

Readonly isn't related to nullability. If you assign a DomNamedNodeMap instead of null the script will also fail, will not it? And the fact that if an object is of type DomElement then property contains objects of DOMNamedNodeMap but not null doesn't make the property not nullable.

@VincentLanglet
Copy link
Contributor

The property is read only so cannot be re-set to null

Readonly isn't related to nullability. If you assign a DomNamedNodeMap instead of null the script will also fail, will not it? And the fact that if an object is of type DomElement then property contains objects of DOMNamedNodeMap but not null doesn't make the property not nullable.

Sure

But

  • When the object is DomElement, the property is initialized (so not null)
  • You cannot set null to this property (because it's readonly)

This means that $domElement->attributes is never null, no ? And that's the meaning of the sentence

A [DOMNamedNodeMap](https://www.php.net/manual/en/class.domnamednodemap.php) containing the attributes of this node (if it is a [DOMElement](https://www.php.net/manual/en/class.domelement.php)) or [null](https://www.php.net/manual/en/reserved.constants.php#constant.null) otherwise.
  • When it's a DOMElement it's a DOMNamedNodeMap
  • When it's a DomNode which is not a DOMElement, it's nul
  • So when it's "just" a DomNode, it's DOMNamedNodeMap|null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants