From 7cc7e0e80e7df221f68c1a39f775ac97ffdf211e Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Tue, 18 Aug 2026 22:44:52 +0200 Subject: [PATCH] Extract the shared parameter-out type comparison into ParameterOutTypeCheck ParameterOutAssignedTypeRule and ParameterOutExecutionEndTypeRule both compare what a by-ref parameter is left holding against the type its callers are promised, and both build the same message from it - the execution-end rule is the assigned-value rule with @param-out known to be present. The comparison, the level-dependent filtering, the function description and the message now live in one place. No behaviour change: the messages, identifiers and the tip are unchanged, and both rule tests keep their expectations - only how the rule is constructed differs. Co-Authored-By: Claude Opus 5 (1M context) --- .../ParameterOutAssignedTypeRule.php | 50 ++--------- .../ParameterOutExecutionEndTypeRule.php | 47 ++-------- src/Rules/Variables/ParameterOutTypeCheck.php | 90 +++++++++++++++++++ .../ParameterOutAssignedTypeRuleTest.php | 20 +++-- .../ParameterOutExecutionEndTypeRuleTest.php | 20 +++-- 5 files changed, 125 insertions(+), 102 deletions(-) create mode 100644 src/Rules/Variables/ParameterOutTypeCheck.php diff --git a/src/Rules/Variables/ParameterOutAssignedTypeRule.php b/src/Rules/Variables/ParameterOutAssignedTypeRule.php index 5bc01180571..b6869b42be5 100644 --- a/src/Rules/Variables/ParameterOutAssignedTypeRule.php +++ b/src/Rules/Variables/ParameterOutAssignedTypeRule.php @@ -6,16 +6,9 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Node\VariableAssignNode; -use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Rules\Rule; -use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\Rules\RuleLevelHelper; -use PHPStan\Type\ErrorType; -use PHPStan\Type\Type; use PHPStan\Type\TypeUtils; -use PHPStan\Type\VerbosityLevel; use function is_string; -use function sprintf; /** * @implements Rule @@ -25,7 +18,7 @@ final class ParameterOutAssignedTypeRule implements Rule { public function __construct( - private RuleLevelHelper $ruleLevelHelper, + private ParameterOutTypeCheck $parameterOutTypeCheck, ) { } @@ -78,45 +71,14 @@ public function processNode(Node $node, Scope $scope): array $outType = TypeUtils::resolveLateResolvableTypes($outType); - $typeResult = $this->ruleLevelHelper->findTypeToCheck( + return $this->parameterOutTypeCheck->check( $scope, + $inFunction, + $foundParameter, $node->getAssignedExpr(), - '', - static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(), + $outType, + $isParamOutType, ); - $type = $typeResult->getType(); - if ($type instanceof ErrorType) { - return $typeResult->getUnknownClassErrors(); - } - - $assignedExprType = $scope->getType($node->getAssignedExpr()); - if ($outType->isSuperTypeOf($assignedExprType)->yes()) { - return []; - } - - if ($inFunction instanceof ExtendedMethodReflection) { - $functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName()); - } else { - $functionDescription = sprintf('function %s()', $inFunction->getName()); - } - - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType); - $errorBuilder = RuleErrorBuilder::message(sprintf( - 'Parameter &$%s %s of %s expects %s, %s given.', - $foundParameter->getName(), - $isParamOutType ? '@param-out type' : 'by-ref type', - $functionDescription, - $outType->describe($verbosityLevel), - $assignedExprType->describe($verbosityLevel), - ))->identifier(sprintf('%s.type', $isParamOutType ? 'paramOut' : 'parameterByRef')); - - if (!$isParamOutType) { - $errorBuilder->tip('You can change the parameter out type with @param-out PHPDoc tag.'); - } - - return [ - $errorBuilder->build(), - ]; } } diff --git a/src/Rules/Variables/ParameterOutExecutionEndTypeRule.php b/src/Rules/Variables/ParameterOutExecutionEndTypeRule.php index edddff8a913..1cfc3bcc9b6 100644 --- a/src/Rules/Variables/ParameterOutExecutionEndTypeRule.php +++ b/src/Rules/Variables/ParameterOutExecutionEndTypeRule.php @@ -12,14 +12,8 @@ use PHPStan\Reflection\FunctionReflection; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; -use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\Rules\RuleLevelHelper; -use PHPStan\Type\ErrorType; use PHPStan\Type\NeverType; -use PHPStan\Type\Type; use PHPStan\Type\TypeUtils; -use PHPStan\Type\VerbosityLevel; -use function sprintf; /** * @implements Rule @@ -29,7 +23,7 @@ final class ParameterOutExecutionEndTypeRule implements Rule { public function __construct( - private RuleLevelHelper $ruleLevelHelper, + private ParameterOutTypeCheck $parameterOutTypeCheck, ) { } @@ -94,41 +88,14 @@ private function processSingleParameter( $outType = TypeUtils::resolveLateResolvableTypes($outType); - $variableExpr = new Node\Expr\Variable($parameter->getName()); - $typeResult = $this->ruleLevelHelper->findTypeToCheck( + return $this->parameterOutTypeCheck->check( $scope, - $variableExpr, - '', - static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(), + $inFunction, + $parameter, + new Node\Expr\Variable($parameter->getName()), + $outType, + true, // this rule only runs when @param-out is present ); - $type = $typeResult->getType(); - if ($type instanceof ErrorType) { - return $typeResult->getUnknownClassErrors(); - } - - $assignedExprType = $scope->getType($variableExpr); - if ($outType->isSuperTypeOf($assignedExprType)->yes()) { - return []; - } - - if ($inFunction instanceof ExtendedMethodReflection) { - $functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName()); - } else { - $functionDescription = sprintf('function %s()', $inFunction->getName()); - } - - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType); - $errorBuilder = RuleErrorBuilder::message(sprintf( - 'Parameter &$%s @param-out type of %s expects %s, %s given.', - $parameter->getName(), - $functionDescription, - $outType->describe($verbosityLevel), - $assignedExprType->describe($verbosityLevel), - ))->identifier(sprintf('paramOut.type')); - - return [ - $errorBuilder->build(), - ]; } } diff --git a/src/Rules/Variables/ParameterOutTypeCheck.php b/src/Rules/Variables/ParameterOutTypeCheck.php new file mode 100644 index 00000000000..b2ddbf64f02 --- /dev/null +++ b/src/Rules/Variables/ParameterOutTypeCheck.php @@ -0,0 +1,90 @@ + + */ + public function check( + Scope $scope, + FunctionReflection|ExtendedMethodReflection $inFunction, + ParameterReflection $parameter, + Expr $checkedExpr, + Type $outType, + bool $isParamOutType, + ): array + { + $typeResult = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $checkedExpr, + '', + static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(), + ); + if ($typeResult->getType() instanceof ErrorType) { + return $typeResult->getUnknownClassErrors(); + } + + $assignedExprType = $scope->getType($checkedExpr); + if ($outType->isSuperTypeOf($assignedExprType)->yes()) { + return []; + } + + if ($inFunction instanceof ExtendedMethodReflection) { + $functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName()); + } else { + $functionDescription = sprintf('function %s()', $inFunction->getName()); + } + + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType); + $errorBuilder = RuleErrorBuilder::message(sprintf( + 'Parameter &$%s %s of %s expects %s, %s given.', + $parameter->getName(), + $isParamOutType ? '@param-out type' : 'by-ref type', + $functionDescription, + $outType->describe($verbosityLevel), + $assignedExprType->describe($verbosityLevel), + ))->identifier(sprintf('%s.type', $isParamOutType ? 'paramOut' : 'parameterByRef')); + + if (!$isParamOutType) { + $errorBuilder->tip('You can change the parameter out type with @param-out PHPDoc tag.'); + } + + return [ + $errorBuilder->build(), + ]; + } + +} diff --git a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php index 4baf8fe8993..abba16bcb0e 100644 --- a/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php +++ b/tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php @@ -16,15 +16,17 @@ class ParameterOutAssignedTypeRuleTest extends RuleTestCase protected function getRule(): TRule { return new ParameterOutAssignedTypeRule( - new RuleLevelHelper( - self::createReflectionProvider(), - checkNullables: true, - checkThisOnly: false, - checkUnionTypes: true, - checkExplicitMixed: true, - checkImplicitMixed: false, - checkBenevolentUnionTypes: false, - discoveringSymbolsTip: true, + new ParameterOutTypeCheck( + new RuleLevelHelper( + self::createReflectionProvider(), + checkNullables: true, + checkThisOnly: false, + checkUnionTypes: true, + checkExplicitMixed: true, + checkImplicitMixed: false, + checkBenevolentUnionTypes: false, + discoveringSymbolsTip: true, + ), ), ); } diff --git a/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php b/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php index 969ad2af409..311ffea2114 100644 --- a/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php +++ b/tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php @@ -15,15 +15,17 @@ class ParameterOutExecutionEndTypeRuleTest extends RuleTestCase protected function getRule(): Rule { return new ParameterOutExecutionEndTypeRule( - new RuleLevelHelper( - self::createReflectionProvider(), - checkNullables: true, - checkThisOnly: false, - checkUnionTypes: true, - checkExplicitMixed: true, - checkImplicitMixed: false, - checkBenevolentUnionTypes: false, - discoveringSymbolsTip: true, + new ParameterOutTypeCheck( + new RuleLevelHelper( + self::createReflectionProvider(), + checkNullables: true, + checkThisOnly: false, + checkUnionTypes: true, + checkExplicitMixed: true, + checkImplicitMixed: false, + checkBenevolentUnionTypes: false, + discoveringSymbolsTip: true, + ), ), ); }