[Caching] Ensure cache cleared when rector.php changed on FileHashComputer::compute()

[samsonasik]

@TomasVotruba @staabm I found that when rector.php changed, the cache doesn't cleared and requires --clear-cache to apply change when add/remove rule from the list directly (one by one).

$rectorConfig->rules([
       // add new rule into the list...
]);

SimpleParameterProvider::hash() seems only cover when parameters (paths, skip, bootstrap) changed, not when rule added/removed from the list.

This patch ensure cache cleared when rector.php changed by read the rector.php content to be part of content passed to sha1().

[samsonasik]

All checks have passed 🎉 @TomasVotruba I think it is ready.

[samsonasik]

another solution is by using sha1_file() instead:

Suggested change

	return sha1(FileSystem::read($filePath) . $parametersHash);
	return sha1_file($filePath);

when rector.php changed, even only space, the hash also changed:

➜  rector-src git:(ensure-always-refresh) php -r 'echo sha1_file(__DIR__ . "/rector.php");';
71902587bce991de196cf2c1f79f1083e3834bbf%      

// change the file ...
// re-run
                                                                                                                                 
➜  rector-src git:(ensure-always-refresh) php -r 'echo sha1_file(__DIR__ . "/rector.php");';
59b7d5b0e9d9672186774efb691a9ccc8dd5ebac% 

[samsonasik]

@TomasVotruba @staabm what do you think?

[samsonasik]

also, sets is not part of $parametersHash, so only rely on $parametersHash, change $rectorConfig->sets() content won't change the cached files, so definitely requires 2 options:

• read the file + parameter hash like current proposed solution
• use sha1_file() instead.

on possible improvement: when using sets, I think the existing rules inside sets need to be extracted + hashed, then compare so when new rule added into set, the cache automatically cleared, but that may need a different PR :)

[staabm]

sha1_file() instead

👍

---

What comes to mind: should we also support auto cache clear when contents of imported configs change?

---

Is the rector version itself also a criteria to invalidate the cache?

[samsonasik]

as I commented previously, for imported config changed, eg: via sets() method, that should be in separate PR.

Rector version changed should be also in separate PR to ease review and revert.

[samsonasik]

I updated to use sha1_file() 7a3b7b3

[staabm]

A new test would be awesome

[samsonasik]

this method no longer needed 👍

[samsonasik]

@staabm I've added unit test for it 71b7c5a

[samsonasik]

All checks have passed 🎉 @TomasVotruba I think it is ready 👍

[samsonasik]

@harikt by this PR, your old issue should finally resolved :)

• Rector saving cache some where  rector#6996

[samsonasik]

It possibly already resolved at PR:.

• Fix Rector invalidation in case of changes sets or rules #4902

I am closing for now, I will check when back to PC.

Next possible improvement is include Rector version in hash string so it will auto-clear after composer up got new version.

[staabm]

At least the testcase should be merged

[samsonasik]

@TomasVotruba I just tried your change on PR:

• Fix Rector invalidation in case of changes sets or rules #4902

and unit test got failure:

There was 1 failure:

1) Rector\Tests\Caching\Config\FileHashComputer\FileHashComputerTest::testRectorPhpChanged
Failed asserting that two strings are not identical.

/Users/samsonasik/www/rector-src/packages-tests/Caching/Config/FileHashComputer/FileHashComputerTest.php:28

@TomasVotruba it seems $rectorConfig->rules() changed doesn't change the hash.

[samsonasik]

@staabm here the failing test case PR:

• Add test case for caching $rectorConfig->rules() changed #4903

[harikt]

@samsonasik Happy to see the bug has been resolved.