FinalizePublicClassConstantRector changes Class that has children

[keulinho]

Bug Report

Subject	Details
Rector version	v0.15.17

FinalizePublicClassConstantRector may change a const to final even if it is overwritten in a child class.

Minimal PHP Code Causing Issue

In our project we have one class:

class CustomerRegisterEvent
{
    public const EVENT_NAME = 'checkout.customer.register';
}

and a second class that extends from it:

class GuestCustomerRegisterEvent extends CustomerRegisterEvent
{
    public const EVENT_NAME = 'checkout.customer.guest_register';
}

When running rector with the FinalizePublicClassConstantRector rector it changes both const to final which is obviously wrong in for the first class.

Expected Behaviour

Rector should not change the first "parent" class, only the const on the child class should be made final.

My Findings so far

It seems not to be a general issue, as the FinalizePublicClassConstantRector checks whether the class has children or not, but it seems that there may be cases where that check does not work correctly. I debugged our case a little and found that in \Rector\FamilyTree\Reflection\FamilyRelationsAnalyzer::getChildrenOfClassReflection() the $classReflections variable does not contain our GuestCustomerRegisterEvent child class when we analyze the CustomerRegisterEvent parent class.

I suspect some Phpstan internals going on here as the \PHPStan\Reflection\ReflectionProvider is used here and more specifically the $classReflections are obtained from the internal $classes property of the \PHPStan\Reflection\ReflectionProvider\MemoizingReflectionProvider. I suspect that in our case the child class was not yet analysed by phpstan and because of that is missing from that list when rector tries to fix the CustomerRegisterEvent parent case.

[samsonasik]

this condition can happen due to autoload overlap, you can read the documention how to troubleshot it, all there :)

https://getrector.org/documentation/static-reflection-and-autoload

[keulinho]

thanks for the fast response, indeed i was not aware of that page in the docs, sadly the composer dump-autoload -o trick did not help, so i'm now manually skipping that rector for that specific files.

Feels not quite right, but it works.

[holtkamp]

this condition can happen due to autoload overlap, you can read the documention how to troubleshot it, all there :)

https://getrector.org/documentation/static-reflection-and-autoload

Encountered the same issue and found out it is reported quite some times: https://github.com/rectorphp/rector/issues/8336, https://github.com/rectorphp/rector/issues/7920, https://github.com/rectorphp/rector/issues/7792, https://github.com/rectorphp/rector/issues/7678, https://github.com/rectorphp/rector/issues/7602

The documentation https://getrector.org/documentation/static-reflection-and-autoload describes the problem and a workaround (skip the files that cause a false positive). But the cause of this problem is not explained. This comment contains some useful insights: https://github.com/rectorphp/rector/issues/7602#issuecomment-1315381484

I agree with:

1. https://github.com/rectorphp/rector/issues/7920#issuecomment-1536243791 that a warning should be added to the Rector Rules that have this risk, and/or:
2. https://github.com/rectorphp/rector/issues/7920#issuecomment-1780935445 that this is a bug and maybe the issue should be kept open to prevent people (like me) considering opening a new issue over and over?