Do not suggest typed property when defined in vendored parent (#2509)

Do not suggest typed property when defined in vendored parent

packages/Php74/src/Rector/Property/TypedPropertyRector.php

@@ -11,6 +11,7 @@ use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
 use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer;
+use Rector\TypeDeclaration\VendorLock\VendorLockResolver;
 use Rector\ValueObject\PhpVersionFeature;
 
 /**
@@ -25,9 +26,15 @@ final class TypedPropertyRector extends AbstractRector
      */
     private $propertyTypeInferer;
 
-    public function __construct(PropertyTypeInferer $propertyTypeInferer)
+    /**
+     * @var VendorLockResolver
+     */
+    private $vendorLockResolver;
+
+    public function __construct(PropertyTypeInferer $propertyTypeInferer, VendorLockResolver $vendorLockResolver)
     {
         $this->propertyTypeInferer = $propertyTypeInferer;
+        $this->vendorLockResolver = $vendorLockResolver;
     }
 
     public function getDefinition(): RectorDefinition
@@ -92,6 +99,10 @@ PHP
             return null;
         }
 
+        if ($this->vendorLockResolver->isPropertyChangeVendorLockedIn($node)) {
+            return null;
+        }
+
         $node->type = $propertyTypeNode;
 
         return $node;

packages/Php74/tests/Rector/Property/TypedPropertyRector/Fixture/parent_has_untyped_property.php.inc

@@ -0,0 +1,20 @@
+<?php
+
+namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;
+
+use Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Source\SomeParent;
+
+final class Child extends SomeParent
+{
+    /**
+     * @var string
+     */
+    protected $name = 'child';
+
+    /**
+     * @var string
+     */
+    protected string $typedName = 'child';
+}
+
+?>

packages/Php74/tests/Rector/Property/TypedPropertyRector/Fixture/parent_of_parent_has_untyped_property.php.inc

@@ -0,0 +1,24 @@
+<?php
+
+namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;
+
+use Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Source\SomeParent;
+
+abstract class Middle extends SomeParent
+{
+}
+
+final class Child2 extends Middle
+{
+    /**
+     * @var string
+     */
+    protected $name = 'child';
+
+    /**
+     * @var string
+     */
+    protected string $typedName = 'child';
+}
+
+?>

packages/Php74/tests/Rector/Property/TypedPropertyRector/Source/SomeParent.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Source;
+
+abstract class SomeParent
+{
+    /**
+     * @var string
+     */
+    protected $name;
+
+    /**
+     * @var string
+     */
+    protected string $typedName;
+}

packages/Php74/tests/Rector/Property/TypedPropertyRector/TypedPropertyRectorTest.php

@@ -11,6 +11,7 @@ use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 final class TypedPropertyRectorTest extends AbstractRectorTestCase
 {
     /**
+     * @requires PHP >= 7.4
      * @dataProvider provideDataForTest()
      */
     public function test(string $file): void

packages/TypeDeclaration/src/VendorLock/VendorLockResolver.php

@@ -4,9 +4,13 @@ declare(strict_types=1);
 
 namespace Rector\TypeDeclaration\VendorLock;
 
+use PhpParser\Node;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Interface_;
+use PhpParser\Node\Stmt\Property;
+use PhpParser\Node\Stmt\PropertyProperty;
 use Rector\NodeContainer\ParsedNodesByType;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PhpParser\Node\Manipulator\ClassManipulator;
@@ -41,7 +45,12 @@ final class VendorLockResolver
 
     public function isParameterChangeVendorLockedIn(ClassMethod $classMethod, int $paramPosition): bool
     {
-        if (! $this->hasParentClassOrImplementsInterface($classMethod)) {
+        $classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
+        if ($classNode === null) {
+            return false;
+        }
+
+        if (! $this->hasParentClassOrImplementsInterface($classNode)) {
             return false;
         }
 
@@ -102,7 +111,12 @@ final class VendorLockResolver
 
     public function isReturnChangeVendorLockedIn(ClassMethod $classMethod): bool
     {
-        if (! $this->hasParentClassOrImplementsInterface($classMethod)) {
+        $classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
+        if ($classNode === null) {
+            return false;
+        }
+
+        if (! $this->hasParentClassOrImplementsInterface($classNode)) {
             return false;
         }
 
@@ -160,13 +174,66 @@ final class VendorLockResolver
         return false;
     }
 
-    private function hasParentClassOrImplementsInterface(ClassMethod $classMethod): bool
+    public function isPropertyChangeVendorLockedIn(Property $property): bool
     {
-        $classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
+        $classNode = $property->getAttribute(AttributeKey::CLASS_NODE);
         if ($classNode === null) {
             return false;
         }
 
+        if (! $this->hasParentClassOrImplementsInterface($classNode)) {
+            return false;
+        }
+
+        $propertyName = $this->nameResolver->getName($property);
+
+        // @todo extract to some "inherited parent method" service
+        /** @var string|null $parentClassName */
+        $parentClassName = $classNode->getAttribute(AttributeKey::PARENT_CLASS_NAME);
+
+        if ($parentClassName !== null) {
+            $parentClassNode = $this->parsedNodesByType->findClass($parentClassName);
+            if ($parentClassNode !== null) {
+                $parentPropertyNode = $this->getProperty($parentClassNode, $propertyName);
+                // @todo validate type is conflicting
+                // parent class property in local scope → it's ok
+                if ($parentPropertyNode !== null) {
+                    return $parentPropertyNode->type !== null;
+                }
+
+                // if not, look for it's parent parent - @todo recursion
+            }
+
+            if (property_exists($parentClassName, $propertyName)) {
+                // @todo validate type is conflicting
+                // parent class property in external scope → it's not ok
+                return true;
+
+                // if not, look for it's parent parent - @todo recursion
+            }
+        }
+
+        return false;
+    }
+
+    // Until we have getProperty (https://github.com/nikic/PHP-Parser/pull/646)
+    private function getProperty(ClassLike $classLike, string $name)
+    {
+        $lowerName = strtolower($name);
+        foreach ($classLike->stmts as $stmt) {
+            if ($stmt instanceof Property) {
+                foreach ($stmt->props as $prop) {
+                    if ($prop instanceof PropertyProperty && $lowerName === $prop->name->toLowerString()) {
+                        return $stmt;
+                    }
+                }
+            }
+        }
+        return null;
+    }
+
+    private function hasParentClassOrImplementsInterface(Node $classNode): bool
+    {
         if ($classNode instanceof Class_ || $classNode instanceof Interface_) {
             if ($classNode->extends) {
                 return true;