Skip to content

Commit 9e4cf9d

Browse files
SanderMullerclaude
SanderMuller
and
claude
authored
Extract the shared parameter-out type comparison into ParameterOutTypeCheck (#6238)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 177821d commit 9e4cf9d

5 files changed

Lines changed: 125 additions & 102 deletions

File tree

‎src/Rules/Variables/ParameterOutAssignedTypeRule.php‎

Lines changed: 6 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,9 @@
66
use PHPStan\Analyser\Scope;
77
use PHPStan\DependencyInjection\RegisteredRule;
88
use PHPStan\Node\VariableAssignNode;
9-
use PHPStan\Reflection\ExtendedMethodReflection;
109
use PHPStan\Rules\Rule;
11-
use PHPStan\Rules\RuleErrorBuilder;
12-
use PHPStan\Rules\RuleLevelHelper;
13-
use PHPStan\Type\ErrorType;
14-
use PHPStan\Type\Type;
1510
use PHPStan\Type\TypeUtils;
16-
use PHPStan\Type\VerbosityLevel;
1711
use function is_string;
18-
use function sprintf;
1912

2013
/**
2114
* @implements Rule<VariableAssignNode>
@@ -25,7 +18,7 @@ final class ParameterOutAssignedTypeRule implements Rule
2518
{
2619

2720
public function __construct(
28-
private RuleLevelHelper $ruleLevelHelper,
21+
private ParameterOutTypeCheck $parameterOutTypeCheck,
2922
)
3023
{
3124
}
@@ -78,45 +71,14 @@ public function processNode(Node $node, Scope $scope): array
7871

7972
$outType = TypeUtils::resolveLateResolvableTypes($outType);
8073

81-
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
74+
return $this->parameterOutTypeCheck->check(
8275
$scope,
76+
$inFunction,
77+
$foundParameter,
8378
$node->getAssignedExpr(),
84-
'',
85-
static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(),
79+
$outType,
80+
$isParamOutType,
8681
);
87-
$type = $typeResult->getType();
88-
if ($type instanceof ErrorType) {
89-
return $typeResult->getUnknownClassErrors();
90-
}
91-
92-
$assignedExprType = $scope->getType($node->getAssignedExpr());
93-
if ($outType->isSuperTypeOf($assignedExprType)->yes()) {
94-
return [];
95-
}
96-
97-
if ($inFunction instanceof ExtendedMethodReflection) {
98-
$functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName());
99-
} else {
100-
$functionDescription = sprintf('function %s()', $inFunction->getName());
101-
}
102-
103-
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType);
104-
$errorBuilder = RuleErrorBuilder::message(sprintf(
105-
'Parameter &$%s %s of %s expects %s, %s given.',
106-
$foundParameter->getName(),
107-
$isParamOutType ? '@param-out type' : 'by-ref type',
108-
$functionDescription,
109-
$outType->describe($verbosityLevel),
110-
$assignedExprType->describe($verbosityLevel),
111-
))->identifier(sprintf('%s.type', $isParamOutType ? 'paramOut' : 'parameterByRef'));
112-
113-
if (!$isParamOutType) {
114-
$errorBuilder->tip('You can change the parameter out type with @param-out PHPDoc tag.');
115-
}
116-
117-
return [
118-
$errorBuilder->build(),
119-
];
12082
}
12183

12284
}

‎src/Rules/Variables/ParameterOutExecutionEndTypeRule.php‎

Lines changed: 7 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,8 @@
1212
use PHPStan\Reflection\FunctionReflection;
1313
use PHPStan\Rules\IdentifierRuleError;
1414
use PHPStan\Rules\Rule;
15-
use PHPStan\Rules\RuleErrorBuilder;
16-
use PHPStan\Rules\RuleLevelHelper;
17-
use PHPStan\Type\ErrorType;
1815
use PHPStan\Type\NeverType;
19-
use PHPStan\Type\Type;
2016
use PHPStan\Type\TypeUtils;
21-
use PHPStan\Type\VerbosityLevel;
22-
use function sprintf;
2317

2418
/**
2519
* @implements Rule<ExecutionEndNode>
@@ -29,7 +23,7 @@ final class ParameterOutExecutionEndTypeRule implements Rule
2923
{
3024

3125
public function __construct(
32-
private RuleLevelHelper $ruleLevelHelper,
26+
private ParameterOutTypeCheck $parameterOutTypeCheck,
3327
)
3428
{
3529
}
@@ -94,41 +88,14 @@ private function processSingleParameter(
9488

9589
$outType = TypeUtils::resolveLateResolvableTypes($outType);
9690

97-
$variableExpr = new Node\Expr\Variable($parameter->getName());
98-
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
91+
return $this->parameterOutTypeCheck->check(
9992
$scope,
100-
$variableExpr,
101-
'',
102-
static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(),
93+
$inFunction,
94+
$parameter,
95+
new Node\Expr\Variable($parameter->getName()),
96+
$outType,
97+
true, // this rule only runs when @param-out is present
10398
);
104-
$type = $typeResult->getType();
105-
if ($type instanceof ErrorType) {
106-
return $typeResult->getUnknownClassErrors();
107-
}
108-
109-
$assignedExprType = $scope->getType($variableExpr);
110-
if ($outType->isSuperTypeOf($assignedExprType)->yes()) {
111-
return [];
112-
}
113-
114-
if ($inFunction instanceof ExtendedMethodReflection) {
115-
$functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName());
116-
} else {
117-
$functionDescription = sprintf('function %s()', $inFunction->getName());
118-
}
119-
120-
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType);
121-
$errorBuilder = RuleErrorBuilder::message(sprintf(
122-
'Parameter &$%s @param-out type of %s expects %s, %s given.',
123-
$parameter->getName(),
124-
$functionDescription,
125-
$outType->describe($verbosityLevel),
126-
$assignedExprType->describe($verbosityLevel),
127-
))->identifier(sprintf('paramOut.type'));
128-
129-
return [
130-
$errorBuilder->build(),
131-
];
13299
}
133100

134101
}

‎src/Rules/Variables/ParameterOutTypeCheck.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PhpParser\Node\Expr;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\DependencyInjection\AutowiredService;
8+
use PHPStan\Reflection\ExtendedMethodReflection;
9+
use PHPStan\Reflection\FunctionReflection;
10+
use PHPStan\Reflection\ParameterReflection;
11+
use PHPStan\Rules\IdentifierRuleError;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\Rules\RuleLevelHelper;
14+
use PHPStan\Type\ErrorType;
15+
use PHPStan\Type\Type;
16+
use PHPStan\Type\VerbosityLevel;
17+
use function sprintf;
18+
19+
/**
20+
* Compares what a by-ref parameter is left holding against the type its callers are promised.
21+
*
22+
* The promise is either an explicit `@param-out` or, in its absence, the parameter's own type.
23+
* Which one it is only shows in the error message, so callers report it via $isParamOutType.
24+
*
25+
* @internal
26+
*/
27+
#[AutowiredService]
28+
final class ParameterOutTypeCheck
29+
{
30+
31+
public function __construct(
32+
private RuleLevelHelper $ruleLevelHelper,
33+
)
34+
{
35+
}
36+
37+
/**
38+
* @param Type $outType Already passed through TypeUtils::resolveLateResolvableTypes()
39+
* @return list<IdentifierRuleError>
40+
*/
41+
public function check(
42+
Scope $scope,
43+
FunctionReflection|ExtendedMethodReflection $inFunction,
44+
ParameterReflection $parameter,
45+
Expr $checkedExpr,
46+
Type $outType,
47+
bool $isParamOutType,
48+
): array
49+
{
50+
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
51+
$scope,
52+
$checkedExpr,
53+
'',
54+
static fn (Type $type): bool => $outType->isSuperTypeOf($type)->yes(),
55+
);
56+
if ($typeResult->getType() instanceof ErrorType) {
57+
return $typeResult->getUnknownClassErrors();
58+
}
59+
60+
$assignedExprType = $scope->getType($checkedExpr);
61+
if ($outType->isSuperTypeOf($assignedExprType)->yes()) {
62+
return [];
63+
}
64+
65+
if ($inFunction instanceof ExtendedMethodReflection) {
66+
$functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName());
67+
} else {
68+
$functionDescription = sprintf('function %s()', $inFunction->getName());
69+
}
70+
71+
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($outType, $assignedExprType);
72+
$errorBuilder = RuleErrorBuilder::message(sprintf(
73+
'Parameter &$%s %s of %s expects %s, %s given.',
74+
$parameter->getName(),
75+
$isParamOutType ? '@param-out type' : 'by-ref type',
76+
$functionDescription,
77+
$outType->describe($verbosityLevel),
78+
$assignedExprType->describe($verbosityLevel),
79+
))->identifier(sprintf('%s.type', $isParamOutType ? 'paramOut' : 'parameterByRef'));
80+
81+
if (!$isParamOutType) {
82+
$errorBuilder->tip('You can change the parameter out type with @param-out PHPDoc tag.');
83+
}
84+
85+
return [
86+
$errorBuilder->build(),
87+
];
88+
}
89+
90+
}

‎tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@ class ParameterOutAssignedTypeRuleTest extends RuleTestCase
1616
protected function getRule(): TRule
1717
{
1818
return new ParameterOutAssignedTypeRule(
19-
new RuleLevelHelper(
20-
self::createReflectionProvider(),
21-
checkNullables: true,
22-
checkThisOnly: false,
23-
checkUnionTypes: true,
24-
checkExplicitMixed: true,
25-
checkImplicitMixed: false,
26-
checkBenevolentUnionTypes: false,
27-
discoveringSymbolsTip: true,
19+
new ParameterOutTypeCheck(
20+
new RuleLevelHelper(
21+
self::createReflectionProvider(),
22+
checkNullables: true,
23+
checkThisOnly: false,
24+
checkUnionTypes: true,
25+
checkExplicitMixed: true,
26+
checkImplicitMixed: false,
27+
checkBenevolentUnionTypes: false,
28+
discoveringSymbolsTip: true,
29+
),
2830
),
2931
);
3032
}

‎tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,17 @@ class ParameterOutExecutionEndTypeRuleTest extends RuleTestCase
1515
protected function getRule(): Rule
1616
{
1717
return new ParameterOutExecutionEndTypeRule(
18-
new RuleLevelHelper(
19-
self::createReflectionProvider(),
20-
checkNullables: true,
21-
checkThisOnly: false,
22-
checkUnionTypes: true,
23-
checkExplicitMixed: true,
24-
checkImplicitMixed: false,
25-
checkBenevolentUnionTypes: false,
26-
discoveringSymbolsTip: true,
18+
new ParameterOutTypeCheck(
19+
new RuleLevelHelper(
20+
self::createReflectionProvider(),
21+
checkNullables: true,
22+
checkThisOnly: false,
23+
checkUnionTypes: true,
24+
checkExplicitMixed: true,
25+
checkImplicitMixed: false,
26+
checkBenevolentUnionTypes: false,
27+
discoveringSymbolsTip: true,
28+
),
2729
),
2830
);
2931
}

0 commit comments

Comments
 (0)