-
-
Notifications
You must be signed in to change notification settings - Fork 863
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
Add Hack-like option to forget property assignments after function call #929
Comments
How do you do it? Do you recursively inspect the called methods bodies what properties do they touch? |
Oh I don't do anything sophisticated. Just as Hack does, I remove all the cached types for properties on all objects in scope after every function call, method call or Meaning this code also fails when that config is on (ditto in Hack): class X {
/** @var ?int **/
private $x;
public function getX(): int {
$this->x = 5;
$this->modifyX();
return $this->x;
}
private function modifyX(): void {
$this->x = 7;
}
} |
Flow for javascript does something similar as well and I find it quite annoying at times. But at least it forces you to write "pure" code and not mess with stuff by side effect which is not a bad thing. |
Yeah, I don’t find the option all that useful (enabling it for Psalm’s own codebase produces 100s of false positives), but we have it enabled for a transactions-related repo that needs to be bulletproof. |
Just a note that Psalm now does a bit more inference, so the above code doesn’t need a special config flag to work. At the scanning step (where Psalm gathers docblocks and other metadata about the code) it checks to see if a method contains Happens here: vimeo/psalm@85ae8f9#diff-b75faef2d9b16277900cfa9edff604d5R469-R484 and here: vimeo/psalm@85ae8f9#diff-11f0b4d80267531254d3fdd0fbc67248R1305-R1308 |
The assignment is now forgotten when calling |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This is a feature request for a feature I don't think many are interested in. Still, worth thinking about.
Hack forbids the above code (though to see it you have to change the property type to one hack understands). Psalm, by default, allows the above code, but a config flag makes it follow Hack's behaviour.
The text was updated successfully, but these errors were encountered: