ext/spl: Narrow tentative return types for DirectoryIterator methods - #22642
Conversation
|
|
||
| CHECK_DIRECTORY_ITERATOR_IS_INITIALIZED(intern); | ||
| RETURN_LONG(intern->u.dir.index); | ||
| RETURN_STRINGL(intern->u.dir.entry.d_name, strlen(intern->u.dir.entry.d_name)); |
There was a problem hiding this comment.
Why did you change the return data here? It's BC break.
There was a problem hiding this comment.
I changed it to match the TODO in the stub, but you're right—it changes the runtime behavior and is a BC break.
There was a problem hiding this comment.
just few things.
- It breaks the "agreement" between
spl_filesystem_dir_current_keythat uses the integer index and this ::key() method. Maybe the stub should be fixed as to return int ? - If you want the current entry name, it seems
::getBasename()(::getFilename()eventually ?) is already doing it.
There was a problem hiding this comment.
class FilesystemIterator extends DirectoryIterator{
public function key(): string {}
}
class DirectoryIterator extends SplFileInfo implements SeekableIterator{
public function key(): int {}
}How to solve it?
There was a problem hiding this comment.
class FilesystemIterator extends DirectoryIterator{ public function key(): string {} } class DirectoryIterator extends SplFileInfo implements SeekableIterator{ public function key(): int {} }How to solve it?
then int|string might work (current() would be string|SplFileInfo) ?
ext/spl: Narrow tentative return types for DirectoryIterator methods Add tentative return type to Directory::current() Add tentative return type to Directory::current() Add tentative return type to Directory::current()
DanielEScherzer
left a comment
DanielEScherzer
left a comment
There was a problem hiding this comment.
okay for ext/reflection, didn't check the other parts
|
@arshidkv12 Can you please write a short upgrading note? I'll merge your changes afterwards :) |
|
thank you, very nice attempt to cleanup the todos! :) |
You are welcome |
5 participants
Update the
DirectoryIteratorstubs to narrow the tentative return typesof
key()andcurrent(), and regenerate the generated arginfo.