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

Purity of @methods in phpdocs #3180

Closed
vudaltsov opened this issue Apr 18, 2020 · 4 comments · Fixed by #6724
Closed

Purity of @methods in phpdocs #3180

vudaltsov opened this issue Apr 18, 2020 · 4 comments · Fixed by #6724
Labels

Comments

@vudaltsov
Copy link
Contributor

Psalm currently considers @method static to be pure by default. Is it on purpose? Seems to be an error.
Also I think that if __callStatic is @pure, then all @method static should be considered pure.
Same for dynamic methods.

https://psalm.dev/r/6da6503a47

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/6da6503a47
<?php

/**
 * @method static void test()
 */
final class Impure
{
    public static function __callStatic(string $name, array $arguments)
    {
    }
}

/**
 * @psalm-pure
 */
function testImpure(): void
{
	Impure::__callStatic('', []); // this is correct
	Impure::test(); // this should also give ImpureMethodCall
}

/**
 * @method static void test()
 */
final class Pure
{
    /**
     * @psalm-pure
     */
    public static function __callStatic(string $name, array $arguments)
    {
    }
}

/**
 * @psalm-pure
 */
function testPure(): void
{
	Pure::__callStatic('', []); // this is correct
	Pure::test(); // this is also correct
}
Psalm output (using commit ddbc1d2):

ERROR: ImpureMethodCall - 18:10 - Cannot call an impure method from a pure context

@muglug
Copy link
Collaborator

muglug commented Apr 18, 2020

Psalm currently considers @method static to be pure by default. Is it on purpose? Seems to be an error.

Not on purpose

Also I think that if __callStatic is @pure, then all @method static should be considered pure.
Same for dynamic methods.

I think that's probably sane? But it's also a weird area where there's no obvious correct solution.

@vudaltsov
Copy link
Contributor Author

I personally don't care much about @method, I usually have real methods in my code :)

But today I was fixing an issue in webmozart/assert and decided to check why for example Assert:: nullOrString never throws an ImpureMethodCall.

@zerkms
Copy link
Contributor

zerkms commented Apr 20, 2020

@vudaltsov there is a relevant webmozarts/assert#126 that I'm trying to get done (not as fast as me and anyone else wanted, but my work + family require my attention too, so my apologies for that :-) )

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

Successfully merging a pull request may close this issue.

4 participants