Treat #[Pure(true)] in PhpStorm stubs as hasSideEffects => true#3880
Conversation
7444b55 to
7753667
Compare
7753667 to
37d1861
Compare
|
This pull request has been marked as ready for review. |
There was a problem hiding this comment.
before this PR phpstan errored
https://phpstan.org/r/57c356b1-b4f6-4a36-94f8-01a21b8996d4
Call to function checkdate() on a separate line has no effect.
after this change it no longer does
➜ phpstan-src git:(pr/3880) ✗ cat test.php
<?php declare(strict_types = 1);
class HelloWorld
{
public function sayHello($m): void
{
checkdate($m, $m, $m);
}
}
➜ phpstan-src git:(pr/3880) ✗ php bin/phpstan analyze test.php --debug
Note: Using configuration file /Users/staabm/workspace/phpstan-src/phpstan.neon.dist.
/Users/staabm/workspace/phpstan-src/test.php
------ ---------------------------------------------
Line test.php
------ ---------------------------------------------
3 Class HelloWorld must be abstract or final.
🪪 phpstan.finalClass
------ ---------------------------------------------
[ERROR] Found 1 error
➜ phpstan-src git:(2.1.x) ✗ php bin/phpstan analyze test.php --debug
Note: Using configuration file /Users/staabm/workspace/phpstan-src/phpstan.neon.dist.
/Users/staabm/workspace/phpstan-src/test.php
------ ----------------------------------------------------------------
Line test.php
------ ----------------------------------------------------------------
3 Class HelloWorld must be abstract or final.
🪪 phpstan.finalClass
7 Call to function checkdate() on a separate line has no effect.
🪪 function.resultUnused
------ ----------------------------------------------------------------
[ERROR] Found 2 errors
➜ phpstan-src git:(2.1.x) ✗
|
@staabm Thanks, I've sent a PR upstream since the |
| 'curl_errno' => ['hasSideEffects' => true], | ||
| 'curl_error' => ['hasSideEffects' => true], |
There was a problem hiding this comment.
This also looks wrong. I guess you should review all lines which changed from false to true
There was a problem hiding this comment.
Although they appear to be pure, they actually rely on some external state associated with the resource, so it makes sense that PhpStorm marks them as #[Pure(true)].
While it may be convenient to make curl_errno() pure for convenience in common use cases, it is inconsistent with impure functions like ob_get_level()(#12577).
To solve this problem, must_use/#[NoDiscard] needs to be supported separately from @pure/@impure.
|
As I understand, both In case |
|
Thank you! |
|
nice, this fixed a few more bugs, see https://github.com/phpstan/phpstan-src/actions/runs/13851103733 regression tests would be awesome |
|
I don't think we need them, they'd just verify "yeah, this function is in this array", not any real logic 😊 |
|
But yeah, three issues can be closed thanks to this :) |
| 'preg_grep' => ['hasSideEffects' => false], | ||
| 'preg_last_error' => ['hasSideEffects' => false], | ||
| 'preg_last_error_msg' => ['hasSideEffects' => false], | ||
| 'preg_last_error' => ['hasSideEffects' => true], |
There was a problem hiding this comment.
what side effects does it have?
There was a problem hiding this comment.
Thanks to this line we solved this issue phpstan/phpstan#10342
There was a problem hiding this comment.
Sure, I mean we should differentiate between pure and side effects. In my understanding "pure" is about return value, "side effects" are about effects on other functions. So I would expect this function to have no side effects, but be pure instead.
related with #118
PhpStorm marks functions that have side effects but whose return value is important as
#[Pure(true)], which in current PHPStan functionality means[hasSideEffects => true].refs #3867 (comment)
refs phpstan/phpstan#12738