Make DOMNamedNodeMap generic#4211
Conversation
470e9bb to
260df38
Compare
stubs/dom.stub
Outdated
| * @property-read int $length | ||
| */ | ||
| class DOMNamedNodeMap | ||
| class DOMNamedNodeMap implements Traversable, IteratorAggregate, Countable |
There was a problem hiding this comment.
It shouldn't need to implements both Traversable and IteratorAggregate in the stub.
There was a problem hiding this comment.
@VincentLanglet I just copied it from DOMNodeList at
Line 75 in c0e6f0e
Do you want me to fix DOMNodeList as well?
There was a problem hiding this comment.
If it's done this way for DOMNodeList you can just ignore my remark I think
There was a problem hiding this comment.
Please fix both DOMNodeList and DOMNamedNodeMap here.
There are some classes where it'd make sense because of the logic here https://github.com/ondrejmirtes/BetterReflection/blob/61b25baaca1ea904447f46d975a4ae7c99b722e6/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php#L528-L574, but it's not true for any DOM* classes.
There was a problem hiding this comment.
I've now removed Traversable from all DOM classes.
ondrejmirtes
left a comment
There was a problem hiding this comment.
2d29cd8 to
ece37d6
Compare
|
I added |
Normally generic classes are required to specify the generic tempalte types in all typehints. So But making a non-generic class generic in a non-major PHPStan version is a BC break, that's where skipCheckGenericClasses comes in. It skips this requirement for the listed classes. |
|
Thank you. |
Resolves phpstan/phpstan#13365.