Skip to content
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

regression: ownerDocument can be null #10877

Open
theseer opened this issue Apr 3, 2024 · 4 comments
Open

regression: ownerDocument can be null #10877

theseer opened this issue Apr 3, 2024 · 4 comments

Comments

@theseer
Copy link

theseer commented Apr 3, 2024

Back in 2020 to resolve issue #3081 a change was made to incorporate the fact that DOMNode::ownerDocument can be null. This was effectively reverted with PR #10619.

This reverting change is technically wrong and not following the actual implementation in PHP.

The example linked in the PR (https://3v4l.org/tWoCc) about PHP enforcing the non-nullable type is misleading/incorrect, as a modified, even simpler, example demonstrates: https://3v4l.org/fGc7t

In other words: The ownerDocument-Property can very much (still) be null. Whether or not this is a desirable behavior is a different story.

Copy link

Hey @theseer, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

@theseer
Copy link
Author

theseer commented Apr 3, 2024

Copy link

I found these snippets:

https://psalm.dev/r/cfd852bd7a
<?php

$node = new DOMElement('foo');
assert($node->ownerDocument instanceof DOMDocument);
Psalm output (using commit ef3b018):

ERROR: RedundantCondition - 4:1 - Type DOMDocument for $node->ownerDocument is always DOMDocument

@sebastianbergmann
Copy link
Contributor

The official stubs show that @theseer is right: https://github.com/php/php-src/blob/PHP-8.3.5/ext/dom/php_dom.stub.php#L335

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

No branches or pull requests

2 participants