Bump to PHP 7.4+ and upgrade to PHPStan v2 by Jean85 · Pull Request #639 · phpspec/prophecy

Jean85 · 2025-04-29T12:48:39Z

@MarcelloDuarte this is the PR that I promised. I'll self-review to add more context to each change.

Jean85 · 2025-04-29T12:49:29Z

phpstan-baseline.neon

+			message: '#^Parameter \#1 \$value of function strval expects bool\|float\|int\|resource\|string\|null, mixed given\.$#'
+			identifier: argument.type


Multiple lines here are changed just because PHPStan v2 does no longer double-escape, and adds the identifier of the error. I'll comment only on new ignored issues.

Jean85 · 2025-04-29T12:54:28Z

phpstan-baseline.neon

+		-
+			message: '#^Method Prophecy\\Doubler\\CachedDoubler\:\:createDoubleClass\(\) should return class\-string\<Prophecy\\Doubler\\DoubleInterface&T of object\> but returns class\-string\.$#'
+			identifier: return.type


This is a method extending its parent which is generic. It seems that the nested-double generic isn't working, we could report this upstream to PHPStan

Jean85 · 2025-04-29T12:55:38Z

phpstan-baseline.neon

+			message: '#^Call to function method_exists\(\) with ''ReflectionClass'' and ''isReadOnly'' will always evaluate to true\.$#'
+			identifier: function.alreadyNarrowedType
+			count: 1
+			path: src/Prophecy/Doubler/Generator/ClassMirror.php
+
+		-
+			message: '#^Call to function method_exists\(\) with ReflectionMethod and ''hasTentativeReturnT…'' will always evaluate to true\.$#'
+			identifier: function.alreadyNarrowedType
+			count: 1
+			path: src/Prophecy/Doubler/Generator/ClassMirror.php


These two are due to the fact that those reflection methods are present only from PHP 8.1. These workaround go away as soon as we bump the minimum PHP version.

Jean85 · 2025-04-29T12:57:06Z

phpstan-baseline.neon

+			message: '#^PHPDoc tag @var with type static\(Prophecy\\Doubler\\LazyDouble\<U of object\>\) is not subtype of native type \$this\(Prophecy\\Doubler\\LazyDouble\<T of object\>\)\.$#'
+			identifier: varTag.nativeType
+			count: 1
+			path: src/Prophecy/Doubler/LazyDouble.php
+
+		-
+			message: '#^Property Prophecy\\Doubler\\LazyDouble\:\:\$class \(ReflectionClass\<T of object\>\|null\) does not accept ReflectionClass\<U of object\>\.$#'
+			identifier: assign.propertyType
+			count: 1
+			path: src/Prophecy/Doubler/LazyDouble.php
+
+		-
+			message: '#^Property Prophecy\\Doubler\\LazyDouble\:\:\$class \(ReflectionClass\<T of object\>\|null\) is never assigned ReflectionClass\<T of object\> so it can be removed from the property type\.$#'
+			identifier: property.unusedType
+			count: 1
+			path: src/Prophecy/Doubler/LazyDouble.php


These 3 are all from the same issue, the setParentClass method is very hard to map against PHPStan because it changes the type of the generic of the instance itself.

Jean85 · 2025-04-29T12:57:25Z

phpstan-baseline.neon

+		-
+			message: '#^Call to function method_exists\(\) with ReflectionMethod and ''hasTentativeReturnT…'' will always evaluate to true\.$#'
+			identifier: function.alreadyNarrowedType
+			count: 1
+			path: src/Prophecy/Prophecy/MethodProphecy.php


Again, 8.1+ reflection feature...

Jean85 · 2025-04-29T12:59:15Z

src/Prophecy/Exception/Prediction/UnexpectedCallsCountException.php

     * @param MethodProphecy $methodProphecy
     * @param int            $count
-     * @param list<Call>     $calls
+     * @param array<Call>     $calls


Inputing any array, casting to list below

do we need this, or should we use a more precise type in our CallCenter (which is probably already using lists) ?

Jean85 · 2025-04-29T12:59:28Z

src/Prophecy/Exception/Prediction/UnexpectedCallsException.php

+     * @param array<Call>     $calls
     */
    public function __construct($message, MethodProphecy $methodProphecy, array $calls)
    {
        parent::__construct($message, $methodProphecy);

-        $this->calls = $calls;
+        $this->calls = array_values($calls);


Same here, casting to list (previous was extending class)

Jean85 · 2025-04-29T12:59:52Z

src/Prophecy/Prediction/CallbackPrediction.php

        $callback = $this->callback;

-        if ($callback instanceof Closure && method_exists('Closure', 'bind') && (new ReflectionFunction($callback))->getClosureThis() !== null) {
+        if ($callback instanceof Closure && (new ReflectionFunction($callback))->getClosureThis() !== null) {


bind is present since PHP 5.4, we can ditch this check.

Jean85 · 2025-04-29T12:59:57Z

src/Prophecy/Promise/CallbackPromise.php

        $callback = $this->callback;

-        if ($callback instanceof Closure && method_exists('Closure', 'bind') && (new ReflectionFunction($callback))->getClosureThis() !== null) {
+        if ($callback instanceof Closure && (new ReflectionFunction($callback))->getClosureThis() !== null) {


bind is present since PHP 5.4, we can ditch this check.

Jean85 · 2025-04-29T13:00:22Z

src/Prophecy/Util/ExportUtil.php


                $values = "\n".$values.$whitespace;
            }
+            \assert(\is_object($value));


This is just a change in how PHPStan detect types; better but not completely.

Jean85 · 2025-04-29T13:57:30Z

@MarcelloDuarte I've bumped to PHP 7.4 as discussed.

I've also fixed the PHPStan analysis, but I saw that the output depends on the PHP version it runs on. It seems acceptable for now.

I also added a changelog entry, so that a 1.22 should be released with this bump. Let me know if this is ok or if it needs more work.

stof · 2025-04-29T13:05:40Z

src/Prophecy/Doubler/Generator/ClassMirror.php

        }

-        return $types;
+        return array_values($types);


this is actually an issue in phpstan IMO. It should know that ReflectionUnionType::getTypes returns a list (so that doing array_map on it still returns a list)

stof · 2025-04-29T13:06:38Z

src/Prophecy/Util/ExportUtil.php

                return 'Array &'.$key;
            }

-            \assert(\is_array($value));


isn't this removal causing the issues reported just below ?

@Jean85 I see you already reverted the above. @stof are you happy for us to merge and release 1.22 to unblock phpspec?

how is this PR unblocking phpspec ? Removing support for PHP versions in a dependency cannot be a blocker (phpspec requires that prophecy supports at least all the PHP versions it needs, but does not care if it supports more).

It's some sort of circular dependency around stan. That wasn't my question though... Is this mergeable? Do we need to keep supporting PHP versions in EOL state to keep the next tag within minor release?

Do we need to keep supporting PHP versions in EOL state to keep the next tag within minor release?

As long as we update our php requirement properly in the composer.json, dropping support for some PHP versions is not a BC break (as composer will still respect that requirement when resolving semver ranges). That's still something that deserves a minor version IMO (it is not a bugfix), but it does not require a major version bump as far as Composer is concerned (Symfony itself has a policy to be stricter for the fullstack framework, but that's for other reasons)

Jean85 added 2 commits April 29, 2025 13:37

Require PHPStan v2

cce8f7a

Resolve PHPStan issues after upgrading to v2

b5c11c3

Jean85 commented Apr 29, 2025

View reviewed changes

Jean85 added 2 commits April 29, 2025 15:00

Solve one additional error

12d783a

Drop support to PHP below 7.4

8a9ea35

Jean85 force-pushed the phpstan-v2 branch from 344bea5 to 80490cc Compare April 29, 2025 13:56

Jean85 changed the title ~~PHPStan v2~~ Bump to PHP 7.4+ and upgrade to PHPStan v2 Apr 29, 2025

Solve more error, based on CI output

c25152c

Jean85 force-pushed the phpstan-v2 branch from 80490cc to c25152c Compare April 29, 2025 13:58

stof reviewed Apr 29, 2025

View reviewed changes

MarcelloDuarte merged commit 35f1adb into phpspec:master Apr 29, 2025
10 checks passed

Jean85 deleted the phpstan-v2 branch April 30, 2025 11:13

		message: '#^Parameter \#1 \$value of function strval expects bool\\|float\\|int\\|resource\\|string\\|null, mixed given\.$#'
		identifier: argument.type

Conversation

Jean85 commented Apr 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jean85 commented Apr 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcelloDuarte Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MarcelloDuarte Apr 29, 2025 •

edited

Loading