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

DOMNamedNodeMap, Dom\NamedNodeMap, DOMNodeList, Dom\NodeList, Dom\HTMLCollection, and Dom\DtdNamedNodeMap raises Cannot access offset error #12235

Closed
Girgias opened this issue Dec 10, 2024 · 4 comments · May be fixed by phpstan/phpstan-src#3724
Labels
Milestone

Comments

@Girgias
Copy link

Girgias commented Dec 10, 2024

Bug report

This could also be considered a feature request.

All these classes can read and check the existence of an index via array notation.

One possible way I can see to fix this in the stubs is to have it implement ArrayAccess and have a return type of never for offsetSet() and offsetUnset(), but it does not have the other two offset methods either.

This would be fixed on the PHP side with https://wiki.php.net/rfc/container-offset-behaviour

Code snippet that reproduces the problem

https://phpstan.org/r/3b7685e4-3ad7-4eed-92a1-e85c88fa22cb

Expected output

No errors.

Did PHPStan help you today? Did it make you happy in any way?

No response

@ondrejmirtes
Copy link
Member

It'd be enough to add the missing classes to this list https://github.com/phpstan/phpstan-src/blob/36656357396bb2a5dadaafbd69e611be8e73f660/src/Type/ObjectType.php#L64.

If the offsets cannot be set in some of them, we'd have to write some extra code in ObjectType::setOffsetValueType() and return ErrorType for those cases.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Dec 11, 2024
@Girgias
Copy link
Author

Girgias commented Dec 11, 2024

Wouldn't it be better to add a parameter to Type::isOffsetAccessLegal() which describes the sort of access.
For example:

enum OffsetAccessType {
    case Read; // Include existence checks
    case Write;
    case Unset;
    case Append;
    case Fetch; // This one would be for references, but possibly delay it?
}

And then overload ObjectType::isOffsetAccessLegal() ?

@phpstan-bot
Copy link
Contributor

@Girgias After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:

@@ @@
 ==========
 
  7: Cannot access offset 'role' on Dom\NamedNodeMap.
-14: Cannot access offset 'role' on DOMNamedNodeMap.
+14: Cannot access offset 'role' on DOMNamedNodeMap&iterable<DOMAttr>.
 
 PHP 7.2 – 8.3 (4 errors)
 ==========
@@ @@
  5: Parameter $element of method HelloWorld::foo1() has invalid type DOM\Element.
  7: Access to property $attributes on an unknown class DOM\Element.
  7: Cannot access offset 'role' on mixed.
-14: Cannot access offset 'role' on DOMNamedNodeMap.
+14: Cannot access offset 'role' on DOMNamedNodeMap&iterable<DOMAttr>.
Full report

PHP 8.4 (2 errors)

Line Error
7 Cannot access offset 'role' on Dom\NamedNodeMap.
14 Cannot access offset 'role' on DOMNamedNodeMap&iterable<DOMAttr>.

PHP 7.2 – 8.3 (4 errors)

Line Error
5 Parameter $element of method HelloWorld::foo1() has invalid type DOM\Element.
7 Access to property $attributes on an unknown class DOM\Element.
7 Cannot access offset 'role' on mixed.
14 Cannot access offset 'role' on DOMNamedNodeMap&iterable<DOMAttr>.

@ondrejmirtes
Copy link
Member

Fixed phpstan/phpstan-src#3725

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