Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PHPStan 2.0] replace instanceof usages #347

Closed

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Nov 13, 2024

This pull request is on top of #345

I attempts to replace all deprecated usages of instaceof in favor of dedicated methods: https://phpstan.org/developing-extensions/type-system#querying-a-specific-type

It seems to work on the majority of cases, but it still fails in propagating the type outside of a method with no return type; I'm not sure if this is a change on PHPStan's side; maybe it now requires to declare the return type explicitly in the code, so that errors arise if it doesn't match?

@Jean85
Copy link
Contributor Author

Jean85 commented Nov 13, 2024

@alexander-schranz I'm re-targeting this toward your PR so you can see only my diff. let me know if I can do anything else.

@sasezaki
Copy link
Contributor

It seems to work on the majority of cases, but it still fails in propagating the type outside of a method with no return type; I'm not sure if this is a change on PHPStan's side; maybe it now requires to declare the return type explicitly in the code, so that errors arise if it doesn't match?

It is caused by level 10 (PHPStan 2's new max level)

I got green with --level=9 on your Jean85:feature/phpstan-2-replace-instanceof branch

vendor/bin/phpstan analyse --configuration=phpstan-with-extension.neon --level=9
 14/14 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors

@sasezaki
Copy link
Contributor

when debugging with

diff --git a/test/StaticAnalysis/Test/ObjectProphecy/ProphesizeTest.php b/test/StaticAnalysis/Test/ObjectProphecy/ProphesizeTest.php
index edcc659..7d3d20f 100644
--- a/test/StaticAnalysis/Test/ObjectProphecy/ProphesizeTest.php
+++ b/test/StaticAnalysis/Test/ObjectProphecy/ProphesizeTest.php
@@ -29,10 +29,12 @@ final class ProphesizeTest extends Framework\TestCase
     protected function setUp(): void
     {
         $this->prophecy = $this->prophesize(Src\BaseModel::class);
+        \PHPStan\dumpType($this->prophecy);
     }

     public function testCreateProphecyInSetUp(): void
     {
+        \PHPStan\dumpType($this->prophecy);
         $this->prophecy
             ->getFoo()
             ->willReturn('bar');

got ( both master and Jean85:feature/phpstan-2-replace-instanceof)

vendor/bin/phpstan analyse --configuration=phpstan-with-extension.neon --level=9
 14/14 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -----------------------------------------------------------------------------------------------------
  Line   test/StaticAnalysis/Test/ObjectProphecy/ProphesizeTest.php
 ------ -----------------------------------------------------------------------------------------------------
  32     Dumped type: Prophecy\Prophecy\ObjectProphecy<JanGregor\Prophecy\Test\StaticAnalysis\Src\BaseModel>
  37     Dumped type: mixed
 ------ -----------------------------------------------------------------------------------------------------

@sasezaki sasezaki mentioned this pull request Nov 15, 2024
3 tasks
@Jean85 Jean85 mentioned this pull request Nov 18, 2024
2 tasks
@Jean85
Copy link
Contributor Author

Jean85 commented Nov 18, 2024

Thanks @sasezaki for the suggestion!

It now works, the test failure seems unrelated and probably it's failing on master too.

@sasezaki
Copy link
Contributor

@Jean85 please see & review #348. I submitted another pull request.

@alexander-schranz
Copy link
Collaborator

close in favor of #348 thx @sasezaki @Jean85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants