Skip to content

Commit 9c04efa

Browse files
committed
[Complexity] Skip more specific type via docblock @var on NoJustPropertyAssignRule
1 parent c7ed282 commit 9c04efa

File tree

4 files changed

+96
-5
lines changed

4 files changed

+96
-5
lines changed

src/Rules/Complexity/NoJustPropertyAssignRule.php

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,24 @@
22

33
namespace Symplify\PHPStanRules\Rules\Complexity;
44

5+
use PhpParser\Comment\Doc;
56
use PhpParser\Node;
67
use PhpParser\Node\Expr\Assign;
78
use PhpParser\Node\Expr\PropertyFetch;
89
use PhpParser\Node\Expr\Variable;
10+
use PhpParser\Node\Stmt\Expression;
911
use PHPStan\Analyser\Scope;
1012
use PHPStan\Rules\Rule;
1113
use PHPStan\Rules\RuleError;
1214
use PHPStan\Rules\RuleErrorBuilder;
1315
use Symfony\Component\Form\AbstractType;
1416
use Symplify\PHPStanRules\Enum\RuleIdentifier;
17+
use Symplify\PHPStanRules\PhpDoc\PhpDocResolver;
1518

1619
/**
1720
* @see \Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\NoJustPropertyAssignRuleTest
1821
*
19-
* @implements Rule<Assign>
22+
* @implements Rule<Expression>
2023
*/
2124
final class NoJustPropertyAssignRule implements Rule
2225
{
@@ -25,30 +28,78 @@ final class NoJustPropertyAssignRule implements Rule
2528
*/
2629
public const ERROR_MESSAGE = 'Instead of assigning service property to a variable, use the property directly';
2730

31+
public function __construct(
32+
private readonly PhpDocResolver $phpDocResolver
33+
) {
34+
35+
}
36+
2837
public function getNodeType(): string
2938
{
30-
return Assign::class;
39+
return Expression::class;
3140
}
3241

3342
/**
34-
* @param Assign $node
43+
* @param Expression $node
3544
* @return RuleError[]
3645
*/
3746
public function processNode(Node $node, Scope $scope): array
3847
{
39-
if (! $this->isLocalPropertyFetchAssignToVariable($node, $scope)) {
48+
$expr = $node->expr;
49+
50+
if (! $expr instanceof Assign) {
51+
return [];
52+
}
53+
54+
if (! $this->isLocalPropertyFetchAssignToVariable($expr, $scope)) {
4055
return [];
4156
}
4257

4358
if ($this->shouldSkipCurrentClass($scope)) {
4459
return [];
4560
}
4661

62+
if ($this->shoulSkipMoreSpecificTypeByDocblock($scope, $node, $node->expr)) {
63+
return [];
64+
}
65+
4766
return [RuleErrorBuilder::message(self::ERROR_MESSAGE)
4867
->identifier(RuleIdentifier::NO_JUST_PROPERTY_ASSIGN)
4968
->build()];
5069
}
5170

71+
private function shoulSkipMoreSpecificTypeByDocblock(Scope $scope, Expression $node, Assign $assign): bool
72+
{
73+
$docComment = $node->getDocComment();
74+
if (! $docComment instanceof Doc) {
75+
return false;
76+
}
77+
78+
/** @var Variable $variable */
79+
$variable = $assign->var;
80+
$varName = $variable->name;
81+
82+
if ($varName === null) {
83+
return false;
84+
}
85+
86+
$r = $this->phpDocResolver->resolve($scope, $scope->getClassReflection(), $docComment);
87+
$exprType = $scope->getType($assign->expr);
88+
89+
foreach ($r->getVarTags() as $key => $varTag) {
90+
if ($key !== $varName) {
91+
continue;
92+
}
93+
94+
// different type means more specific type on purpose
95+
if (! $varTag->getType()->equals($exprType)) {
96+
return true;
97+
}
98+
}
99+
100+
return false;
101+
}
102+
52103
private function isLocalPropertyFetchAssignToVariable(Assign $assign, Scope $scope): bool
53104
{
54105
if (! $assign->var instanceof Variable) {
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\Fixture;
6+
7+
use ArrayIterator;
8+
use Iterator;
9+
10+
final class SkipSpecificVarDoc
11+
{
12+
public Iterator $it;
13+
14+
public function __construct()
15+
{
16+
$this->it = new ArrayIterator([]);
17+
}
18+
19+
public function run()
20+
{
21+
/**
22+
* @var ArrayIterator $it
23+
*/
24+
$it = $this->it;
25+
}
26+
}

tests/Rules/Complexity/NoJustPropertyAssignRule/NoJustPropertyAssignRuleTest.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,19 @@ public static function provideData(): Iterator
2828
]];
2929

3030
yield [__DIR__ . '/Fixture/SkipScalarAssign.php', []];
31+
yield [__DIR__ . '/Fixture/SkipSpecificVarDoc.php', []];
32+
}
33+
34+
/**
35+
* @return string[]
36+
*/
37+
public static function getAdditionalConfigFiles(): array
38+
{
39+
return [__DIR__ . '/config/configured_rule.neon'];
3140
}
3241

3342
protected function getRule(): Rule
3443
{
35-
return new NoJustPropertyAssignRule();
44+
return self::getContainer()->getByType(NoJustPropertyAssignRule::class);
3645
}
3746
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
includes:
2+
- ../../../../config/included_services.neon
3+
4+
rules:
5+
- Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule

0 commit comments

Comments
 (0)