[TypedPropertyRector] Compile Error: Type of Child::$name must not be defined (as in class VendorParent)

[ruudk]

Subject	Details
Rector version	v0.6.3
PHP version	PHP 7.4
Full Command	vendor/bin/rector process --dry-run file.php

Problem

There is a problem with TypedPropertyRector. When Child extends VendorParent (package coming from Composer) and overwrites a property with a default value, Rector will try to apply the string property type.

When running the code, this fails:

 Compile Error: Type of Child::$name must not be defined (as in class VendorParent)

The reason is that the parent is not changed, so the parent has no type, but the child introduces a type. That's not allowed in PHP 7.4.

Failing test scenario:

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

<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

class VendorParent
{
    /**
     * @var string
     */
    protected $name;
}

final class Child extends VendorParent
{
    /**
     * @var string
     */
    protected $name = 'child';
}

?>
-----
<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

class VendorParent
{
    /**
     * @var string
     */
    protected $name;
}

final class Child extends VendorParent
{
    /**
     * @var string
     */
    protected $name = 'child';
}

?>

result:

Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
     /**
      * @var string
      */
-    protected $name;
+    protected string $name;
 }
 
 final class Child extends VendorParent
@@ @@
     /**
      * @var string
      */
-    protected $name = 'child';
+    protected string $name = 'child';
 }
 
 ?>

(it also tries to update the VendorParent, but that's because I didn't exclude this one. Should propably be done in the test somewhere, not sure where).

[TomasVotruba]

I see.

So the expecting behavior is not to update vendor-locked cases? = skip yours?

[ruudk]

Not sure if it's about "vendor" only, I think that the fixer should look at the parent. And only fix when the parent has that type. But that can be a race condition, what if the fixer first fixes the child and then the parent, it needs to come back to the child and fix that too.

Maybe it can look and see if the parent is not excluded (or vendored) and fixable? And then fix them together.

[TomasVotruba]

Indeed.
Same vendor lock logic is applied to param/return types, let's stick with that.

[ruudk]

Where do I find this? Maybe I can create a PR to fix this.

Any idea how I can simulate ParentVendor to be considered vendored in tests?

[TomasVotruba]

That would be great!

Look this service and it's use:
https://github.com/rectorphp/rector/blob/master/packages/TypeDeclaration/src/VendorLock/VendorLockResolver.ph
Is used in param and return, to prevent exactly these conflicts.

New method with same logic just for property has to be added there and uses in the property Rector

[ruudk]

I'm glancing at the VendorLockResolver but how does it know that something is vendored? I don't see anything related to that, no comments, and no tests. Any clue?

[TomasVotruba]

It does it on many levels. If you look here:

rector/packages/TypeDeclaration/src/VendorLock/VendorLockResolver.php

Lines 55 to 57 in 21bbcff

	$parentClassNode = $this->parsedNodesByType->findClass($parentClassName);
	if ($parentClassNode !== null) {
	$parentMethodNode = $parentClassNode->getMethod($methodName);

It checks the parent class/interfacce is parsed in src. If not, it's vendor locker, because only parsed classes can be changed.

Then it checks the parent class/interface has the method/property.

If not, it's not vendor locked, because it's only in current class.

Etc.

[ruudk]

Smart. How do I simulate this in a test? If I add the VendorParent class to the .inc.php it will be considered as parsed, right?

[TomasVotruba]

Yes, exactly. There is directory Source,where you put the parent class. It will not be parsed

[ruudk]

@TomasVotruba Yeey, it worked. #2509