Skip to content

Commit 92e29c7

Browse files
ondrejmirtesondrejmirtes
committed
Bleeding edge - check @param-out type
1 parent 0a109da commit 92e29c7

File tree

9 files changed

+288
-3
lines changed

9 files changed

+288
-3
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,4 @@ parameters:
4949
callUserFunc: true
5050
finalByPhpDoc: true
5151
magicConstantOutOfContext: true
52+
paramOutType: true

‎conf/config.level3.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ conditionalTags:
88
phpstan.rules.rule: %featureToggles.readOnlyByPhpDoc%
99
PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRule:
1010
phpstan.rules.rule: %featureToggles.readOnlyByPhpDoc%
11+
PHPStan\Rules\Variables\ParameterOutAssignedTypeRule:
12+
phpstan.rules.rule: %featureToggles.paramOutType%
1113

1214
rules:
1315
- PHPStan\Rules\Arrays\ArrayDestructuringRule
@@ -92,3 +94,5 @@ services:
9294

9395
-
9496
class: PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRule
97+
-
98+
class: PHPStan\Rules\Variables\ParameterOutAssignedTypeRule

‎conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ parameters:
8484
callUserFunc: false
8585
finalByPhpDoc: false
8686
magicConstantOutOfContext: false
87+
paramOutType: false
8788
fileExtensions:
8889
- php
8990
checkAdvancedIsset: false

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ parametersSchema:
7979
callUserFunc: bool()
8080
finalByPhpDoc: bool()
8181
magicConstantOutOfContext: bool()
82+
paramOutType: bool()
8283
])
8384
fileExtensions: listOf(string())
8485
checkAdvancedIsset: bool()

‎src/Analyser/NodeScopeResolver.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
use PHPStan\Node\Expr\PropertyInitializationExpr;
8484
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
8585
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
86+
use PHPStan\Node\Expr\TypeExpr;
8687
use PHPStan\Node\Expr\UnsetOffsetExpr;
8788
use PHPStan\Node\FinallyExitPointsNode;
8889
use PHPStan\Node\FunctionCallableNode;
@@ -107,6 +108,7 @@
107108
use PHPStan\Node\ReturnStatement;
108109
use PHPStan\Node\StaticMethodCallableNode;
109110
use PHPStan\Node\UnreachableStatementNode;
111+
use PHPStan\Node\VariableAssignNode;
110112
use PHPStan\Node\VarTagChangedExpressionTypeNode;
111113
use PHPStan\Parser\ArrowFunctionArgVisitor;
112114
use PHPStan\Parser\ClosureArgVisitor;
@@ -1554,7 +1556,7 @@ public function leaveNode(Node $node): ?ExistingArrayDimFetch
15541556
$clonedVar,
15551557
new UnsetOffsetExpr($var->var, $var->dim),
15561558
static function (Node $node, Scope $scope) use ($nodeCallback): void {
1557-
if (!$node instanceof PropertyAssignNode) {
1559+
if (!$node instanceof PropertyAssignNode && !$node instanceof VariableAssignNode) {
15581560
return;
15591561
}
15601562

@@ -2845,7 +2847,7 @@ static function (): void {
28452847
$expr->var,
28462848
$newExpr,
28472849
static function (Node $node, Scope $scope) use ($nodeCallback): void {
2848-
if (!$node instanceof PropertyAssignNode) {
2850+
if (!$node instanceof PropertyAssignNode && !$node instanceof VariableAssignNode) {
28492851
return;
28502852
}
28512853

@@ -3891,6 +3893,7 @@ private function processArgs(
38913893
if ($assignByReference) {
38923894
$argValue = $arg->value;
38933895
if ($argValue instanceof Variable && is_string($argValue->name)) {
3896+
$nodeCallback(new VariableAssignNode($argValue, new TypeExpr($byRefType), false), $scope);
38943897
$scope = $scope->assignVariable($argValue->name, $byRefType, new MixedType());
38953898
} else {
38963899
$scope = $scope->invalidateExpression($argValue);
@@ -3987,6 +3990,7 @@ private function processAssignVar(
39873990
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
39883991
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
39893992

3993+
$nodeCallback(new VariableAssignNode($var, $assignedExpr, $isAssignOp), $result->getScope());
39903994
$scope = $result->getScope()->assignVariable($var->name, $type, $scope->getNativeType($assignedExpr));
39913995
foreach ($conditionalExpressions as $exprString => $holders) {
39923996
$scope = $scope->addConditionalExpressions($exprString, $holders);
@@ -4143,6 +4147,7 @@ private function processAssignVar(
41434147

41444148
if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) {
41454149
if ($var instanceof Variable && is_string($var->name)) {
4150+
$nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
41464151
$scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite);
41474152
} else {
41484153
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
@@ -4169,7 +4174,9 @@ private function processAssignVar(
41694174
}
41704175
}
41714176
} else {
4172-
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
4177+
if ($var instanceof Variable) {
4178+
$nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
4179+
} elseif ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
41734180
$nodeCallback(new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
41744181
if ($var instanceof PropertyFetch && $var->name instanceof Node\Identifier && !$isAssignOp) {
41754182
$scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString());
@@ -4385,6 +4392,7 @@ static function (): void {
43854392
}
43864393

43874394
if ($var instanceof Variable && is_string($var->name)) {
4395+
$nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);
43884396
$scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite);
43894397
} else {
43904398
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {

‎src/Node/VariableAssignNode.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use PhpParser\Node\Expr;
6+
use PhpParser\NodeAbstract;
7+
8+
class VariableAssignNode extends NodeAbstract implements VirtualNode
9+
{
10+
11+
public function __construct(
12+
private Expr\Variable $variable,
13+
private Expr $assignedExpr,
14+
private bool $assignOp,
15+
)
16+
{
17+
parent::__construct($variable->getAttributes());
18+
}
19+
20+
public function getVariable(): Expr\Variable
21+
{
22+
return $this->variable;
23+
}
24+
25+
public function getAssignedExpr(): Expr
26+
{
27+
return $this->assignedExpr;
28+
}
29+
30+
public function isAssignOp(): bool
31+
{
32+
return $this->assignOp;
33+
}
34+
35+
public function getType(): string
36+
{
37+
return 'PHPStan_Node_VariableAssignNodeNode';
38+
}
39+
40+
/**
41+
* @return string[]
42+
*/
43+
public function getSubNodeNames(): array
44+
{
45+
return [];
46+
}
47+
48+
}

‎src/Rules/Variables/ParameterOutAssignedTypeRule.php

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Node\VariableAssignNode;
8+
use PHPStan\Reflection\ExtendedMethodReflection;
9+
use PHPStan\Reflection\FunctionReflection;
10+
use PHPStan\Reflection\ParametersAcceptorSelector;
11+
use PHPStan\Rules\Rule;
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 is_string;
18+
use function sprintf;
19+
20+
/**
21+
* @implements Rule<VariableAssignNode>
22+
*/
23+
class ParameterOutAssignedTypeRule implements Rule
24+
{
25+
26+
public function __construct(
27+
private RuleLevelHelper $ruleLevelHelper,
28+
)
29+
{
30+
}
31+
32+
public function getNodeType(): string
33+
{
34+
return VariableAssignNode::class;
35+
}
36+
37+
public function processNode(Node $node, Scope $scope): array
38+
{
39+
$inFunction = $scope->getFunction();
40+
if ($inFunction === null) {
41+
return [];
42+
}
43+
44+
if ($scope->isInAnonymousFunction()) {
45+
return [];
46+
}
47+
48+
$variable = $node->getVariable();
49+
if (!is_string($variable->name)) {
50+
return [];
51+
}
52+
53+
$variant = ParametersAcceptorSelector::selectSingle($inFunction->getVariants());
54+
$parameters = $variant->getParameters();
55+
$foundParameter = null;
56+
foreach ($parameters as $parameter) {
57+
if (!$parameter->passedByReference()->createsNewVariable()) {
58+
continue;
59+
}
60+
if ($parameter->getName() !== $variable->name) {
61+
continue;
62+
}
63+
64+
$foundParameter = $parameter;
65+
break;
66+
}
67+
68+
if ($foundParameter === null) {
69+
return [];
70+
}
71+
72+
if ($foundParameter->getOutType() === null) {
73+
return [];
74+
}
75+
76+
$typeResult = $this->ruleLevelHelper->findTypeToCheck(
77+
$scope,
78+
$node->getAssignedExpr(),
79+
'',
80+
static fn (Type $type): bool => $foundParameter->getOutType()->isSuperTypeOf($type)->yes(),
81+
);
82+
$type = $typeResult->getType();
83+
if ($type instanceof ErrorType) {
84+
return $typeResult->getUnknownClassErrors();
85+
}
86+
87+
$assignedExprType = $scope->getType($node->getAssignedExpr());
88+
if ($foundParameter->getOutType()->isSuperTypeOf($assignedExprType)->yes()) {
89+
return [];
90+
}
91+
92+
if ($inFunction instanceof ExtendedMethodReflection) {
93+
$functionDescription = sprintf('method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName());
94+
} else {
95+
$functionDescription = sprintf('function %s()', $inFunction->getName());
96+
}
97+
98+
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($foundParameter->getOutType(), $assignedExprType);
99+
100+
return [
101+
RuleErrorBuilder::message(sprintf(
102+
'Parameter &$%s out type of %s expects %s, %s given.',
103+
$foundParameter->getName(),
104+
$functionDescription,
105+
$foundParameter->getOutType()->describe($verbosityLevel),
106+
$assignedExprType->describe($verbosityLevel),
107+
))->build(),
108+
];
109+
}
110+
111+
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PHPStan\Rules\Rule as TRule;
6+
use PHPStan\Rules\RuleLevelHelper;
7+
use PHPStan\Testing\RuleTestCase;
8+
9+
/**
10+
* @extends RuleTestCase<ParameterOutAssignedTypeRule>
11+
*/
12+
class ParameterOutAssignedTypeRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): TRule
16+
{
17+
return new ParameterOutAssignedTypeRule(
18+
new RuleLevelHelper($this->createReflectionProvider(), true, false, true, true, false, true, false),
19+
);
20+
}
21+
22+
public function testRule(): void
23+
{
24+
$this->analyse([__DIR__ . '/data/parameter-out-assigned-type.php'], [
25+
[
26+
'Parameter &$p out type of function ParameterOutAssignedType\foo() expects int, string given.',
27+
10,
28+
],
29+
[
30+
'Parameter &$p out type of method ParameterOutAssignedType\Foo::doFoo() expects int, string given.',
31+
21,
32+
],
33+
[
34+
'Parameter &$p out type of method ParameterOutAssignedType\Foo::doBar() expects string, int given.',
35+
29,
36+
],
37+
[
38+
'Parameter &$p out type of method ParameterOutAssignedType\Foo::doBaz() expects list<int>, array<0|int<2, max>, int> given.',
39+
38,
40+
],
41+
[
42+
'Parameter &$p out type of method ParameterOutAssignedType\Foo::doBaz2() expects list<int>, non-empty-list<\'str\'|int> given.',
43+
47,
44+
],
45+
[
46+
'Parameter &$p out type of method ParameterOutAssignedType\Foo::doBaz3() expects list<list<int>>, array<int<0, max>, array<int<0, max>, int>> given.',
47+
56,
48+
],
49+
]);
50+
}
51+
52+
}

‎tests/PHPStan/Rules/Variables/data/parameter-out-assigned-type.php

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
3+
namespace ParameterOutAssignedType;
4+
5+
/**
6+
* @param-out int $p
7+
*/
8+
function foo(&$p): void {
9+
$p = 1;
10+
$p = 'str';
11+
}
12+
13+
class Foo
14+
{
15+
16+
/**
17+
* @param-out int $p
18+
*/
19+
public function doFoo(&$p): void {
20+
$p = 1;
21+
$p = 'str';
22+
}
23+
24+
/**
25+
* @param-out string $p
26+
*/
27+
function doBar(&$p): void
28+
{
29+
$this->doFoo($p);
30+
}
31+
32+
/**
33+
* @param list<int> $p
34+
* @param-out list<int> $p
35+
*/
36+
function doBaz(&$p): void
37+
{
38+
unset($p[1]);
39+
}
40+
41+
/**
42+
* @param list<int> $p
43+
* @param-out list<int> $p
44+
*/
45+
function doBaz2(&$p): void
46+
{
47+
$p[] = 'str';
48+
}
49+
50+
/**
51+
* @param list<list<int>> $p
52+
* @param-out list<list<int>> $p
53+
*/
54+
function doBaz3(&$p): void
55+
{
56+
unset($p[1][2]);
57+
}
58+
59+
}

0 commit comments

Comments
 (0)