Introduce all-methods-are-(im)pure

[VincentLanglet]

Rework of #4409
and #4415

I added a non regression of phpstan/phpstan#10215 as a proof it works.

I don't like the naming of my methods, suggestion are opened.
Also I'm not fully sure getDefaultMethodPurity shouldn't be called somewhere else too...

[VincentLanglet]

I dunno which strategy you will prefer @ondrejmirtes between

• doing calls like isPure() ?? getDefaultMethodPurity() like 846d7bd#diff-fb5399c56567ad1b8be9256fe9008a03a43b7686e02156b9f1b3118e76d95e2fR895

• or using the fallback directly in the isPure method like https://github.com/phpstan/phpstan-src/pull/4422/files#diff-260042b10e8fde4d3e77fce0c1fc6fcf0851c7e442271dbefb5eded974d607e6R837-R843

The second one seems simpler to me because isPure was used multiple times like in NodeScopeResolver::getPhpDocs and it would have require more changes.

[staabm]

The phpstan downgrade process does not support null-safe calls. You need regular if-null checks for php7 compat

[ondrejmirtes]

I think that ResolvedPhpDocBlock should simply add isAllMethodsPure and isAllMethodsImpure.

I imagine the new tags should be handled similarly to isImmutable / isReadOnly.

Above class there can be @phpstan-immutable tag which marks all properties as readonly. And above properties you can have @phpstan-readonly.

Having @phpstan-immutable above class does not change what ResolvedPhpDocBlock says about isReadOnly.

Instead, PhpClassReflectionExtension handles this when creating properties by asking about both tags:

phpstan-src/src/Reflection/Php/PhpClassReflectionExtension.php

Line 291 in d78a5d8

$isReadOnlyByPhpDoc = $isReadOnlyByPhpDoc || $resolvedPhpDoc->isReadOnly();

So handling these new tags should be equivalent.

Please do not forget to test that PureMethodRule still correctly reports errors with all possible combinations. This means that these new tags also have to be read in NodeScopeResolver::getPhpDocs() so that they're available in PhpMethodFromParserNodeReflection (which is the object used when analysing method body itself).

[phpstan-bot]

This pull request has been marked as ready for review.

[VincentLanglet]

I think it's ready for a new review :)

[ondrejmirtes]

Also I'd like some more tests:

1. Calling all-methods-pure + nothing method from impure method (should report that impure method has no side effects)

2. Calling all-methods-pure + 'impure' method from impure method (no errors)

3. Calling all-methods-pure + nothing method from pure method (no errors)

4. Calling all-methods-pure + 'impure' method from pure method (should report a side effect in a pure method)

5. Calling all-methods-impure + nothing method from pure method (should report a side effect in a pure method)

6. Calling all-methods-impure + 'pure' method from pure method (no errors)

7. Calling all-methods-impure + nothing method from impure method (no errors)

8. Calling all-methods-impure + 'pure' method from impure method (should report that impure method has no side effects)

[ondrejmirtes]

I think that void methods (except the constructor) should be excluded from @phpstan-all-methods-pure by default.

By looking at logic in PhpMethodReflection::hasSideEffects you can see that PHPDocs don't affect that at all.

[VincentLanglet]

Lot of test added and void behavior :)