Add @implements to SPL iterator stubs#5672
Conversation
|
You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x. |
| * @template-covariant TKey | ||
| * @template-covariant TValue | ||
| * @template TIterator of \RecursiveIterator<TKey, TValue>|\IteratorAggregate<TKey, TValue> |
There was a problem hiding this comment.
Does it means that RecursiveIteratorIterator has now three template like
RecursiveIteratorIterator<int, int, IteratorAggregate<int, int>>
if so, it will break code where people used the single-template RecursiveIteratorIterator no ?
Also, this won't be compatible with the stub from psalm
https://github.com/vimeo/psalm/blob/ca151242c84d8962b921bf59c509b49362ce0eec/stubs/CoreGenericIterators.phpstub#L902-L903
There was a problem hiding this comment.
You are right, this introduces a breaking change.
I revised my code.
Probably CI errors are not related I guess...
|
Most of the errors are legit. However I fixed the type of |
|
please also check issue-bot results. especially I wonder why we get a |
|
@staabm |
| /** | ||
| * @implements RecursiveIterator<string, SplFileInfo|string> | ||
| */ | ||
| class RecursiveDirectoryIterator extends FilesystemIterator implements RecursiveIterator |
There was a problem hiding this comment.
looking at the above baseline.neon comment, I realized it might be a nice improvement to infer the RecursiveDirectoryIterator generics based on __construct $flags parameter.
if we could do this, we would not have the baseline entry (this might also be stuff for a future PR)
might be doable for more iterators which work with FilesystemIterator::* constants
There was a problem hiding this comment.
Is there a way to do something like BitwiseFlagHelper::bitwiseOrContainsConstant() in stubs?
If we need a new TypeSpecifierExtension or something, I would like it to be done in another PR.
There was a problem hiding this comment.
I think we need a extension and I agree, it should be a followup
staabm
force-pushed
the
recursive-iterator-iterator
branch
from
bb4270f to
85f685d
Compare
|
@takaram at best you could rebase this onto 2.2.x as we likely will not have a 2.1.x release anymore |
staabm
force-pushed
the
recursive-iterator-iterator
branch
from
85f685d to
4f8b267
Compare
`DirectoryIterator::current()` itself always returns `DirectoryIterator` but `FilesystemIterator`, a child class of `DirectoryIterator`, gives `string|SplFileIterator` which is not a subtype of `DirectoryIterator`. The declared return type of `DirectoryIterator::current()` is actually `mixed`. So the stub should be `mixed` too in order not to violate LSP. https://www.php.net/directoryiterator.current
takaram
force-pushed
the
recursive-iterator-iterator
branch
from
4f8b267 to
93455b3
Compare
| $beginNew = "<?php declare(strict_types = 1);\n\n//"; | ||
| $emptyDirectoriesToCheck = []; | ||
| foreach ($files as $file) { | ||
| assert($file instanceof SplFileInfo); |
There was a problem hiding this comment.
Out of computer today
@staabm do we have assert in the codebase or should it be a shouldNotHappen exception ?
There was a problem hiding this comment.
|
the added types might be considered a breaking change. maybe these should be loaded in bleeding edge only. I am not sure. |
4 participants
This PR adds
@implementannotations to the stub of three classes:RecursiveIteratorIteratorDirectoryIteratorRecursiveDirectoryIteratorCloses phpstan/phpstan#8435