Yesterday, I opened an issue regarding a change in the pgsql extension (
https://bugs.php.net/bug.php?id=81464).
PHP 8.0 introduced the concept of "resource objects". Where previously we
would have resources, and use `get_resource_type()` when we needed to
differentiate various resources, resource objects give us immutable objects
instead that allow us to type hint. Personally, I think this is wonderful!
The rollout for 8.0 was incomplete, however, and only touched on something
like 4-6 different resource types. Still, a good start.
With PHP 8.1, we're seeing the addition of more of these, and it was
encountering one of those changes that prompted the bug I previously linked
to.
Here's the issue: while overall, I like the move to resource objects,
introducing them in a MINOR release is hugely problematic.
Previously, you would do constructs such as the following:
if (! is_resource($resource) || get_resource_type($resource) !==
$someSpecificType) {
// skip a test or raise an exception
}
Resource objects, however:
- Return `false` for `is_resource()` checks.
- Raise a warning for `get_resource_type()` checks, and/or report the
resource object class name — which differs from the previous resource names
in all cases.
This means conditionals like the above BREAK. As a concrete example, I did
PHP 8.1 updates for laminas-db last week, and assumed our postgres
integration tests were running when we finally had all tests passing
successfully. However, what was really happening was that our test suite
was testing with `is_resource()` and skipping tests if resources were not
present. We shipped with broken pgsql support as a result, and it wasn't
until test suites in other components started failing that we were able to
identify the issue.
Further, the "fix" so that the code would work on both 8.1 AND versions
prior to 8.1 meant complicating the conditional, adding a `! $resource
instanceof \PgSql\Connection` into the mix. The code gets unwieldy very
quickly, and having to do this to support a new minor version was
irritating.
When I opened the aforementioned bug report, it was immediately closed as
"not an issue" with the explanation that it was "documented in UPGRADING".
This is not an acceptable explanation.
- There was no RFC related to 8.1 indicating these changes were happening.
(In fact, there was no RFC for resource objects in the first place — which
is concerning considering the BC implications!)
- In semantic versioning, existing APIs MUST NOT change in a new minor
version, only in new major versions.
Reading the UPGRADING guide, there's a HUGE section of backwards
incompatible changes for 8.1 — THIRTY-FOUR of them. Nested in these are
notes of around a half-dozen extensions that once produced resources now
producing resource objects.
The pace of change in PHP is already breathtaking when you consider large
projects (both OSS and in userland); keeping up with new features is in and
of itself quite a challenge. Introducing BC breaks in minor versions makes
things harder for everyone, as now you have to figure out not only if
there's new features you want to adopt, but whether or not there are
changes that will actively break your existing code. I strongly feel that
anything in the backwards incompatible section of the UPGRADING guide
should be deferred to 9.0, when people actually expect things to change.
--
Matthew Weier O'Phinney
mweierophin...@gmail.com
https://mwop.net/
he/him
