Migrate Cache namespace to php 8

[greg0ire]

This one comes with a controversial change that I did not have the courage to extract to a separate commit PR/commit (feel free to insist I do so): switching @readonly properties to native readonly. I had to tweak some tests to achieve that, and I've extracted that part in a separate commit, which I can backport if you think it should happen.

[greg0ire]

This doesn't seem to be an issue with Psalm v5: https://psalm.dev/r/6cd352a83c

[derrabus]

This one comes with a controversial change […]: switching @readonly properties to native readonly.

That was then plan after all. Good job. 🙂

I had to tweak some tests to achieve that, and I've extracted that part in a separate commit, which I can backport if you think it should happen.

I think that would be a good idea.

I think, Doctrine\ORM\Cache\CacheKey::$hash can also be declared as readonly. Can you have a look?

[derrabus]

Suggested change

	Closure::fromCallable($this->getCacheEntryKey(...)),
	$this->getCacheEntryKey(...),

[greg0ire]

I think, Doctrine\ORM\Cache\CacheKey::$hash can also be declared as readonly. Can you have a look?

There are a few more I missed, probably got distracted during my grep 😅

[SenseException]

I think, Doctrine\ORM\Cache\CacheKey::$hash can also be declared as readonly. Can you have a look?

I haven't done a deeper look into it, but does it make sense to introduce a constructor for the abstract class?

[SenseException]

The second argument of EntityCacheKey is an array.

Suggested change

	$generateKeys = static fn ($id): EntityCacheKey => new EntityCacheKey($assocMetadata->rootEntityName, $id);
	$generateKeys = static fn (array $id): EntityCacheKey => new EntityCacheKey($assocMetadata->rootEntityName, $id);

[greg0ire]

think, Doctrine\ORM\Cache\CacheKey::$hash can also be declared as readonly.

This causes ~300 errors and failures in the test suite, I don't think it's possible, and if it is, it should happen in another PR IMO.

[greg0ire]

I haven't done a deeper look into it, but does it make sense to introduce a constructor for the abstract class?

Ah maybe that's an easy way to fix it, yes 🤔

[greg0ire]

@SenseException that was it, thanks! I left out a few more instances of @readonly inside Parser.php, which is in another namespace, and looks a bit more involving.

[derrabus]

think, Doctrine\ORM\Cache\CacheKey::$hash can also be declared as readonly.

This causes ~300 errors and failures in the test suite

Awesome. 🙈 Let's do this in a follow-up then.

[greg0ire]

Awesome. see_no_evil Let's do this in a follow-up then.

They all stemmed from the same 5 or so lines, after following @SenseException recommendation, they're gone 🙂