[Complexity] Skip more specific type via docblock @var on NoJustPropertyAssignRule (#179)

* [Complexity] Skip more specific type via docblock @var on NoJustPropertyAssignRule

* cs fix

* phpstan fix

* final touch: add fixture for equal type

* final touch: instanceof check

Author: samsonasik

src/Rules/Complexity/NoJustPropertyAssignRule.php

@@ -2,53 +2,111 @@
 
 namespace Symplify\PHPStanRules\Rules\Complexity;
 
+use PhpParser\Comment\Doc;
 use PhpParser\Node;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Stmt\Expression;
 use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ClassReflection;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleError;
 use PHPStan\Rules\RuleErrorBuilder;
 use Symfony\Component\Form\AbstractType;
 use Symplify\PHPStanRules\Enum\RuleIdentifier;
+use Symplify\PHPStanRules\PhpDoc\PhpDocResolver;
 
 /**
  * @see \Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\NoJustPropertyAssignRuleTest
  *
- * @implements Rule<Assign>
+ * @implements Rule<Expression>
  */
-final class NoJustPropertyAssignRule implements Rule
+final readonly class NoJustPropertyAssignRule implements Rule
 {
     /**
      * @var string
      */
     public const ERROR_MESSAGE = 'Instead of assigning service property to a variable, use the property directly';
 
+    public function __construct(
+        private PhpDocResolver $phpDocResolver
+    ) {
+
+    }
+
     public function getNodeType(): string
     {
-        return Assign::class;
+        return Expression::class;
     }
 
     /**
-     * @param Assign $node
+     * @param Expression $node
      * @return RuleError[]
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (! $this->isLocalPropertyFetchAssignToVariable($node, $scope)) {
+        $expr = $node->expr;
+
+        if (! $expr instanceof Assign) {
+            return [];
+        }
+
+        if (! $this->isLocalPropertyFetchAssignToVariable($expr, $scope)) {
             return [];
         }
 
         if ($this->shouldSkipCurrentClass($scope)) {
             return [];
         }
 
+        if ($this->shoulSkipMoreSpecificTypeByDocblock($scope, $node, $expr)) {
+            return [];
+        }
+
         return [RuleErrorBuilder::message(self::ERROR_MESSAGE)
             ->identifier(RuleIdentifier::NO_JUST_PROPERTY_ASSIGN)
             ->build()];
     }
 
+    private function shoulSkipMoreSpecificTypeByDocblock(Scope $scope, Expression $expression, Assign $assign): bool
+    {
+        $docComment = $expression->getDocComment();
+        if (! $docComment instanceof Doc) {
+            return false;
+        }
+
+        /** @var Variable $variable */
+        $variable = $assign->var;
+        $varName = $variable->name;
+
+        if ($varName instanceof Expr) {
+            return false;
+        }
+
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return false;
+        }
+
+        $resolvedPhpDocBlock = $this->phpDocResolver->resolve($scope, $classReflection, $docComment);
+        $exprType = $scope->getType($assign->expr);
+
+        foreach ($resolvedPhpDocBlock->getVarTags() as $key => $varTag) {
+            if ($key !== $varName) {
+                continue;
+            }
+
+            // different type means more specific type on purpose
+            if (! $varTag->getType()->equals($exprType)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private function isLocalPropertyFetchAssignToVariable(Assign $assign, Scope $scope): bool
     {
         if (! $assign->var instanceof Variable) {

tests/Rules/Complexity/NoJustPropertyAssignRule/Fixture/SkipSpecificVarDoc.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\Fixture;
+
+use ArrayIterator;
+use Iterator;
+
+final class SkipSpecificVarDoc
+{
+    public Iterator $it;
+
+    public function __construct()
+    {
+        $this->it = new ArrayIterator([]);
+    }
+
+    public function run()
+    {
+        /**
+         * @var ArrayIterator $it
+         */
+        $it = $this->it;
+    }
+}

tests/Rules/Complexity/NoJustPropertyAssignRule/Fixture/WithEqualTypeVarDoc.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Complexity\NoJustPropertyAssignRule\Fixture;
+
+use Iterator;
+
+final class WithEqualTypeVarDoc
+{
+    public Iterator $it;
+
+    public function run()
+    {
+        /**
+         * @var Iterator $it
+         */
+        $it = $this->it;
+    }
+}

tests/Rules/Complexity/NoJustPropertyAssignRule/NoJustPropertyAssignRuleTest.php

@@ -26,12 +26,24 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/ServiceAssign.php', [
             [NoJustPropertyAssignRule::ERROR_MESSAGE, 20],
         ]];
+        yield [__DIR__ . '/Fixture/WithEqualTypeVarDoc.php', [
+            [NoJustPropertyAssignRule::ERROR_MESSAGE, 18],
+        ]];
 
         yield [__DIR__ . '/Fixture/SkipScalarAssign.php', []];
+        yield [__DIR__ . '/Fixture/SkipSpecificVarDoc.php', []];
+    }
+
+    /**
+     * @return string[]
+     */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [__DIR__ . '/config/configured_rule.neon'];
     }
 
     protected function getRule(): Rule
     {
-        return new NoJustPropertyAssignRule();
+        return self::getContainer()->getByType(NoJustPropertyAssignRule::class);
     }
 }

tests/Rules/Complexity/NoJustPropertyAssignRule/config/configured_rule.neon

@@ -0,0 +1,5 @@
+includes:
+    - ../../../../config/included_services.neon
+
+rules:
+    - Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule