rectorphp / rector

Instant Upgrades and Automated Refactoring of any PHP 5.3+ code
https://getrector.com
MIT License
8.61k stars 680 forks source link

Process dependent files when running with the cache #8801

Closed carlos-granados closed 2 weeks ago

carlos-granados commented 3 weeks ago

Feature Request

Currently, if we are using the cache, Rector will only process the files that have changed or which are not cached. But there may be dependent files of these files which Rector will not analyse and Rector might have found changes for these files if it analysed them. Rector should analyse any dependent files of any non cached file for completeness, to make sure it has handled all possible changes.

One example, suppose we have these two classes:

//DependentClass.php

class DependentClass
{
    public function callFunction()
    {
        $dependencyClass = new DependencyClass();
        return $dependencyClass->calledFunction(1);
    }
}

and

//DependencyClass.php

class DependencyClass
{
    public function calledFunction(int $a)
    {
        return $a;
    }
}

If we run Rector with the NumericReturnTypeFromStrictReturnsRector rule it will add this change to the second file:

-    public function calledFunction(int $a)
+    public function calledFunction(int $a): int

If we run rector again using the cache, it will not find any more changes to apply because it will only process the DependencyClass.php file. But if we had run Rector without the cache it would have found a new change for the DependentClass.php file:

-    public function callFunction()
+    public function callFunction(): int

If Rector had processed the dependent classes for any class that is not cached, this second change would have been detected and Rector would have been able to process a complete set of changes

This is currently causing issues in our setup. We run Rector locally using the cache and we run it until no more changes are applied. Then in CI Rector is run without a cache and it may find changes that have not been incorporated, forcing us to modify the pull request and re-run all the tests again, which is expensive in time and resources

I think I have an idea of how this could be implemented using PHPStan's DependencyResolver, if you agree this is a good feature, I can give it a go

staabm commented 3 weeks ago

The cache itself is a rather complicated topic.

If possible I think it would be best to re-use as much from PHPStan as possible instead of re-implementing

TomasVotruba commented 2 weeks ago

Closing per https://github.com/rectorphp/rector-src/pull/6256#issuecomment-2312160243

Thanks for understanding :pray: