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

check __callStatic purity instead of the pseudoMethod purity #6953

Merged
merged 2 commits into from Nov 21, 2021

Conversation

orklah
Copy link
Collaborator

@orklah orklah commented Nov 21, 2021

In #3180, users reported that pseudo methods with @method static plus a return type were pure by default.

It was not accurate, it's just that impurity was not checked. In #6724, I added code to check purity through __callStatic by checking the pure attribute of the pseudo method that was called.

It was successful in flagging impure calls in pure context but I did not see that the pure calls in pure context were now broken because I used the pure flag of the pseudo method (that is never flagged) instead of the pure flag of __callStatic itself.

This is now fixed and both way works, so this will fix #6941

@weirdan
Copy link
Collaborator

weirdan commented Nov 21, 2021

I wonder whether it would work with usePhpDocMethodsWithoutMagicCall="true".

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Nov 21, 2021
@orklah
Copy link
Collaborator Author

orklah commented Nov 21, 2021

I wonder whether it would work with usePhpDocMethodsWithoutMagicCall="true".

Both tests I added use __callStatic so the property does nothing here (I tested that locally)

However, if you have this config and you try to call a static method declared in docblock, you have no way to declare the call as pure but Psalm won't complain about it.

It could be an enhancement

@orklah orklah merged commit 79fa7f5 into vimeo:master Nov 21, 2021
@Ocramius
Copy link
Contributor

Thanks @orklah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static methods declared via @method static ReturnType methodName() do not inherit __callStatic() purity
3 participants