Add PHP 8.1 support

[javer]

Fixes #531

[alexpott]

Thanks @javer for working on this. I've been testing this PR using Drupal's tests and it works great. Deprecation messages that were incorrectly triggered when mocking a class that implements \Iterator are no longer triggered. Nice one. 👍

[alexpott]

There's one issue that's come up from testing on Drupal. If you have a interface that extends \Traversable then on PHP 8.1 creating a double will trigger deprecation notices. If you add ->willImplement(\IteratorAggregator::class) then the deprecation notices go away. I think this is because \Traversable needs the object to implement either \IteratorAggregator or \Iterator - so it's a bit of an odd one.

[javer]

@ciaranmcnulty Feature freeze for PHP 8.1 was done on Jul 20 2021, so no changes in the language specs are expected since that moment.

Many changes in the tests related to the fact that they are checking implementation of the internal PHP classes which have been changed significantly in PHP 8.1, see:

• https://github.com/php/php-src/blob/master/ext/spl/spl_array.stub.php
• https://github.com/php/php-src/blob/master/ext/reflection/php_reflection.stub.php

So since PHP 8.1 many such tests became incorrect, because behavior of the tested methods was changed inside PHP.

I suggest not to wait for RC, because this library is special in the sense that 99% PHP packages indirectly depend on it, because all of them use phpunit/phpunit for unit testing, and all phpunit/phpunit versions except unreleased 10.x depends on this library.

In other words, missing support for PHP 8.1 in this library blocks almost all packages even to test whether they support PHP 8.1 or not, because all of them at least need to put this hack to CI flow https://github.com/php-amqplib/php-amqplib/pull/929/files#diff-ea12f60188cdd90bc99e5d0af2eb91647bbe4a9199176aa1ec5240f65efed510R79 and then they see many failed tests just because of incompatibility of this library with PHP 8.1, but not because of their failures.

[javer]

@alexpott Please check whether doubling \Traversable doesn't trigger deprecation notices with the latest changes regardless of the willImplement().

[alexpott]

@javer nope the recent changes don't appear to have changed the outcome of doing

    $t = $this->prophesize(\Traversable::class);
    $t->reveal();

This still result in the follow deprecation notices:

  1x: Return type of Double\Traversable\P1::current() should either be compatible with Iterator::current(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::next() should either be compatible with Iterator::next(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::key() should either be compatible with Iterator::key(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::valid() should either be compatible with Iterator::valid(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::rewind() should either be compatible with Iterator::rewind(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

in a test using PHPUnit with phpspec/prophecy-phpunit.

[alexpott]

@javer nope the recent changes don't appear to have changed the outcome of doing

    $t = $this->prophesize(\Traversable::class);
    $t->reveal();

This still result in the follow deprecation notices:

  1x: Return type of Double\Traversable\P1::current() should either be compatible with Iterator::current(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::next() should either be compatible with Iterator::next(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::key() should either be compatible with Iterator::key(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::valid() should either be compatible with Iterator::valid(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::rewind() should either be compatible with Iterator::rewind(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

in a test using PHPUnit with phpspec/prophecy-phpunit.

~@javer nope the recent changes don't appear to have changed the outcome of doing~

    $t = $this->prophesize(\Traversable::class);
    $t->reveal();

~This still result in the follow deprecation notices:~

  1x: Return type of Double\Traversable\P1::current() should either be compatible with Iterator::current(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::next() should either be compatible with Iterator::next(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::key() should either be compatible with Iterator::key(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::valid() should either be compatible with Iterator::valid(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

  1x: Return type of Double\Traversable\P1::rewind() should either be compatible with Iterator::rewind(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

~in a test using PHPUnit with phpspec/prophecy-phpunit.~

@javer sorry I was wrong - cause of general awkwardness testing a PR with composer I missed the changes to src/Prophecy/Doubler/ClassPatch/TraversablePatch.php - your recent changes work great. Thanks!

[javer]

Added spec for testing new (and old) behavior of TraversablePatch, the build is green: https://github.com/javer/prophecy/runs/3218018475?check_suite_focus=true

[ciaranmcnulty]

I really don't want to tag a release of Prophecy that says it supports 8.1 before we've tested against a release candidate. If it's screwing up PHPUnit then I could maybe be persuaded (cc @sebastianbergmann)

Is there a reason libraries wanting to test can't use composer's ignore-platform-req=php flag?

[sebastianbergmann]

PHPUnit 8.5 and PHPUnit 9.5 still depend on Prophecy. This leads to

$ composer require --dev phpunit/phpunit ^8.5
./composer.json has been created
Running composer update phpunit/phpunit
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit[8.5.12, ..., 8.5.19] require phpspec/prophecy ^1.10.3 -> satisfiable by phpspec/prophecy[v1.10.3, ..., 1.13.0].
    - phpunit/phpunit[8.5.0, ..., 8.5.11] require php ^7.2 -> your php version (8.1.0-dev) does not satisfy that requirement.
    - phpspec/prophecy[1.11.0, ..., 1.11.1] require php ^7.2 -> your php version (8.1.0-dev) does not satisfy that requirement.
    - phpspec/prophecy v1.10.3 requires php ^5.3|^7.0 -> your php version (8.1.0-dev) does not satisfy that requirement.
    - phpspec/prophecy[1.12.0, ..., 1.13.0] require php ^7.2 || ~8.0, <8.1 -> your php version (8.1.0-dev) does not satisfy that requirement.
    - Root composer.json requires phpunit/phpunit ^8.5 -> satisfiable by phpunit/phpunit[8.5.0, ..., 8.5.19].

Installation failed, deleting ./composer.json.

and

$ composer require --dev phpunit/phpunit ^9.5
./composer.json has been created
Running composer update phpunit/phpunit
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit[9.5.0, ..., 9.5.8] require phpspec/prophecy ^1.12.1 -> satisfiable by phpspec/prophecy[1.12.1, 1.12.2, 1.13.0].
    - phpspec/prophecy[1.12.1, ..., 1.13.0] require php ^7.2 || ~8.0, <8.1 -> your php version (8.1.0-dev) does not satisfy that requirement.
    - Root composer.json requires phpunit/phpunit ^9.5 -> satisfiable by phpunit/phpunit[9.5.0, ..., 9.5.8].

Installation failed, deleting ./composer.json.

respectively.

PHPUnit 10, which is still in development, no longer depends on Prophecy.

[ciaranmcnulty]

@sebastianbergmann I'm happy to do whatever makes your life easier, but is there a reason --ignore-platform-reqs=php doesn't solve this issue?

[sebastianbergmann]

I think having to use --ignore-platform-reqs=php because of Prophecy should be fine until PHP 8.1 RC, PHP 8.1 GA at the latest. After that, I do not think it would be acceptable anymore.

In other words: are you planning to support PHP 8.1 in Prophecy and if so, when?

[alecpl]

With all respect. The number of changes in this PR shows clearly to me that ignore-platform-req=php is not a complete fix. It might depend on what features phpunit and peoples tests are using.

[sebastianbergmann]

It might depend on what features phpunit and peoples tests are using.

PHPUnit depends on Prophecy to provide the TestCase::prophesize() method:

protected function prophesize($classOrInterface = null): ObjectProphecy
{
    if (is_string($classOrInterface)) {
        $this->recordDoubledType($classOrInterface);
    }

    return $this->getProphet()->prophesize($classOrInterface);
}

And in the TestCase::verifyMockObjects() method, PHPUnit uses Prophecy like so:

if ($this->prophet !== null) {
    try {
        $this->prophet->checkPredictions();
    } finally {
        foreach ($this->prophet->getProphecies() as $objectProphecy) {
            foreach ($objectProphecy->getMethodProphecies() as $methodProphecies) {
                foreach ($methodProphecies as $methodProphecy) {
                    assert($methodProphecy instanceof MethodProphecy);

                    $this->numAssertions += count($methodProphecy->getCheckedPredictions());
                }
            }
        }
    }
}

Many years ago, I thought that it would be a good idea to 1) have PHPUnit depend on Prophecy to make the latter always available when PHPUnit is availabe and 2) provide a convenient integration of Prophecy with PHPUnit. This was deprecated in PHPUnit 9.1 and has been removed for PHPUnit 10.

[ADmad]

I really don't want to tag a release of Prophecy that says it supports 8.1 before we've tested against a release candidate.

You can merge this into a dev branch instead of master then. That way the whole downstream deps chain isn't blocked from upgrading/trying out their code with PHP 8.1.

[ciaranmcnulty]

I've added some issues for things we'll need to check on PHP 8.1

https://github.com/phpspec/prophecy/issues?q=is%3Aopen+is%3Aissue+label%3APHP8.1

Any extra issues people can think of, please raise them

[ciaranmcnulty]

@ADmad

You can merge this into a dev branch instead of master then. That way the whole downstream deps chain isn't blocked from upgrading/trying out their code with PHP 8.1.

If someone can depend on a dev branch instead, they can use the --ignore-platform-reqs=php flag too

[ciaranmcnulty]

@sebastianbergmann

In other words: are you planning to support PHP 8.1 in Prophecy and if so, when?

I'm happy to start testing against it and would aim to tag a release around the RC stage

[sebastianbergmann]

@sebastianbergmann In other words: are you planning to support PHP 8.1 in Prophecy and if so, when?

PHPUnit 8.5 and PHPUnit 9.5 are supported on PHP 8.1 and depend on Prophecy. I am not sure what you mean by "are you planning to support PHP 8.1 in Prophecy", though.

[ciaranmcnulty]

@sebastianbergmann Sorry that was meant to be quoting your question from upthread and then answering it

[ciaranmcnulty]

Thanks, I'm going to merge this so dev-master is 'compatible', but won't tag a release until the outstanding issues are closed off

[ADmad]

If someone can depend on a dev branch instead, they can use the --ignore-platform-reqs=php flag too

https://github.com/phpspec/prophecy/pull/533#issuecomment-891617715

[ciaranmcnulty]

Thanks for the work @javer

[ciaranmcnulty]

It's demoralising to have so many on here emphasising how important it is for Prophecy to support 8.1, but no work has been done on any of the 8.1 issues in the month since, nor has anyone reviewed or commented on #538

It'll all eventually get done, of course, but at the moment it's bottlenecked on my time and availability

[ciaranmcnulty]

FYI https://github.com/phpspec/prophecy/releases/tag/1.14.0