From 4a1786e34efc1ba90165ff15bdbd5319ef735a42 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Sun, 4 Oct 2026 03:20:46 +0900 Subject: [PATCH 1/2] Check the purity declarations of methods without a body --- conf/bleedingEdge.neon | 1 + conf/config.level2.neon | 5 + conf/config.neon | 1 + conf/parametersSchema.neon | 1 + src/Rules/Pure/FunctionPurityCheck.php | 191 +++++++++++------- src/Rules/Pure/PureAbstractMethodRule.php | 48 +++++ .../Rules/Pure/PureAbstractMethodRuleTest.php | 53 +++++ .../Rules/Pure/data/pure-abstract-method.php | 84 ++++++++ 8 files changed, 309 insertions(+), 75 deletions(-) create mode 100644 src/Rules/Pure/PureAbstractMethodRule.php create mode 100644 tests/PHPStan/Rules/Pure/PureAbstractMethodRuleTest.php create mode 100644 tests/PHPStan/Rules/Pure/data/pure-abstract-method.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 7098877fb44..65c3a4b45ae 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -34,3 +34,4 @@ parameters: unusedVariable: true unusedParameters: true reuseCollectedDataRuleErrors: true + reportAbstractMethodPurity: true diff --git a/conf/config.level2.neon b/conf/config.level2.neon index bb615c07c34..8de4ba41c42 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -19,6 +19,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.checkDynamicConstantNameValues% PHPStan\Rules\Constants\ValueAssignedToGlobalConstantRule: phpstan.rules.rule: %featureToggles.checkDynamicConstantNameValues% + PHPStan\Rules\Pure\PureAbstractMethodRule: + phpstan.rules.rule: %featureToggles.reportAbstractMethodPurity% services: - @@ -32,3 +34,6 @@ services: - class: PHPStan\Rules\Constants\ValueAssignedToGlobalConstantRule + + - + class: PHPStan\Rules\Pure\PureAbstractMethodRule diff --git a/conf/config.neon b/conf/config.neon index 0eb6c0c764b..804adec7805 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -66,6 +66,7 @@ parameters: unusedVariable: false unusedParameters: false reuseCollectedDataRuleErrors: false + reportAbstractMethodPurity: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 62dab9d9c3b..85aa3c6d45f 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -63,6 +63,7 @@ parametersSchema: unusedVariable: bool() unusedParameters: bool() reuseCollectedDataRuleErrors: bool() + reportAbstractMethodPurity: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Pure/FunctionPurityCheck.php b/src/Rules/Pure/FunctionPurityCheck.php index 94d265f6732..a5e355fd16a 100644 --- a/src/Rules/Pure/FunctionPurityCheck.php +++ b/src/Rules/Pure/FunctionPurityCheck.php @@ -16,6 +16,7 @@ use PHPStan\Rules\Functions\CallToFunctionStatementWithoutSideEffectsRule; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\TrinaryLogic; use PHPStan\Type\Type; use function array_filter; use function array_key_exists; @@ -51,18 +52,111 @@ public function check( bool $isConstructor, ): array { - $errors = []; + $errors = $this->checkSignature($scope, $functionDescription, $identifier, $functionReflection, $parameters, $returnType, $isConstructor); $isPure = $functionReflection->isPure(); + $pureUnlessCallableParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessCallableIsImpureParameters()); + $pureUnlessParameterPassedParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessParameterPassedParameters()); + + if ($isPure->yes()) { + $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); + } elseif ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== []) { + // A function declared @pure-unless-callable-is-impure is pure except + // for the flagged callables, so its body is checked for purity while + // the flagged callables' own invocations are exempt. The same holds for + // @pure-unless-parameter-passed: writing through the flagged by-ref + // parameter is not an impure point, so the rest of the body still has + // to be pure for the conditional verdict to hold. + $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); + } elseif ($isPure->no()) { + if ( + count($throwPoints) === 0 + && count($impurePoints) === 0 + && count($functionReflection->getAsserts()->getAll()) === 0 + && ( + !$functionReflection instanceof ExtendedMethodReflection + || $functionReflection->isFinal()->yes() + || $functionReflection->getDeclaringClass()->isFinal() + ) + ) { + $errors[] = RuleErrorBuilder::message(sprintf( + '%s is marked as impure but does not have any side effects.', + $functionDescription, + ))->identifier(sprintf('impure%s.pure', $identifier))->build(); + } + } elseif ($returnType->isVoid()->yes()) { + if ( + count($throwPoints) === 0 + && count($impurePoints) === 0 + && !$isConstructor + && (!$functionReflection instanceof ExtendedMethodReflection || $functionReflection->isPrivate()) + && count($functionReflection->getAsserts()->getAll()) === 0 + ) { + $hasByRef = false; + foreach ($parameters as $parameter) { + if (!$parameter->passedByReference()->createsNewVariable()) { + continue; + } + + $hasByRef = true; + break; + } + + $statements = array_filter($statements, static function (Stmt $stmt): bool { + if ($stmt instanceof Stmt\Nop) { + return false; + } + + if (!$stmt instanceof Stmt\Expression) { + return true; + } + if (!$stmt->expr instanceof FuncCall) { + return true; + } + if (!$stmt->expr->name instanceof Name) { + return true; + } + + return !in_array($stmt->expr->name->toString(), CallToFunctionStatementWithoutSideEffectsRule::PHPSTAN_TESTING_FUNCTIONS, true); + }); + + if (!$hasByRef && count($statements) > 0) { + $errors[] = RuleErrorBuilder::message(sprintf( + '%s returns void but does not have any side effects.', + $functionDescription, + ))->identifier('void.pure')->build(); + } + } + } + + return $errors; + } + + /** + * The checks of a purity declaration that need only the signature, so they + * also apply to methods without a body. + * + * @param 'Function'|'Method'|'PropertyHook' $identifier + * @param ExtendedParameterReflection[] $parameters + * @return list + */ + public function checkSignature( + Scope $scope, + string $functionDescription, + string $identifier, + FunctionReflection|ExtendedMethodReflection $functionReflection, + array $parameters, + Type $returnType, + bool $isConstructor, + ): array + { + $errors = []; $pureUnlessCallableParameters = $functionReflection->getPureUnlessCallableIsImpureParameters(); - $pureUnlessCallableParamNames = []; foreach ($parameters as $parameter) { if (!array_key_exists($parameter->getName(), $pureUnlessCallableParameters)) { continue; } - $pureUnlessCallableParamNames[$parameter->getName()] = true; - $acceptors = $parameter->getType()->getCallableParametersAcceptors($scope); if (count($acceptors) === 0) { continue; @@ -92,14 +186,11 @@ public function check( } $pureUnlessParameterPassedParameters = $functionReflection->getPureUnlessParameterPassedParameters(); - $pureUnlessParameterPassedParamNames = []; foreach ($parameters as $parameter) { if (!array_key_exists($parameter->getName(), $pureUnlessParameterPassedParameters)) { continue; } - $pureUnlessParameterPassedParamNames[$parameter->getName()] = true; - if (!$parameter->isOptional()) { $errors[] = RuleErrorBuilder::message(sprintf( '%s is marked @pure-unless-parameter-passed for parameter $%s, but $%s is not optional, so %s is never pure.', @@ -127,7 +218,7 @@ public function check( ))->identifier(sprintf('pure%s.parameterPassedNotByRef', $identifier))->build(); } - if ($isPure->yes()) { + if ($functionReflection->isPure()->yes()) { foreach ($parameters as $parameter) { if (!$parameter->passedByReference()->createsNewVariable()) { continue; @@ -152,78 +243,28 @@ public function check( $functionDescription, ))->identifier(sprintf('pure%s.void', $identifier))->build(); } + } - $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); - } elseif ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== []) { - // A function declared @pure-unless-callable-is-impure is pure except - // for the flagged callables, so its body is checked for purity while - // the flagged callables' own invocations are exempt. The same holds for - // @pure-unless-parameter-passed: writing through the flagged by-ref - // parameter is not an impure point, so the rest of the body still has - // to be pure for the conditional verdict to hold. - $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); - } elseif ($isPure->no()) { - if ( - count($throwPoints) === 0 - && count($impurePoints) === 0 - && count($functionReflection->getAsserts()->getAll()) === 0 - && ( - !$functionReflection instanceof ExtendedMethodReflection - || $functionReflection->isFinal()->yes() - || $functionReflection->getDeclaringClass()->isFinal() - ) - ) { - $errors[] = RuleErrorBuilder::message(sprintf( - '%s is marked as impure but does not have any side effects.', - $functionDescription, - ))->identifier(sprintf('impure%s.pure', $identifier))->build(); - } - } elseif ($returnType->isVoid()->yes()) { - if ( - count($throwPoints) === 0 - && count($impurePoints) === 0 - && !$isConstructor - && (!$functionReflection instanceof ExtendedMethodReflection || $functionReflection->isPrivate()) - && count($functionReflection->getAsserts()->getAll()) === 0 - ) { - $hasByRef = false; - foreach ($parameters as $parameter) { - if (!$parameter->passedByReference()->createsNewVariable()) { - continue; - } - - $hasByRef = true; - break; - } - - $statements = array_filter($statements, static function (Stmt $stmt): bool { - if ($stmt instanceof Stmt\Nop) { - return false; - } - - if (!$stmt instanceof Stmt\Expression) { - return true; - } - if (!$stmt->expr instanceof FuncCall) { - return true; - } - if (!$stmt->expr->name instanceof Name) { - return true; - } - - return !in_array($stmt->expr->name->toString(), CallToFunctionStatementWithoutSideEffectsRule::PHPSTAN_TESTING_FUNCTIONS, true); - }); + return $errors; + } - if (!$hasByRef && count($statements) > 0) { - $errors[] = RuleErrorBuilder::message(sprintf( - '%s returns void but does not have any side effects.', - $functionDescription, - ))->identifier('void.pure')->build(); - } + /** + * @param ExtendedParameterReflection[] $parameters + * @param array $flaggedParameters + * @return array + */ + private function collectFlaggedParameterNames(array $parameters, array $flaggedParameters): array + { + $names = []; + foreach ($parameters as $parameter) { + if (!array_key_exists($parameter->getName(), $flaggedParameters)) { + continue; } + + $names[$parameter->getName()] = true; } - return $errors; + return $names; } /** diff --git a/src/Rules/Pure/PureAbstractMethodRule.php b/src/Rules/Pure/PureAbstractMethodRule.php new file mode 100644 index 00000000000..4a47fd13732 --- /dev/null +++ b/src/Rules/Pure/PureAbstractMethodRule.php @@ -0,0 +1,48 @@ + + */ +final class PureAbstractMethodRule implements Rule +{ + + public function __construct(private FunctionPurityCheck $check) + { + } + + public function getNodeType(): string + { + return InClassMethodNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node->getOriginalNode()->stmts !== null) { + return []; + } + + $method = $node->getMethodReflection(); + + return $this->check->checkSignature( + $scope, + sprintf('Method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName()), + 'Method', + $method, + $method->getParameters(), + $method->getReturnType(), + $method->isConstructor(), + ); + } + +} diff --git a/tests/PHPStan/Rules/Pure/PureAbstractMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureAbstractMethodRuleTest.php new file mode 100644 index 00000000000..83ab3dce01e --- /dev/null +++ b/tests/PHPStan/Rules/Pure/PureAbstractMethodRuleTest.php @@ -0,0 +1,53 @@ + + */ +class PureAbstractMethodRuleTest extends RuleTestCase +{ + + public function getRule(): Rule + { + return new PureAbstractMethodRule(new FunctionPurityCheck()); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/pure-abstract-method.php'], [ + [ + 'Method PureAbstractMethod\Foo::pureByRef() is marked as pure but parameter $p is passed by reference.', + 11, + ], + [ + 'Method PureAbstractMethod\Foo::pureVoid() is marked as pure but returns void.', + 16, + ], + [ + 'Method PureAbstractMethod\Foo::requiredCount() is marked @pure-unless-parameter-passed for parameter $count, but $count is not optional, so method PureAbstractMethod\Foo::requiredCount() is never pure.', + 33, + ], + [ + 'Method PureAbstractMethod\Foo::byValueFlag() is marked @pure-unless-parameter-passed for parameter $flag, but $flag is not passed by reference.', + 38, + ], + [ + 'Method PureAbstractMethod\Foo::alreadyPureCallback() is marked @pure-unless-callable-is-impure for parameter $cb, but $cb is already a pure callable, so method PureAbstractMethod\Foo::alreadyPureCallback() can be marked @phpstan-pure instead.', + 50, + ], + [ + 'Method PureAbstractMethod\Bar::pureByRef() is marked as pure but parameter $p is passed by reference.', + 66, + ], + [ + 'Method PureAbstractMethod\Bar::requiredCount() is marked @pure-unless-parameter-passed for parameter $count, but $count is not optional, so method PureAbstractMethod\Bar::requiredCount() is never pure.', + 72, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Pure/data/pure-abstract-method.php b/tests/PHPStan/Rules/Pure/data/pure-abstract-method.php new file mode 100644 index 00000000000..9210617a19d --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/pure-abstract-method.php @@ -0,0 +1,84 @@ + Date: Tue, 3 Nov 2026 02:15:51 +0900 Subject: [PATCH 2/2] Declare FunctionPurityCheck::checkSignature() before check() --- src/Rules/Pure/FunctionPurityCheck.php | 200 ++++++++++++------------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/src/Rules/Pure/FunctionPurityCheck.php b/src/Rules/Pure/FunctionPurityCheck.php index a5e355fd16a..a31d646195f 100644 --- a/src/Rules/Pure/FunctionPurityCheck.php +++ b/src/Rules/Pure/FunctionPurityCheck.php @@ -31,106 +31,6 @@ final class FunctionPurityCheck { - /** - * @param 'Function'|'Method'|'PropertyHook' $identifier - * @param ExtendedParameterReflection[] $parameters - * @param ImpurePoint[] $impurePoints - * @param ThrowPoint[] $throwPoints - * @param Stmt[] $statements - * @return list - */ - public function check( - Scope $scope, - string $functionDescription, - string $identifier, - FunctionReflection|ExtendedMethodReflection $functionReflection, - array $parameters, - Type $returnType, - array $impurePoints, - array $throwPoints, - array $statements, - bool $isConstructor, - ): array - { - $errors = $this->checkSignature($scope, $functionDescription, $identifier, $functionReflection, $parameters, $returnType, $isConstructor); - $isPure = $functionReflection->isPure(); - $pureUnlessCallableParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessCallableIsImpureParameters()); - $pureUnlessParameterPassedParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessParameterPassedParameters()); - - if ($isPure->yes()) { - $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); - } elseif ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== []) { - // A function declared @pure-unless-callable-is-impure is pure except - // for the flagged callables, so its body is checked for purity while - // the flagged callables' own invocations are exempt. The same holds for - // @pure-unless-parameter-passed: writing through the flagged by-ref - // parameter is not an impure point, so the rest of the body still has - // to be pure for the conditional verdict to hold. - $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); - } elseif ($isPure->no()) { - if ( - count($throwPoints) === 0 - && count($impurePoints) === 0 - && count($functionReflection->getAsserts()->getAll()) === 0 - && ( - !$functionReflection instanceof ExtendedMethodReflection - || $functionReflection->isFinal()->yes() - || $functionReflection->getDeclaringClass()->isFinal() - ) - ) { - $errors[] = RuleErrorBuilder::message(sprintf( - '%s is marked as impure but does not have any side effects.', - $functionDescription, - ))->identifier(sprintf('impure%s.pure', $identifier))->build(); - } - } elseif ($returnType->isVoid()->yes()) { - if ( - count($throwPoints) === 0 - && count($impurePoints) === 0 - && !$isConstructor - && (!$functionReflection instanceof ExtendedMethodReflection || $functionReflection->isPrivate()) - && count($functionReflection->getAsserts()->getAll()) === 0 - ) { - $hasByRef = false; - foreach ($parameters as $parameter) { - if (!$parameter->passedByReference()->createsNewVariable()) { - continue; - } - - $hasByRef = true; - break; - } - - $statements = array_filter($statements, static function (Stmt $stmt): bool { - if ($stmt instanceof Stmt\Nop) { - return false; - } - - if (!$stmt instanceof Stmt\Expression) { - return true; - } - if (!$stmt->expr instanceof FuncCall) { - return true; - } - if (!$stmt->expr->name instanceof Name) { - return true; - } - - return !in_array($stmt->expr->name->toString(), CallToFunctionStatementWithoutSideEffectsRule::PHPSTAN_TESTING_FUNCTIONS, true); - }); - - if (!$hasByRef && count($statements) > 0) { - $errors[] = RuleErrorBuilder::message(sprintf( - '%s returns void but does not have any side effects.', - $functionDescription, - ))->identifier('void.pure')->build(); - } - } - } - - return $errors; - } - /** * The checks of a purity declaration that need only the signature, so they * also apply to methods without a body. @@ -248,6 +148,106 @@ public function checkSignature( return $errors; } + /** + * @param 'Function'|'Method'|'PropertyHook' $identifier + * @param ExtendedParameterReflection[] $parameters + * @param ImpurePoint[] $impurePoints + * @param ThrowPoint[] $throwPoints + * @param Stmt[] $statements + * @return list + */ + public function check( + Scope $scope, + string $functionDescription, + string $identifier, + FunctionReflection|ExtendedMethodReflection $functionReflection, + array $parameters, + Type $returnType, + array $impurePoints, + array $throwPoints, + array $statements, + bool $isConstructor, + ): array + { + $errors = $this->checkSignature($scope, $functionDescription, $identifier, $functionReflection, $parameters, $returnType, $isConstructor); + $isPure = $functionReflection->isPure(); + $pureUnlessCallableParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessCallableIsImpureParameters()); + $pureUnlessParameterPassedParamNames = $this->collectFlaggedParameterNames($parameters, $functionReflection->getPureUnlessParameterPassedParameters()); + + if ($isPure->yes()) { + $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); + } elseif ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== []) { + // A function declared @pure-unless-callable-is-impure is pure except + // for the flagged callables, so its body is checked for purity while + // the flagged callables' own invocations are exempt. The same holds for + // @pure-unless-parameter-passed: writing through the flagged by-ref + // parameter is not an impure point, so the rest of the body still has + // to be pure for the conditional verdict to hold. + $errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription)); + } elseif ($isPure->no()) { + if ( + count($throwPoints) === 0 + && count($impurePoints) === 0 + && count($functionReflection->getAsserts()->getAll()) === 0 + && ( + !$functionReflection instanceof ExtendedMethodReflection + || $functionReflection->isFinal()->yes() + || $functionReflection->getDeclaringClass()->isFinal() + ) + ) { + $errors[] = RuleErrorBuilder::message(sprintf( + '%s is marked as impure but does not have any side effects.', + $functionDescription, + ))->identifier(sprintf('impure%s.pure', $identifier))->build(); + } + } elseif ($returnType->isVoid()->yes()) { + if ( + count($throwPoints) === 0 + && count($impurePoints) === 0 + && !$isConstructor + && (!$functionReflection instanceof ExtendedMethodReflection || $functionReflection->isPrivate()) + && count($functionReflection->getAsserts()->getAll()) === 0 + ) { + $hasByRef = false; + foreach ($parameters as $parameter) { + if (!$parameter->passedByReference()->createsNewVariable()) { + continue; + } + + $hasByRef = true; + break; + } + + $statements = array_filter($statements, static function (Stmt $stmt): bool { + if ($stmt instanceof Stmt\Nop) { + return false; + } + + if (!$stmt instanceof Stmt\Expression) { + return true; + } + if (!$stmt->expr instanceof FuncCall) { + return true; + } + if (!$stmt->expr->name instanceof Name) { + return true; + } + + return !in_array($stmt->expr->name->toString(), CallToFunctionStatementWithoutSideEffectsRule::PHPSTAN_TESTING_FUNCTIONS, true); + }); + + if (!$hasByRef && count($statements) > 0) { + $errors[] = RuleErrorBuilder::message(sprintf( + '%s returns void but does not have any side effects.', + $functionDescription, + ))->identifier('void.pure')->build(); + } + } + } + + return $errors; + } + /** * @param ExtendedParameterReflection[] $parameters * @param array $flaggedParameters