diff --git a/src/Analyser/PhpDocsResolver.php b/src/Analyser/PhpDocsResolver.php index 2bb8fe85215..2cc6717b222 100644 --- a/src/Analyser/PhpDocsResolver.php +++ b/src/Analyser/PhpDocsResolver.php @@ -215,7 +215,15 @@ public function getPhpDocs(Scope $scope, Node\FunctionLike|Node\Stmt\Property $n $acceptsNamedArguments = $scope->getClassReflection()->acceptsNamedArguments(); } - if ($isPure === null && $node instanceof Node\FunctionLike && $scope->isInClass()) { + // A @pure-unless-* tag on the method, written or inherited, states its purity + // just like @phpstan-pure does, so the class-level tags do not override it. + if ( + $isPure === null + && $phpDocPureUnlessCallableIsImpureParameters === [] + && $phpDocPureUnlessParameterPassedParameters === [] + && $node instanceof Node\FunctionLike + && $scope->isInClass() + ) { // a set hook has no return type node of its own, but it always returns // void - the class-level @phpstan-pure must not make it pure $isSetHook = $node instanceof Node\PropertyHook && $node->name->toLowerString() === 'set'; diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index 20f6500bc82..8f5dab55a61 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -971,7 +971,9 @@ public function createUserlandMethodReflection(ClassReflection $fileDeclaringCla } } - if ($isPure === null) { + // A @pure-unless-* tag on the method, written or inherited, states its purity + // just like @phpstan-pure does, so the class-level tags do not override it. + if ($isPure === null && $pureUnlessCallableIsImpureParameters === [] && $pureUnlessParameterPassedParameters === []) { $classResolvedPhpDoc = $phpDocBlockClassReflection->getResolvedPhpDoc(); if ($classResolvedPhpDoc !== null && $classResolvedPhpDoc->areAllMethodsPure()) { if ( diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index 321fc69c486..3d6ecee0c1a 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -33,7 +33,7 @@ final class TurboExtensionEnabler { - public const EXPECTED_EXTENSION_VERSION = '33df0ad'; + public const EXPECTED_EXTENSION_VERSION = '5ead7ed'; private static bool $active = false; diff --git a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php index f24317d6b61..20304585042 100644 --- a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php +++ b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php @@ -656,6 +656,10 @@ public function testPureUnlessCallableIsImpureOverride(): void 'Impure method MethodSignaturePureUnlessCallable\ImpureChild::run() overrides method MethodSignaturePureUnlessCallable\PureUnlessParent::run() marked @pure-unless-callable-is-impure.', 21, ], + [ + 'Impure method MethodSignaturePureUnlessCallable\ImpureChildOfAllMethodsPureParent::run() overrides method MethodSignaturePureUnlessCallable\AllMethodsPureParent::run() marked @pure-unless-callable-is-impure.', + 62, + ], ]); } @@ -670,8 +674,8 @@ public function testPureUnlessParameterPassedOverride(): void 22, ], [ - 'Impure method MethodSignaturePureUnlessParameterPassed\AllMethodsImpureChild::replace() overrides method MethodSignaturePureUnlessParameterPassed\PureUnlessParent::replace() marked @pure-unless-parameter-passed.', - 76, + 'Impure method MethodSignaturePureUnlessParameterPassed\ImpureChildOfAllMethodsPureParent::replace() overrides method MethodSignaturePureUnlessParameterPassed\AllMethodsPureParent::replace() marked @pure-unless-parameter-passed.', + 114, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-callable.php b/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-callable.php index 465bdfb396a..6c0a55f2c05 100644 --- a/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-callable.php +++ b/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-callable.php @@ -36,3 +36,47 @@ public function run(callable $cb): int } } + +/** + * @phpstan-all-methods-pure + */ +class AllMethodsPureParent +{ + + /** + * @pure-unless-callable-is-impure $cb + */ + public function run(callable $cb): int + { + return $cb(1); + } + +} + +class ImpureChildOfAllMethodsPureParent extends AllMethodsPureParent +{ + + /** + * @phpstan-impure + */ + public function run(callable $cb): int + { + echo 'side effect'; + + return $cb(1); + } + +} + +class PureChild implements PureUnlessParent +{ + + /** + * @phpstan-pure + */ + public function run(callable $cb): int + { + return 1; + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-parameter-passed.php b/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-parameter-passed.php index 600a91a91b9..20c1f5d0ab1 100644 --- a/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-parameter-passed.php +++ b/tests/PHPStan/Rules/Methods/data/method-signature-pure-unless-parameter-passed.php @@ -68,6 +68,9 @@ public function replace(string $subject, int &$count = 0): string } /** + * The inherited @pure-unless-parameter-passed takes precedence over the + * class-level tag, as an inherited @phpstan-pure does. + * * @phpstan-all-methods-impure */ class AllMethodsImpureChild implements PureUnlessParent @@ -82,3 +85,38 @@ public function replace(string $subject, int &$count = 0): string } } + +/** + * @phpstan-all-methods-pure + */ +class AllMethodsPureParent +{ + + /** + * @param-out int $count + * @pure-unless-parameter-passed $count + */ + public function replace(string $subject, int &$count = 0): string + { + $count = 1; + + return $subject; + } + +} + +class ImpureChildOfAllMethodsPureParent extends AllMethodsPureParent +{ + + /** + * @phpstan-impure + */ + public function replace(string $subject, int &$count = 0): string + { + echo 'side effect'; + $count = 1; + + return $subject; + } + +} diff --git a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php b/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php index d6cf230e4c0..24f2a1a9d16 100644 --- a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php @@ -541,4 +541,26 @@ public function testPureUnlessParameterPassedBuiltin(): void ]); } + public function testPureUnlessAllMethodsPure(): void + { + $this->analyse([__DIR__ . '/data/pure-unless-all-methods-pure.php'], [ + [ + 'Impure call to method PureUnlessAllMethodsPure\InheritingReplacer::replace() in pure function PureUnlessAllMethodsPure\passingCount().', + 111, + ], + [ + 'Impure call to method PureUnlessAllMethodsPure\Replacer::replace() in pure function PureUnlessAllMethodsPure\passingCount().', + 111, + ], + [ + 'Impure call to method PureUnlessAllMethodsPure\InheritingReplacer::map() in pure function PureUnlessAllMethodsPure\impureCallback().', + 135, + ], + [ + 'Impure call to method PureUnlessAllMethodsPure\Replacer::map() in pure function PureUnlessAllMethodsPure\impureCallback().', + 135, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php index f67e9172b60..e2d665c6b72 100644 --- a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php @@ -438,4 +438,18 @@ public function testPureUnlessParameterPassed(): void ]); } + public function testPureUnlessAllMethodsPure(): void + { + $this->analyse([__DIR__ . '/data/pure-unless-all-methods-pure.php'], [ + [ + 'Impure echo in pure method PureUnlessAllMethodsPure\ImpureClassInheritingReplacer::replace().', + 78, + ], + [ + 'Impure echo in pure method PureUnlessAllMethodsPure\ImpureClassInheritingReplacer::map().', + 86, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Pure/data/pure-unless-all-methods-pure.php b/tests/PHPStan/Rules/Pure/data/pure-unless-all-methods-pure.php new file mode 100644 index 00000000000..7c517036de0 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/pure-unless-all-methods-pure.php @@ -0,0 +1,136 @@ +replace($s) . $i->replace($s); +} + +/** + * @phpstan-pure + */ +function passingCount(Replacer $r, InheritingReplacer $i, string $s): string +{ + $c = 0; + $d = 0; + // $count is passed, so the calls are impure although the classes are marked + // @phpstan-all-methods-pure. + return $r->replace($s, $c) . $i->replace($s, $d); +} + +/** + * @phpstan-pure + */ +function pureCallback(Replacer $r, InheritingReplacer $i, string $s): string +{ + return $r->map(static fn (string $x): string => $x, $s) . $i->map(static fn (string $x): string => $x, $s); +} + +/** + * @phpstan-pure + */ +function impureCallback(Replacer $r, InheritingReplacer $i, string $s): string +{ + // The callback is impure, so the calls are impure although the classes are + // marked @phpstan-all-methods-pure. + $cb = static function (string $x): string { + echo $x; + + return $x; + }; + + return $r->map($cb, $s) . $i->map($cb, $s); +} diff --git a/turbo-ext/src/PhpClassReflectionExtension.cpp b/turbo-ext/src/PhpClassReflectionExtension.cpp index 9f2d92b8451..bd160fc30ad 100644 --- a/turbo-ext/src/PhpClassReflectionExtension.cpp +++ b/turbo-ext/src/PhpClassReflectionExtension.cpp @@ -2403,7 +2403,13 @@ class PhpClassReflectionExtension } } - if (isPure < 0) { + // A @pure-unless-* tag on the method, written or inherited, states its purity + // just like @phpstan-pure does, so the class-level tags do not override it. + if ( + isPure < 0 + && zend_hash_num_elements(pureUnlessCallableIsImpureParameters.table()) == 0 + && zend_hash_num_elements(pureUnlessParameterPassedParameters.table()) == 0 + ) { zv::Val classResolvedPhpDoc = call(phpDocBlockClassReflection.raw(), PT_LC("getresolvedphpdoc")); if (UNEXPECTED(classResolvedPhpDoc.isUndef())) return zv::Val(); if (Z_TYPE_P(classResolvedPhpDoc.raw()) != IS_NULL) { diff --git a/turbo-ext/src/PhpDocsResolver.cpp b/turbo-ext/src/PhpDocsResolver.cpp index 039d21f19c9..2f58a7aaf7d 100644 --- a/turbo-ext/src/PhpDocsResolver.cpp +++ b/turbo-ext/src/PhpDocsResolver.cpp @@ -489,7 +489,14 @@ class PhpDocsResolver } } - if (isPure.isNull() && isFunctionLike) { + // A @pure-unless-* tag on the method, written or inherited, states its purity + // just like @phpstan-pure does, so the class-level tags do not override it. + if ( + isPure.isNull() + && zend_hash_num_elements(phpDocPureUnlessCallableIsImpureParameters.table()) == 0 + && zend_hash_num_elements(phpDocPureUnlessParameterPassedParameters.table()) == 0 + && isFunctionLike + ) { bool stillInClass; if (UNEXPECTED(!pt_scope_is_in_class(Z_OBJ_P(scope), stillInClass))) return false; if (stillInClass && UNEXPECTED(!resolveClassPurity(scope, node, classReflection, functionName.raw(), phpDocReturnType.raw(), isPure))) return false; diff --git a/turbo-ext/tests/php-class-reflection-family-fixture.php b/turbo-ext/tests/php-class-reflection-family-fixture.php index 14da17e0b55..39c05650872 100644 --- a/turbo-ext/tests/php-class-reflection-family-fixture.php +++ b/turbo-ext/tests/php-class-reflection-family-fixture.php @@ -221,3 +221,83 @@ public function rich(callable $callback, int &$counter, mixed $input): bool } } + +/** + * The class-level tag applies to plain(); the methods carrying a + * @pure-unless-* tag keep their conditional purity. + * + * @phpstan-all-methods-pure + */ +class FixtureAllMethodsPure +{ + + /** + * @param-out int $count + * @pure-unless-parameter-passed $count + */ + public function replaceWithCount(string $subject, int &$count = 0): string + { + $count = 1; + + return $subject; + } + + /** + * @param callable(string): string $cb + * @pure-unless-callable-is-impure $cb + */ + public function mapWithCallback(callable $cb, string $subject): string + { + return $cb($subject); + } + + public function plain(): int + { + return 1; + } + +} + +interface FixturePureUnlessInterface +{ + + /** + * @param-out int $count + * @pure-unless-parameter-passed $count + */ + public function replaceWithCount(string $subject, int &$count = 0): string; + + /** + * @param callable(string): string $cb + * @pure-unless-callable-is-impure $cb + */ + public function mapWithCallback(callable $cb, string $subject): string; + +} + +/** + * The inherited @pure-unless-* tags take precedence over the class-level tag. + * + * @phpstan-all-methods-impure + */ +class FixtureAllMethodsImpureInheriting implements FixturePureUnlessInterface +{ + + public function replaceWithCount(string $subject, int &$count = 0): string + { + $count = 1; + + return $subject; + } + + public function mapWithCallback(callable $cb, string $subject): string + { + return $cb($subject); + } + + public function plain(): int + { + return 1; + } + +} diff --git a/turbo-ext/tests/php-class-reflection-family.php b/turbo-ext/tests/php-class-reflection-family.php index fdf3f9be371..9dae5109411 100644 --- a/turbo-ext/tests/php-class-reflection-family.php +++ b/turbo-ext/tests/php-class-reflection-family.php @@ -360,6 +360,9 @@ public function normalizeClass(string $class): string 'PhpClassReflectionFamilyFixture\FixtureAnnotated', 'PhpClassReflectionFamilyFixture\FixtureAnnotatedStrict', 'PhpClassReflectionFamilyFixture\FixtureTrait', + 'PhpClassReflectionFamilyFixture\FixtureAllMethodsPure', + 'PhpClassReflectionFamilyFixture\FixturePureUnlessInterface', + 'PhpClassReflectionFamilyFixture\FixtureAllMethodsImpureInheriting', 'Exception', 'ArrayObject', 'DateTimeImmutable', @@ -376,6 +379,7 @@ public function normalizeClass(string $class): string '__construct', '__get', '__call', 'name', 'value', 'cases', 'from', 'tryFrom', 'label', 'getMessage', 'getCode', 'getPrevious', 'count', 'offsetGet', 'offsetSet', 'format', 'modify', 'attach', 'fromCallable', 'bindTo', 'nonExistentMember', + 'replaceWithCount', 'mapWithCallback', 'plain', // the adapter's `$name === ''` early return, and a spelling the // lowercased-name memo has to normalize '', 'GETMESSAGE', 'TraitMethod', @@ -424,7 +428,7 @@ public function normalizeClass(string $class): string } // createUserlandMethodReflection on the class's own native methods - foreach (['traitMethod', 'rich', 'fromInterface', '__construct'] as $pcreMethodName) { + foreach (['traitMethod', 'rich', 'fromInterface', '__construct', 'replaceWithCount', 'mapWithCallback', 'plain'] as $pcreMethodName) { try { $pcreNative = $pcreClassReflection->getNativeReflection(); if (!$pcreNative->hasMethod($pcreMethodName)) { diff --git a/turbo-ext/tests/walk-trace-fixtures/pure-unless-all-methods-pure.php b/turbo-ext/tests/walk-trace-fixtures/pure-unless-all-methods-pure.php new file mode 100644 index 00000000000..31b288872e8 --- /dev/null +++ b/turbo-ext/tests/walk-trace-fixtures/pure-unless-all-methods-pure.php @@ -0,0 +1,139 @@ +replace($s) . $i->replace($s); +} + +/** + * @phpstan-pure + */ +function passingCount(Replacer $r, InheritingReplacer $i, string $s): string +{ + $c = 0; + $d = 0; + // $count is passed, so the calls are impure although the classes are marked + // @phpstan-all-methods-pure. + return $r->replace($s, $c) . $i->replace($s, $d); +} + +/** + * @phpstan-pure + */ +function pureCallback(Replacer $r, InheritingReplacer $i, string $s): string +{ + return $r->map(static fn (string $x): string => $x, $s) . $i->map(static fn (string $x): string => $x, $s); +} + +/** + * @phpstan-pure + */ +function impureCallback(Replacer $r, InheritingReplacer $i, string $s): string +{ + // The callback is impure, so the calls are impure although the classes are + // marked @phpstan-all-methods-pure. + $cb = static function (string $x): string { + echo $x; + + return $x; + }; + + return $r->map($cb, $s) . $i->map($cb, $s); +}