[Core] Use PropertyFetchAnalyzer::isFilledViaMethodCallInConstructStm…

…ts() in ConstructorAssignDetector::isPropertyAssigned() (#2351) * [Core] Use PropertyFetchAnalyzer::isFilledViaMethodCallInConstructStmts() in ConstructorAssignDetector::isPropertyAssigned() * more * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify * add test for self static * more tests * phpstan * [ci-review] Rector Rectify * StaticPropertyFetch detection fix * final touch: clean up: donot create object when not needed * final touch: clean up * final touch: clean up * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify Co-authored-by: GitHub Action <action@github.com>
rectorphp · May 23, 2022 · 1691157 · 1691157
1 parent f134b61
commit 1691157
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 56 deletions.
diff --git a/...ountOnNullRector/Fixture/skip_property_array_filled_by_construct_via_method_call2.php.inc b/...ountOnNullRector/Fixture/skip_property_array_filled_by_construct_via_method_call2.php.inc
@@ -0,0 +1,24 @@
+<?php
+
+namespace Rector\Tests\Php71\Rector\FuncCall\CountOnNullRector\Fixture;
+
+final class SkipPropertyArrayFilledByConstructViaMethodCall2
+{
+    /** @var array */
+    private static $property;
+
+    public function __construct()
+    {
+        self::fill();
+    }
+
+    private static function fill()
+    {
+        self::$property = [];
+    }
+
+    public static function run(): int
+    {
+        return count(self::$property);
+    }
+}
diff --git a/...ountOnNullRector/Fixture/skip_property_array_filled_by_construct_via_method_call3.php.inc b/...ountOnNullRector/Fixture/skip_property_array_filled_by_construct_via_method_call3.php.inc
@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\Tests\Php71\Rector\FuncCall\CountOnNullRector\Fixture;
+
+use Rector\Tests\Php71\Rector\FuncCall\CountOnNullRector\Source\ParentFiller;
+
+final class SkipPropertyArrayFilledByConstructViaMethodCall3 extends ParentFiller
+{
+    /** @var array */
+    public static $property;
+
+    public function __construct()
+    {
+        parent::fill();
+    }
+
+    public static function run(): int
+    {
+        return count(static::$property);
+    }
+}
diff --git a/rules-tests/Php71/Rector/FuncCall/CountOnNullRector/Source/ParentFiller.php b/rules-tests/Php71/Rector/FuncCall/CountOnNullRector/Source/ParentFiller.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php71\Rector\FuncCall\CountOnNullRector\Source;
+
+class ParentFiller
+{
+    public static function fill()
+    {
+        static::$property = [];
+    }
+}
diff --git a/rules/Php71/NodeAnalyzer/CountableAnalyzer.php b/rules/Php71/NodeAnalyzer/CountableAnalyzer.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\ClassLike;
 use PHPStan\Analyser\Scope;
@@ -32,23 +33,26 @@ public function __construct(
         private readonly NodeTypeResolver $nodeTypeResolver,
         private readonly NodeNameResolver $nodeNameResolver,
         private readonly ReflectionProvider $reflectionProvider,
-        private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer,
         private readonly BetterNodeFinder $betterNodeFinder,
+        private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer,
         private readonly ConstructorAssignDetector $constructorAssignDetector
     ) {
     }
 
     public function isCastableArrayType(Expr $expr, ArrayType $arrayType): bool
     {
-        if (! $expr instanceof PropertyFetch) {
+        if (! $this->propertyFetchAnalyzer->isPropertyFetch($expr)) {
             return false;
         }
 
         if ($arrayType instanceof ConstantArrayType) {
             return false;
         }
 
-        $callerObjectType = $this->nodeTypeResolver->getType($expr->var);
+        /** @var StaticPropertyFetch|PropertyFetch $expr */
+        $callerObjectType = $expr instanceof StaticPropertyFetch
+            ? $this->nodeTypeResolver->getType($expr->class)
+            : $this->nodeTypeResolver->getType($expr->var);
 
         $propertyName = $this->nodeNameResolver->getName($expr->name);
         if (! is_string($propertyName)) {
@@ -88,10 +92,6 @@ public function isCastableArrayType(Expr $expr, ArrayType $arrayType): bool
             return false;
         }
 
-        if ($this->propertyFetchAnalyzer->isFilledViaMethodCallInConstructStmts($expr)) {
-            return false;
-        }
-
         $propertyDefaultValue = $propertiesDefaults[$propertyName];
         return $propertyDefaultValue === null;
     }
@@ -105,8 +105,10 @@ private function isCallerObjectClassNameStmtOrArray(TypeWithClassName $typeWithC
         return is_a($typeWithClassName->getClassName(), Array_::class, true);
     }
 
-    private function isIterableOrFilledAtConstruct(Type $nativeType, PropertyFetch $propertyFetch): bool
-    {
+    private function isIterableOrFilledAtConstruct(
+        Type $nativeType,
+        StaticPropertyFetch|PropertyFetch $propertyFetch
+    ): bool {
         if ($nativeType->isIterable()->yes()) {
             return true;
         }
@@ -125,7 +127,7 @@ private function isIterableOrFilledAtConstruct(Type $nativeType, PropertyFetch $
     }
 
     private function resolveProperty(
-        PropertyFetch $propertyFetch,
+        StaticPropertyFetch|PropertyFetch $propertyFetch,
         ClassReflection $classReflection,
         string $propertyName
     ): ?PropertyReflection {

diff --git a/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php b/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php
@@ -12,6 +12,7 @@
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\NodeTraverser;
 use PHPStan\Type\ObjectType;
+use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
 use Rector\Core\ValueObject\MethodName;
 use Rector\NodeTypeResolver\NodeTypeResolver;
 use Rector\TypeDeclaration\Matcher\PropertyAssignMatcher;
@@ -29,7 +30,8 @@ public function __construct(
         private readonly NodeTypeResolver $nodeTypeResolver,
         private readonly PropertyAssignMatcher $propertyAssignMatcher,
         private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
-        private readonly AutowiredClassMethodOrPropertyAnalyzer $autowiredClassMethodOrPropertyAnalyzer
+        private readonly AutowiredClassMethodOrPropertyAnalyzer $autowiredClassMethodOrPropertyAnalyzer,
+        private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
     ) {
     }
 
@@ -68,6 +70,10 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName):
             });
         }
 
+        if (! $isAssignedInConstructor) {
+            return $this->propertyFetchAnalyzer->isFilledViaMethodCallInConstructStmts($classLike, $propertyName);
+        }
+
         return $isAssignedInConstructor;
     }
 

diff --git a/src/NodeAnalyzer/PropertyFetchAnalyzer.php b/src/NodeAnalyzer/PropertyFetchAnalyzer.php
@@ -8,15 +8,18 @@
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name;
-use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\Trait_;
+use PHPStan\Type\ObjectType;
 use Rector\Core\Enum\ObjectReference;
 use Rector\Core\PhpParser\AstResolver;
-use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Core\ValueObject\MethodName;
 use Rector\NodeNameResolver\NodeNameResolver;
@@ -31,7 +34,6 @@ final class PropertyFetchAnalyzer
     public function __construct(
         private readonly NodeNameResolver $nodeNameResolver,
         private readonly BetterNodeFinder $betterNodeFinder,
-        private readonly NodeComparator $nodeComparator,
         private readonly AstResolver $astResolver
     ) {
     }
@@ -51,7 +53,10 @@ public function isLocalPropertyFetch(Node $node): bool
                 return false;
             }
 
-            return $this->nodeNameResolver->isName($node->class, ObjectReference::SELF()->getValue());
+            return $this->nodeNameResolver->isNames($node->class, [
+                ObjectReference::SELF()->getValue(),
+                ObjectReference::STATIC()->getValue(),
+            ]);
         }
 
         return false;
@@ -134,44 +139,46 @@ public function isVariableAssignToThisPropertyFetch(Node $node, string $variable
         return $this->isLocalPropertyFetch($node->var);
     }
 
-    public function isFilledViaMethodCallInConstructStmts(PropertyFetch $propertyFetch): bool
+    public function isFilledViaMethodCallInConstructStmts(ClassLike $classLike, string $propertyName): bool
     {
-        $class = $this->betterNodeFinder->findParentType($propertyFetch, Class_::class);
-        if (! $class instanceof Class_) {
-            return false;
-        }
-
-        $construct = $class->getMethod(MethodName::CONSTRUCT);
-        if (! $construct instanceof ClassMethod) {
+        $classMethod = $classLike->getMethod(MethodName::CONSTRUCT);
+        if (! $classMethod instanceof ClassMethod) {
             return false;
         }
 
-        /** @var MethodCall[] $methodCalls */
-        $methodCalls = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped(
-            $construct,
-            [MethodCall::class]
-        );
+        $className = (string) $this->nodeNameResolver->getName($classLike);
+        $stmts = (array) $classMethod->stmts;
 
-        foreach ($methodCalls as $methodCall) {
-            if (! $methodCall->var instanceof Variable) {
+        foreach ($stmts as $stmt) {
+            if (! $stmt instanceof Expression) {
                 continue;
             }
 
-            if (! $this->nodeNameResolver->isName($methodCall->var, self::THIS)) {
+            if (! $stmt->expr instanceof MethodCall && ! $stmt->expr instanceof StaticCall) {
                 continue;
             }
 
-            $classMethod = $this->astResolver->resolveClassMethodFromMethodCall($methodCall);
-            if (! $classMethod instanceof ClassMethod) {
+            $callerClassMethod = $this->astResolver->resolveClassMethodFromCall($stmt->expr);
+            if (! $callerClassMethod instanceof ClassMethod) {
                 continue;
             }
 
-            $isFound = $this->isPropertyAssignFoundInClassMethod($classMethod, $propertyFetch);
-            if (! $isFound) {
+            $callerClass = $this->betterNodeFinder->findParentType($callerClassMethod, Class_::class);
+            if (! $callerClass instanceof Class_) {
                 continue;
             }
 
-            return true;
+            $callerClassName = (string) $this->nodeNameResolver->getName($callerClass);
+            $isFound = $this->isPropertyAssignFoundInClassMethod(
+                $classLike,
+                $className,
+                $callerClassName,
+                $callerClassMethod,
+                $propertyName
+            );
+            if ($isFound) {
+                return true;
+            }
         }
 
         return false;
@@ -190,21 +197,36 @@ public function isLocalPropertyOfNames(Node $node, array $propertyNames): bool
         return $this->nodeNameResolver->isNames($node->name, $propertyNames);
     }
 
-    private function isPropertyAssignFoundInClassMethod(ClassMethod $classMethod, PropertyFetch $propertyFetch): bool
-    {
-        return (bool) $this->betterNodeFinder->findFirstInFunctionLikeScoped(
-            $classMethod,
-            function (Node $subNode) use ($propertyFetch): bool {
-                if (! $subNode instanceof Assign) {
-                    return false;
-                }
-
-                if (! $subNode->var instanceof PropertyFetch) {
-                    return false;
-                }
-
-                return $this->nodeComparator->areNodesEqual($propertyFetch, $subNode->var);
+    private function isPropertyAssignFoundInClassMethod(
+        ClassLike $classLike,
+        string $className,
+        string $callerClassName,
+        ClassMethod $classMethod,
+        string $propertyName
+    ): bool {
+        if ($className !== $callerClassName && ! $classLike instanceof Trait_) {
+            $objectType = new ObjectType($className);
+            $callerObjectType = new ObjectType($callerClassName);
+
+            if (! $callerObjectType->isSuperTypeOf($objectType)->yes()) {
+                return false;
             }
-        );
+        }
+
+        foreach ((array) $classMethod->stmts as $stmt) {
+            if (! $stmt instanceof Expression) {
+                continue;
+            }
+
+            if (! $stmt->expr instanceof Assign) {
+                continue;
+            }
+
+            if ($this->isLocalPropertyFetchName($stmt->expr->var, $propertyName)) {
+                return true;
+            }
+        }
+
+        return false;
     }
 }
diff --git a/src/PhpParser/AstResolver.php b/src/PhpParser/AstResolver.php
@@ -183,11 +183,6 @@ public function resolveClassMethod(string $className, string $methodName): ?Clas
         return $classMethod;
     }
 
-    public function resolveClassMethodFromMethodCall(MethodCall $methodCall): ?ClassMethod
-    {
-        return $this->resolveClassMethodFromCall($methodCall);
-    }
-
     public function resolveClassMethodFromCall(MethodCall | StaticCall $call): ?ClassMethod
     {
         if ($call instanceof MethodCall) {