Add support for PHPStan 2 #348
Conversation
| Type\TypeCombinator::intersect( | ||
| new Type\ObjectType($className), | ||
| ...$calledOnType->getTypes() | ||
| //...$calledOnType->templ |
There was a problem hiding this comment.
commenting this looks wrong.
I would also suggest trying to remove that extension entirely (without removing the tests) as I think the phpdoc tags in ObjectProphecy should be enough to describe that already.
There was a problem hiding this comment.
Sorry, this is my fault, it's pending in #347 since I didn't know how to do this.
There was a problem hiding this comment.
I think the phpdoc tags in ObjectProphecy should be enough to describe that already.
I agree with your suggestion.
sadly but, current prophecy's phpdoc (from phpspec/prophecy#574) is only about this-out.
* @template U of object
* @phpstan-param class-string<U> $interface
* @phpstan-this-out static<T&U>
*/
public function willImplement($interface)not defined like @phpstan-return static<T&U>.
so we got bellow errors when remove this WillExtendOrImplementDynamicReturnTypeExtension removed.
------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Line test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php
------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
62 Parameter #1 $bar of method JanGregor\Prophecy\Test\StaticAnalysis\Src\BaseModel::bar() expects JanGregor\Prophecy\Test\StaticAnalysis\Src\Bar, JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo given.
🪪 argument.type
84 Method JanGregor\Prophecy\Test\StaticAnalysis\Test\ObjectProphecy\WillImplementTest::createProphecy() should return Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Bar> but returns
Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo>.
🪪 return.type
------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- refs. ( debugging with dumpType)
diff --git a/test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php b/test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php
index 05b61de..62693c7 100644
--- a/test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php
+++ b/test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php
@@ -50,7 +50,12 @@ final class WillImplementTest extends Framework\TestCase
public function testCreateProphecyInTestMethod(): void
{
+ $prophecyParamOut = $this->prophesize(Src\Foo::class);
+ $prophecyParamOut->willImplement(Src\Bar::class);
+ \PHPStan\dumpType($prophecyParamOut);
+
$prophecy = $this->prophesize(Src\Foo::class)->willImplement(Src\Bar::class);
+ \PHPStan\dumpType($prophecy); ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Line test/StaticAnalysis/Test/ObjectProphecy/WillImplementTest.php
------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
55 Dumped type: Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Bar&JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo>
58 Dumped type: Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo>
67 Parameter #1 $bar of method JanGregor\Prophecy\Test\StaticAnalysis\Src\BaseModel::bar() expects JanGregor\Prophecy\Test\StaticAnalysis\Src\Bar, JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo given.
🪪 argument.type
89 Method JanGregor\Prophecy\Test\StaticAnalysis\Test\ObjectProphecy\WillImplementTest::createProphecy() should return Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Bar> but returns
Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\Foo>.
🪪 return.type
------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
There was a problem hiding this comment.
This is actually a bug in phpstan: phpstan/phpstan#8439
There was a problem hiding this comment.
Nope, that can be solved with a conditional type: https://phpstan.org/r/4bd81d3d-c0cf-40bf-a8a8-9b951e317771
* @return ($class is class-string ? ObjectProphecy<T> : ObjectProphecy<object>)
There was a problem hiding this comment.
Woha, you already came to the same conclusion a year ago 😄
phpspec/prophecy@9ab4a3c#diff-81b4cc8adbc7615b7b55568528ba2dc2436049e7fd1fd1c1567d8d9ed0d234d5R77
There was a problem hiding this comment.
The conditional type does not solve the phpstan bug with willExtend. It only hides it in the case of prophesize. See my new comment on the bug.
There was a problem hiding this comment.
|
The only remaining blocker seems phpstan/phpstan#8439, but we can keep the |
…-phpunit requirements to ^2.3 for prophesize() precise types updates https://github.com/Jan0707/phpstan-prophecy/pull/348\#discussion_r1846385958
…ricObjectType will be reported as method not found)
| $className = $scope->getClassReflection()->getName(); | ||
| } | ||
|
|
||
| \assert(\get_class($calledOnType) === Type\Generic\GenericObjectType::class); |
There was a problem hiding this comment.
This is to avoid assert($calledOnType instanceof Type\Generic\GenericObjectType::class);
( see https://phpstan.org/blog/why-is-instanceof-type-wrong-and-getting-deprecated )
.. I'm not sure this assertion check is good, or not.
There was a problem hiding this comment.
checking get_class() === is actually a lot worse than instanceof, as it would even break for child classes. Such check is actually breaking the LSP rule of SOLID if the class being checked is not final (and is strictly equivalent to instanceof for final classes)
There was a problem hiding this comment.
Yes, that's absolutely correct. I think I've been relying too much on workarounds.
Looking at PHPStan's own baseline, I noticed that a lot of phpstanApi.instanceofType remains. I think I'll revert it back to instanceof.
https://github.com/phpstan/phpstan-src/blob/2.0.2/phpstan-baseline.neon
I think keeping the WillExtendOrImplementDynamicReturnTypeExtension is still needed. |
|
I would keep it aslong as phpstan/phpstan#8439 is not solved. If that is solved we can in future versions remove it and increase the phpstan min version. |
|
Ok, so we're ok with leaving the |
|
@Jean85 fine to continue things in another Pull request. |
on top of #345, #347
and do " Remove not longer required stubs" @see #309 .
Todos - needs HELP!
.docker(or remove?)This PR includes a lot of changes to pass CI.
requiments, dependency
.php_csto.php-cs-fixer.php)tests
Reflection\ClassReflectionis final class.prophesize()in these unit tests.