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..a31d646195f 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; @@ -31,38 +32,31 @@ final class FunctionPurityCheck { /** + * 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 - * @param ImpurePoint[] $impurePoints - * @param ThrowPoint[] $throwPoints - * @param Stmt[] $statements * @return list */ - public function check( + public function checkSignature( Scope $scope, string $functionDescription, string $identifier, FunctionReflection|ExtendedMethodReflection $functionReflection, array $parameters, Type $returnType, - array $impurePoints, - array $throwPoints, - array $statements, bool $isConstructor, ): array { $errors = []; - $isPure = $functionReflection->isPure(); $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 +86,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 +118,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,7 +143,38 @@ public function check( $functionDescription, ))->identifier(sprintf('pure%s.void', $identifier))->build(); } + } + + 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 @@ -226,6 +248,25 @@ public function check( return $errors; } + /** + * @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 $names; + } + /** * @param ImpurePoint[] $impurePoints * @param array $pureUnlessCallableParamNames 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 @@ +