Convert file_info resources to objects

[cmb69]

Besides our general desire to get rid of the legacy resource types,
this is particularly appealing for fileinfo, because there are already
respective objects.

---

Should probably inline php_fileinfo in finfo_object right away. Also, I wonder about serialization and cloning of finfo objects – is that properly supported?

[GrahamCampbell]

// cc @frankdejonge Not sure if this affects you?

[cmb69]

If there are BC concerns, please say so! :)

[frankdejonge]

@GrahamCampbell If I see this correctly, this is only the function-style interface, the new finfo OOP-interface is unaffected, right?

[cmb69]

Indeed, the OOP interface is completely unaffected. The procedural interface now handles objects, so both could be mixed (which might provide a smoother upgrade path).

[frankdejonge]

Off-topic: if the fopen will become something other than resource in the future too, that would impact Flysystem. If there is something we can instanceof against, allowing us to be backwards compatible, that would be good to have in place.

[Girgias]

Off-topic: if the fopen will become something other than resource in the future too, that would impact Flysystem. If there is something we can instanceof against, allowing us to be backwards compatible, that would be good to have in place.

The recommended way is to not check for an instanceof an object or use a is_resource() check but check against !== false. It is pretty likely that stream will change to objects as @sgolemon has been looking at this again.

[frankdejonge]

The recommended way is to not check for an instanceof an object or use a is_resource() check but check against !== false. It is pretty likely that stream will change to objects as @sgolemon has been looking at this again.

While that would work for the code that is opening a file-handle, it is not suitable for accepting one. For Flysystem, for example, the method accepts a resource opened elsewhere, so we need to have a way to check if it's the right input to prevent weird errors.

[Girgias]

The recommended way is to not check for an instanceof an object or use a is_resource() check but check against !== false. It is pretty likely that stream will change to objects as @sgolemon has been looking at this again.

While that would work for the code that is opening a file-handle, it is not suitable for accepting one. For Flysystem, for example, the method accepts a resource opened elsewhere, so we need to have a way to check if it's the right input to prevent weird errors.

As the object does not yet exists (and won't before the conversion is done) the only way would to do a double condition, mind linking to the relevant code?

[frankdejonge]

@Girgias it's this kind of an assertion: https://github.com/thephpleague/flysystem/blob/2.x/src/Filesystem.php#L141

[kocsismate]

@frankdejonge

While that would work for the code that is opening a file-handle, it is not suitable for accepting one. For Flysystem, for example, the method accepts a resource opened elsewhere, so we need to have a way to check if it's the right input to prevent weird errors.

You can add && !instanceof(finfo::class), can't you?

[frankdejonge]

@kocsismate that is correct, however, that was deemed not the best practice by @Girgias. Hence the further discussion. Perhaps we should take this discussion elsewhere since it's derailing the conversation (or preventing it even) about the current PR.

[cmb69]

I can't imagine that we switch stream resources to objects before PHP 9, so probably no need to discuss right now. :)

[cmb69]

If there are no objections, I'll merge this in a week.

[cmb69]

Nope, to late for this change for PHP 8.0.

[GrahamCampbell]

Oh. Gonna be a shame to delay a lot of these changes till 8.1 when not strictly necessary. It will make things harder for library authors to make their code work on both PHP 8.0 and 8.1.

[cmb69]

Well, we're more than 2 weeks after feature freeze, and so such changes would make the upgrade to PHP 8.0 harder for users who already have started that process.

[GrahamCampbell]

I suppose this has already been discussed and decided upon. ;)

[nikic]

Yeah, the release management decision here was to not do further resource -> object conversions past feature freeze. We don't stand a realistic chance of converting all resources to objects in PHP 8.0, some will necessarily happen later, so there's no strong motivation to allow them past feature freeze (unlike the warning -> Error conversions).

[nikic]

Also, I wonder about serialization and cloning of finfo objects – is that properly supported?

Cloning: Very, very no. Denied in PHP-7.3 via 7817fc0.
Serialization: No. Denied in PHP-8.0 via 6d3695a.

[cmb69]

Any objections to merging this?