DirectoryListing: fix generic declarations

[JanTvrdik]

• marks T as covariant, this is required to make DirectoryListing<FileAttributes> subtype of DirectoryListing<StorageAttributes>
• add missing upper bound for T, because sortByPath relies on items being subtypes of StorageAttributes
• add missing generic parameter for implemented interface IteratorAggregate

---

playground

[SamMousa]

I don't think DirectoryListing should even be a generic. DirectoryListing is a collection with objects of type StorageAttributes. Each instance of this collection will very likely contain multiple implemenations, specifically FileAttributes and DirectoryAttributes. After all a directory can contain both directories and files.

So in conclusion I believe both the current annotation and this PR are wrong. What do you think?

[JanTvrdik]

That would also be an option, but there are some usecase for keeping it generic, i.e. you could do sth like

    /**
     * @param  DirectoryListing<StorageAttributes> $listing
     * @return DirectoryListing<FileAttributes>
     */
    private function getFilesInDirectory(DirectoryListing $listing): DirectoryListing
    {
        return $listing->filter(static function (StorageAttributes $attributes): bool {
            return $attributes->isFile();
        });
    }

[frankdejonge]

I'm not entirely sure why this is needed. In the case somebody maps to filenames this PR would cause their CI to be broken, I'd reckon?

[SamMousa]

That would also be an option, but there are some usecase for keeping it generic, i.e. you could do sth like

Fair enough, but according to my tests the covariant type doesn't matter. If you're doing a filter you'll need to override the return type using assertions regardless (since PHPStan won't be able to detect your type narrowing in the filter).

https://phpstan.org/r/e0a04a5e-b74d-4da8-8591-62e842497bc7

Theoretically, the DirectoryListing class may be extended by 3rd party code (since it is not final or marked as @internal). This means that a subclass could allow adding items to the listing. This is where using template-covariant becomes hairy. (Since code expecting my MutableDirectoryListing<StorageAttributes> cannot except a MutableDirectoryListing<FileAttributes>. See this blog post for details: https://phpstan.org/blog/whats-up-with-template-covariant

Here's an example that shows you why we MUST have template-covariant in some cases: https://phpstan.org/r/4caabe57-29aa-4bdc-8b70-b506f53e066c

Essentially imagine a function that prints a directory listing, and you want to pass it a file list (DirectoryListing<FileAttributes>).

In conclusion and upon further reflection I believe:

1. We should make it template-covariant.
2. • We should make the DirectoryListing class final (best option, breaks BC)
   • OR We should specifiy in comments that the DirectoryListing class must not allow adding items to the listing

[frankdejonge]

@SamMousa based on your example, I think we can achieve most (if not all) of that is proposed and BC by not scoping down to StorageAttributes like this: https://phpstan.org/r/63dc9580-0d59-476e-b3e0-f52ef6a71b6e